From 594db623ee2aaac47960f6f29acd2e71e35212d8 Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:18:46 +0100 Subject: [PATCH 1/8] Lock critical section when adding new Client to list Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWClient.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MQTTSNGateway/src/MQTTSNGWClient.cpp b/MQTTSNGateway/src/MQTTSNGWClient.cpp index 9a8a043..f5ab22c 100644 --- a/MQTTSNGateway/src/MQTTSNGWClient.cpp +++ b/MQTTSNGateway/src/MQTTSNGWClient.cpp @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #include "MQTTSNGWClient.h" @@ -236,6 +237,8 @@ Client* ClientList::createClient(SensorNetAddress* addr, MQTTSNString* clientId, free(dummyId.cstring); } + _mutex.lock(); + /* add the list */ if ( _firstClient == 0 ) { From 1ef38fb7a0529775e694ee21e3a8931d812502ce Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:20:32 +0100 Subject: [PATCH 2/8] Fix adding elements to WaitREGACKPacketList Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWClient.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/MQTTSNGateway/src/MQTTSNGWClient.cpp b/MQTTSNGateway/src/MQTTSNGWClient.cpp index f5ab22c..ad7f48b 100644 --- a/MQTTSNGateway/src/MQTTSNGWClient.cpp +++ b/MQTTSNGateway/src/MQTTSNGWClient.cpp @@ -1155,8 +1155,12 @@ int WaitREGACKPacketList::setPacket(MQTTSNPacket* packet, uint16_t REGACKMsgId) _first = elm; _end = elm; } - elm->_prev = _end; - _end->_next = elm; + else + { + _end->_next = elm; + elm->_prev = _end; + _end = elm; + } return 1; } From 6dffa66bb05cb153e8f6b214d96f7a6247ac6e8c Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:23:36 +0100 Subject: [PATCH 3/8] Fix gateway crash when subscribing to short topics (two octets length) Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp index 3b93e41..1640b4b 100644 --- a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #include "MQTTSNGWSubscribeHandler.h" @@ -89,6 +90,7 @@ void MQTTSNSubscribeHandler::handleSubscribe(Client* client, MQTTSNPacket* packe } else { + uint16_t topicId = 0; MQTTGWPacket* subscribe = new MQTTGWPacket(); topic = client->getTopics()->getTopic(&topicFilter); if (topic == 0) @@ -96,6 +98,7 @@ void MQTTSNSubscribeHandler::handleSubscribe(Client* client, MQTTSNPacket* packe if (topicFilter.type == MQTTSN_TOPIC_TYPE_NORMAL) { topic = client->getTopics()->add(&topicFilter); + topicId = topic->getTopicId(); subscribe->setSUBSCRIBE((char*)topic->getTopicName()->c_str(), (uint8_t)qos, (uint16_t)msgId); } else if (topicFilter.type == MQTTSN_TOPIC_TYPE_SHORT) @@ -104,17 +107,19 @@ void MQTTSNSubscribeHandler::handleSubscribe(Client* client, MQTTSNPacket* packe topic[0] = topicFilter.data.short_name[0]; topic[1] = topicFilter.data.short_name[1]; topic[2] = 0; + topicId = topicFilter.data.id; subscribe->setSUBSCRIBE(topic, (uint8_t)qos, (uint16_t)msgId); } } else { + topicId = topic->getTopicId(); subscribe->setSUBSCRIBE((char*)topic->getTopicName()->c_str(), (uint8_t)qos, (uint16_t)msgId); } if ( msgId > 0 ) { - client->setWaitedSubTopicId(msgId, topic->getTopicId(), topicFilter.type); + client->setWaitedSubTopicId(msgId, topicId, topicFilter.type); } Event* ev1 = new Event(); From fbab6ee91faa87c5aef398d44c430d0fcf2c8622 Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:24:52 +0100 Subject: [PATCH 4/8] Fix unsubscribing from short topics (two octets length) Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp index 1640b4b..5d4aab8 100644 --- a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp @@ -185,7 +185,6 @@ void MQTTSNSubscribeHandler::handleUnsubscribe(Client* client, MQTTSNPacket* pac } else if (topicFilter.type == MQTTSN_TOPIC_TYPE_SHORT) { - MQTTGWPacket* unsubscribe = new MQTTGWPacket(); char shortTopic[3]; shortTopic[0] = topicFilter.data.short_name[0]; shortTopic[1] = topicFilter.data.short_name[1]; From c49a0405698b6dd805b03283a6079f17bc25fa44 Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:29:36 +0100 Subject: [PATCH 5/8] Preventing memory leaks Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTGWPublishHandler.cpp | 3 +++ MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/MQTTSNGateway/src/MQTTGWPublishHandler.cpp b/MQTTSNGateway/src/MQTTGWPublishHandler.cpp index c6ebcef..0b818ab 100644 --- a/MQTTSNGateway/src/MQTTGWPublishHandler.cpp +++ b/MQTTSNGateway/src/MQTTGWPublishHandler.cpp @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #include "MQTTGWPublishHandler.h" @@ -113,6 +114,8 @@ void MQTTGWPublishHandler::handlePublish(Client* client, MQTTGWPacket* packet) { replyACK(client, &pub, PUBREC); } + + delete snPacket; return; } diff --git a/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp b/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp index ea7f9b0..24b077d 100644 --- a/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #include "MQTTSNGWPublishHandler.h" @@ -180,8 +181,6 @@ void MQTTSNPublishHandler::handlePuback(Client* client, MQTTSNPacket* packet) if ( client->isActive() ) { - MQTTGWPacket* pubAck = new MQTTGWPacket(); - if ( packet->getPUBACK(&topicId, &msgId, &rc) == 0 ) { return; @@ -189,6 +188,7 @@ void MQTTSNPublishHandler::handlePuback(Client* client, MQTTSNPacket* packet) if ( rc == MQTTSN_RC_ACCEPTED) { + MQTTGWPacket* pubAck = new MQTTGWPacket(); pubAck->setAck(PUBACK, msgId); Event* ev1 = new Event(); ev1->setBrokerSendEvent(client, pubAck); @@ -226,10 +226,8 @@ void MQTTSNPublishHandler::handleRegister(Client* client, MQTTSNPacket* packet) MQTTSNString topicName; MQTTSN_topicid topicid; - if ( client->isActive() || client->isAwake()) { - MQTTSNPacket* regAck = new MQTTSNPacket(); if ( packet->getREGISTER(&id, &msgId, &topicName) == 0 ) { return; @@ -240,6 +238,8 @@ void MQTTSNPublishHandler::handleRegister(Client* client, MQTTSNPacket* packet) topicid.data.long_.name = topicName.lenstring.data; id = client->getTopics()->add(&topicid)->getTopicId(); + + MQTTSNPacket* regAck = new MQTTSNPacket(); regAck->setREGACK(id, msgId, MQTTSN_RC_ACCEPTED); Event* ev = new Event(); ev->setClientSendEvent(client, regAck); From bba3f1dede59d3cd031fdae5462a0664fb9fd1ae Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:30:45 +0100 Subject: [PATCH 6/8] Fix topic name registration procedure started by GW when handling PUBLISH message Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTGWPublishHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/MQTTSNGateway/src/MQTTGWPublishHandler.cpp b/MQTTSNGateway/src/MQTTGWPublishHandler.cpp index 0b818ab..12ad28e 100644 --- a/MQTTSNGateway/src/MQTTGWPublishHandler.cpp +++ b/MQTTSNGateway/src/MQTTGWPublishHandler.cpp @@ -129,6 +129,7 @@ void MQTTGWPublishHandler::handlePublish(Client* client, MQTTGWPacket* packet) MQTTSNPacket* regPacket = new MQTTSNPacket(); MQTTSNString topicName; + topicName.cstring = 0; topicName.lenstring.len = topicId.data.long_.len; topicName.lenstring.data = topicId.data.long_.name; From b6192d863b4bed9217424d61cbc4fda0e30ab53d Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:32:07 +0100 Subject: [PATCH 7/8] Fix handling PUBREC message received from client Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWPacketHandleTask.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MQTTSNGateway/src/MQTTSNGWPacketHandleTask.cpp b/MQTTSNGateway/src/MQTTSNGWPacketHandleTask.cpp index 43cdd24..02dd9ab 100644 --- a/MQTTSNGateway/src/MQTTSNGWPacketHandleTask.cpp +++ b/MQTTSNGateway/src/MQTTSNGWPacketHandleTask.cpp @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #include "MQTTSNGWDefines.h" @@ -155,6 +156,7 @@ void PacketHandleTask::run() case MQTTSN_PUBACK: _mqttsnPublish->handlePuback(client, snPacket); break; + case MQTTSN_PUBREC: _mqttsnPublish->handleAck(client, snPacket, PUBREC); break; case MQTTSN_PUBREL: From ef715ebfa71536a2b7b49e620e43a12bc774428f Mon Sep 17 00:00:00 2001 From: Mariusz Suchora Date: Fri, 23 Feb 2018 10:40:06 +0100 Subject: [PATCH 8/8] Set pointer to client to 0 when client was erased Signed-off-by: Mariusz Suchora --- MQTTSNGateway/src/MQTTSNGWClient.cpp | 2 +- MQTTSNGateway/src/MQTTSNGWClient.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/MQTTSNGateway/src/MQTTSNGWClient.cpp b/MQTTSNGateway/src/MQTTSNGWClient.cpp index ad7f48b..c0a7d03 100644 --- a/MQTTSNGateway/src/MQTTSNGWClient.cpp +++ b/MQTTSNGateway/src/MQTTSNGWClient.cpp @@ -124,7 +124,7 @@ bool ClientList::authorize(const char* fileName) return _authorize; } -void ClientList::erase(Client* client) +void ClientList::erase(Client*& client) { if ( !_authorize && client->erasable()) { diff --git a/MQTTSNGateway/src/MQTTSNGWClient.h b/MQTTSNGateway/src/MQTTSNGWClient.h index 690f98b..3b52fa1 100644 --- a/MQTTSNGateway/src/MQTTSNGWClient.h +++ b/MQTTSNGateway/src/MQTTSNGWClient.h @@ -12,6 +12,7 @@ * * Contributors: * Tomoaki Yamaguchi - initial API and implementation and/or initial documentation + * Tieto Poland Sp. z o.o. - Gateway improvements **************************************************************************************/ #ifndef MQTTSNGWCLIENT_H_ @@ -341,7 +342,7 @@ public: ClientList(); ~ClientList(); bool authorize(const char* fileName); - void erase(Client*); + void erase(Client*&); Client* getClient(SensorNetAddress* addr); Client* getClient(uint8_t* clientId); Client* createClient(SensorNetAddress* addr, MQTTSNString* clientId, bool unstableLine,