From 718a495437f966ffcc4cf5aca187190ceab1983e Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 4 Feb 2021 13:19:51 +0100 Subject: [PATCH 1/3] Fix MqttPackdt payload assignment. QByteArray(str) assumes that str is '\0'-terminated which is not the case when sending binary data. --- libnymea-mqtt/mqttpacket.cpp | 5 +++-- tests/operation/test_operation.cpp | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/libnymea-mqtt/mqttpacket.cpp b/libnymea-mqtt/mqttpacket.cpp index 5cda7f4..51d0fb1 100644 --- a/libnymea-mqtt/mqttpacket.cpp +++ b/libnymea-mqtt/mqttpacket.cpp @@ -522,8 +522,9 @@ int MqttPacket::parse(const QByteArray &buffer) } memset(str, 0, MAX_STRLEN); - inputStream.readRawData(str, qMin(MAX_STRLEN, remainingLength)); - d_ptr->payload = QByteArray(str); + qint16 payloadLen = qMin(MAX_STRLEN, remainingLength); + inputStream.readRawData(str, payloadLen); + d_ptr->payload = QByteArray(str, payloadLen); break; } case TypePuback: diff --git a/tests/operation/test_operation.cpp b/tests/operation/test_operation.cpp index 75656a2..cef1d7b 100644 --- a/tests/operation/test_operation.cpp +++ b/tests/operation/test_operation.cpp @@ -82,6 +82,8 @@ private slots: void testEmptyClientId(); + void testBinaryPaylaod(); + private: // Connects and waits for the MQTT CONNECT to be finished MqttClient *connectAndWait(const QString &clientId, bool cleanSession = true, quint16 keepAlive = 300, const QString &willTopic = QString(), const QString &willMessage = QString(), Mqtt::QoS willQoS = Mqtt::QoS0, bool willRetain = false); @@ -820,6 +822,19 @@ void OperationTests::testEmptyClientId() QTRY_VERIFY2(client3.first->isConnected() == false, "Connection should have been dropped"); } +void OperationTests::testBinaryPaylaod() +{ + MqttClient *client = connectAndWait(""); + QVERIFY2(client->isConnected(), "Client did not connect"); + client->subscribe("#"); + const char binData[] = {'\xA5', '\x20', '\x00', '\x04', '\x00', '\x52'}; + QByteArray payload(QByteArray::fromRawData(binData, 6)); + QSignalSpy publishReceivedSpy(client, &MqttClient::publishReceived); + client->publish("testtopic", payload); + QTRY_VERIFY2(publishReceivedSpy.count() == 1, "Did not receive publish message"); + QCOMPARE(publishReceivedSpy.first().at(1).toByteArray(), payload); +} + #endif QTEST_MAIN(OperationTests) From 5eebdf75f7262349f4890677cd94eac3f41c2c62 Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 4 Feb 2021 13:27:10 +0100 Subject: [PATCH 2/3] Start keep alive timer before emitting connected() By chaning the order MqttClient::isConnected() returns true in slot connected to MqttClient::connected(). --- libnymea-mqtt/mqttclient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnymea-mqtt/mqttclient.cpp b/libnymea-mqtt/mqttclient.cpp index e164e89..d19dda4 100644 --- a/libnymea-mqtt/mqttclient.cpp +++ b/libnymea-mqtt/mqttclient.cpp @@ -364,9 +364,9 @@ void MqttClientPrivate::onReadyRead() } socket->write(retryPacket.serialize()); } + restartKeepAliveTimer(); // Make sure we emit connected after having handled all the retransmission queue emit q_ptr->connected(packet.connectReturnCode(), packet.connackFlags()); - restartKeepAliveTimer(); break; case MqttPacket::TypePublish: qCDebug(dbgClient) << "Publish received from server. Topic:" << packet.topic() << "Payload:" << packet.payload() << "QoS:" << packet.qos(); From 4347e0106f4ee5489f30408a24c9653f96810cc0 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Fri, 5 Feb 2021 07:29:58 +0100 Subject: [PATCH 3/3] Add a unit test for the connected signal (#1) --- tests/operation/test_operation.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/operation/test_operation.cpp b/tests/operation/test_operation.cpp index cef1d7b..a50b863 100644 --- a/tests/operation/test_operation.cpp +++ b/tests/operation/test_operation.cpp @@ -184,7 +184,16 @@ void OperationTests::connectAndDisconnect() QSignalSpy serverSpy(m_server, &MqttServer::clientConnected); QString clientId = "connectAndDisconnect-client"; - MqttClient* client = connectAndWait(clientId); + QPair result = connectToServer(clientId); + MqttClient* client = result.first; + connect(client, &MqttClient::connected, this, [client](Mqtt::ConnectReturnCode connectReturnCode, Mqtt::ConnackFlags connackFlags){ + QVERIFY2(client->isConnected(), "MqttClient::isConnected not returning true in connected signal()"); + QCOMPARE(connectReturnCode, Mqtt::ConnectReturnCodeAccepted); + QCOMPARE(connackFlags, Mqtt::ConnackFlagNone); + }); + if (result.second->count() == 0) { + result.second->wait(); + } QVERIFY2(serverSpy.count() == 1, "Server didn't emit clientConnected"); QVERIFY2(serverSpy.at(0).at(1) == clientId, "ClientId not matching on server side.");