From 5c278f1aec6a7c96c7090b994666f96061162485 Mon Sep 17 00:00:00 2001 From: tomoaki Date: Fri, 19 May 2017 09:44:51 +0900 Subject: [PATCH] BugFix: MQTTSNDeserialize_XXXX() functions don't return error code when error occurs. Do not store the length of packet into rc. If the packet is incorrect, skip the processing that follows. Bugfix: Print out the value of TopicId in SUBACK and UNSUBACK. Signed-off-by: tomoaki --- .../GatewayTester/src/LMqttsnClientApp.h | 2 +- .../GatewayTester/src/LSubscribeManager.cpp | 4 +-- .../src/MQTTSNGWConnectionHandler.cpp | 31 ++++++++++--------- MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp | 22 ++++++++++--- .../src/MQTTSNGWSubscribeHandler.cpp | 10 ++++-- MQTTSNPacket/src/MQTTSNConnectClient.c | 6 ++-- MQTTSNPacket/src/MQTTSNConnectServer.c | 8 ++--- MQTTSNPacket/src/MQTTSNDeserializePublish.c | 10 +++--- MQTTSNPacket/src/MQTTSNSearchClient.c | 4 +-- MQTTSNPacket/src/MQTTSNSearchServer.c | 2 +- MQTTSNPacket/src/MQTTSNSubscribeClient.c | 2 +- MQTTSNPacket/src/MQTTSNSubscribeServer.c | 2 +- MQTTSNPacket/src/MQTTSNUnsubscribeClient.c | 2 +- MQTTSNPacket/src/MQTTSNUnsubscribeServer.c | 2 +- 14 files changed, 64 insertions(+), 43 deletions(-) diff --git a/MQTTSNGateway/GatewayTester/src/LMqttsnClientApp.h b/MQTTSNGateway/GatewayTester/src/LMqttsnClientApp.h index 6457abb..627ac92 100644 --- a/MQTTSNGateway/GatewayTester/src/LMqttsnClientApp.h +++ b/MQTTSNGateway/GatewayTester/src/LMqttsnClientApp.h @@ -185,7 +185,7 @@ struct LUdpConfig{ /*================================= * Starting prompt ==================================*/ -#define TESTER_VERSION " * Version: 0.1.0" +#define TESTER_VERSION " * Version: 1.1.0" #define PAHO_COPYRIGHT0 " * MQTT-SN Gateway Tester" #define PAHO_COPYRIGHT1 " * Part of Project Paho in Eclipse" diff --git a/MQTTSNGateway/GatewayTester/src/LSubscribeManager.cpp b/MQTTSNGateway/GatewayTester/src/LSubscribeManager.cpp index 3133476..d0c13bf 100644 --- a/MQTTSNGateway/GatewayTester/src/LSubscribeManager.cpp +++ b/MQTTSNGateway/GatewayTester/src/LSubscribeManager.cpp @@ -222,7 +222,7 @@ void LSubscribeManager::responce(const uint8_t* msg) { tt->add((char*) elm->topicName, topicId, elm->topicType, elm->callback); getElement(msgId)->done = SUB_DONE; - DISPLAY("\033[0m\033[0;32m Topic \"%s\" Id : %d was Subscribed. \033[0m\033[0;37m\n\n", getElement(msgId)->topicName, getElement(msgId)->topicId); + DISPLAY("\033[0m\033[0;32m Topic \"%s\" Id : %d was Subscribed. \033[0m\033[0;37m\n\n", getElement(msgId)->topicName, topicId); } else { @@ -239,7 +239,7 @@ void LSubscribeManager::responce(const uint8_t* msg) { LTopicTable* tt = theClient->getGwProxy()->getTopicTable(); tt->setCallback(elm->topicName, 0); - DISPLAY("\033[0m\033[0;32m Topic \"%s\" Id : %d was Unsubscribed. \033[0m\033[0;37m\n\n", getElement(msgId)->topicName, getElement(msgId)->topicId); + DISPLAY("\033[0m\033[0;32m Topic \"%s\" was Unsubscribed. \033[0m\033[0;37m\n\n", getElement(msgId)->topicName); remove(getElement(msgId)); } else diff --git a/MQTTSNGateway/src/MQTTSNGWConnectionHandler.cpp b/MQTTSNGateway/src/MQTTSNGWConnectionHandler.cpp index 3eeba5e..9512ad5 100644 --- a/MQTTSNGateway/src/MQTTSNGWConnectionHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWConnectionHandler.cpp @@ -69,7 +69,10 @@ void MQTTSNConnectionHandler::handleSearchgw(MQTTSNPacket* packet) void MQTTSNConnectionHandler::handleConnect(Client* client, MQTTSNPacket* packet) { MQTTSNPacket_connectData data; - packet->getCONNECT(&data); + if ( packet->getCONNECT(&data) == 0 ) + { + return; + } /* clear ConnectData of Client */ Connect* connectData = client->getConnectData(); @@ -142,7 +145,10 @@ void MQTTSNConnectionHandler::handleWilltopic(Client* client, MQTTSNPacket* pack uint8_t willRetain; MQTTSNString willTopic; - packet->getWILLTOPIC(&willQos, &willRetain, &willTopic); + if ( packet->getWILLTOPIC(&willQos, &willRetain, &willTopic) == 0 ) + { + return; + } client->setWillTopic(willTopic); Connect* connectData = client->getConnectData(); @@ -168,17 +174,6 @@ void MQTTSNConnectionHandler::handleWillmsg(Client* client, MQTTSNPacket* packet if ( !client->isWaitWillMsg() ) { DEBUGLOG(" MQTTSNConnectionHandler::handleWillmsg WaitWillMsgFlg is off.\n"); - //if ( !client->isSecureNetwork() ) - //{ - // /* create CONNACK message */ - // MQTTSNPacket* connack = new MQTTSNPacket(); - // connack->setCONNACK(MQTTSN_RC_REJECTED_CONGESTED); - - // /* return to the client */ - // Event* evt = new Event(); - // evt->setClientSendEvent(client, connack); - // _gateway->getClientSendQue()->post(evt); - //} return; } @@ -188,7 +183,10 @@ void MQTTSNConnectionHandler::handleWillmsg(Client* client, MQTTSNPacket* packet if( client->isConnectSendable() ) { /* save WillMsg in the client */ - packet->getWILLMSG(&willmsg); + if ( packet->getWILLMSG(&willmsg) == 0 ) + { + return; + } client->setWillMsg(willmsg); /* create CONNECT message */ @@ -216,7 +214,10 @@ void MQTTSNConnectionHandler::handleDisconnect(Client* client, MQTTSNPacket* pac _gateway->getClientSendQue()->post(ev); uint16_t duration = 0; - packet->getDISCONNECT(&duration); + if ( packet->getDISCONNECT(&duration) == 0 ) + { + return; + } if ( duration == 0 ) { MQTTGWPacket* mqMsg = new MQTTGWPacket(); diff --git a/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp b/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp index e23bcae..b4e091e 100644 --- a/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWPublishHandler.cpp @@ -56,7 +56,10 @@ void MQTTSNPublishHandler::handlePublish(Client* client, MQTTSNPacket* packet) return; } - packet->getPUBLISH(&dup, &qos, &retained, &msgId, &topicid, &payload, &payloadlen); + if ( packet->getPUBLISH(&dup, &qos, &retained, &msgId, &topicid, &payload, &payloadlen) ==0 ) + { + return; + } pub.msgId = msgId; pub.header.bits.dup = dup; pub.header.bits.qos = qos; @@ -180,7 +183,12 @@ void MQTTSNPublishHandler::handlePuback(Client* client, MQTTSNPacket* packet) return; } MQTTGWPacket* pubAck = new MQTTGWPacket(); - packet->getPUBACK(&topicId, &msgId, &rc); + + if ( packet->getPUBACK(&topicId, &msgId, &rc) == 0 ) + { + return; + } + if ( rc == MQTTSN_RC_ACCEPTED) { pubAck->setAck(PUBACK, msgId); @@ -202,7 +210,10 @@ void MQTTSNPublishHandler::handleAck(Client* client, MQTTSNPacket* packet, uint8 { return; } - packet->getACK(&msgId); + if ( packet->getACK(&msgId) == 0 ) + { + return; + } MQTTGWPacket* ackPacket = new MQTTGWPacket(); ackPacket->setAck(packetType, msgId); Event* ev1 = new Event(); @@ -223,7 +234,10 @@ void MQTTSNPublishHandler::handleRegister(Client* client, MQTTSNPacket* packet) return; } MQTTSNPacket* regAck = new MQTTSNPacket(); - packet->getREGISTER(&id, &msgId, &topicName); + if ( packet->getREGISTER(&id, &msgId, &topicName) == 0 ) + { + return; + } topicid.type = MQTTSN_TOPIC_TYPE_NORMAL; topicid.data.long_.len = topicName.lenstring.len; diff --git a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp index f720ba9..3b93e41 100644 --- a/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp +++ b/MQTTSNGateway/src/MQTTSNGWSubscribeHandler.cpp @@ -41,7 +41,10 @@ void MQTTSNSubscribeHandler::handleSubscribe(Client* client, MQTTSNPacket* packe MQTTSN_topicid topicFilter; Topic* topic = 0; - packet->getSUBSCRIBE(&dup, &qos, &msgId, &topicFilter); + if ( packet->getSUBSCRIBE(&dup, &qos, &msgId, &topicFilter) == 0 ) + { + return; + } if (topicFilter.type <= MQTTSN_TOPIC_TYPE_SHORT) { @@ -139,7 +142,10 @@ void MQTTSNSubscribeHandler::handleUnsubscribe(Client* client, MQTTSNPacket* pac uint16_t msgId; MQTTSN_topicid topicFilter; - packet->getUNSUBSCRIBE(&msgId, &topicFilter); + if ( packet->getUNSUBSCRIBE(&msgId, &topicFilter) == 0 ) + { + return; + } if ( topicFilter.type == MQTTSN_TOPIC_TYPE_PREDEFINED ) { diff --git a/MQTTSNPacket/src/MQTTSNConnectClient.c b/MQTTSNPacket/src/MQTTSNConnectClient.c index d62d3c1..2db7817 100644 --- a/MQTTSNPacket/src/MQTTSNConnectClient.c +++ b/MQTTSNPacket/src/MQTTSNConnectClient.c @@ -200,7 +200,7 @@ int MQTTSNDeserialize_pingresp(unsigned char* buf, int buflen) int mylen; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata < 2) goto exit; @@ -428,7 +428,7 @@ int MQTTSNDeserialize_willtopicresp(int* resp_rc, unsigned char* buf, int buflen int mylen; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - buf < 3) goto exit; @@ -460,7 +460,7 @@ int MQTTSNDeserialize_willmsgresp(int* resp_rc, unsigned char* buf, int buflen) int mylen; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - buf < 3) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNConnectServer.c b/MQTTSNPacket/src/MQTTSNConnectServer.c index 0647bf2..1b707ef 100644 --- a/MQTTSNPacket/src/MQTTSNConnectServer.c +++ b/MQTTSNPacket/src/MQTTSNConnectServer.c @@ -110,7 +110,7 @@ int MQTTSNDeserialize_disconnect(int* duration, unsigned char* buf, int buflen) int mylen; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata < 1) goto exit; @@ -202,7 +202,7 @@ int MQTTSNDeserialize_pingreq(MQTTSNString* clientID, unsigned char* buf, int le int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, len, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, len, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata < 1) goto exit; @@ -265,7 +265,7 @@ int MQTTSNDeserialize_willtopic1(int *willQoS, unsigned char *willRetain, MQTTSN int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, len, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, len, &mylen); /* read length */ enddata = buf + mylen; if (enddata > buf + len) goto exit; @@ -327,7 +327,7 @@ int MQTTSNDeserialize_willmsg1(MQTTSNString* willMsg, unsigned char* buf, int le int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, len, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, len, &mylen); /* read length */ enddata = buf + mylen; if (enddata > buf + len) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNDeserializePublish.c b/MQTTSNPacket/src/MQTTSNDeserializePublish.c index 8dd0f7e..98de94b 100644 --- a/MQTTSNPacket/src/MQTTSNDeserializePublish.c +++ b/MQTTSNPacket/src/MQTTSNDeserializePublish.c @@ -43,7 +43,7 @@ int MQTTSNDeserialize_publish(unsigned char* dup, int* qos, unsigned char* retai int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; @@ -95,7 +95,7 @@ int MQTTSNDeserialize_puback(unsigned short* topicid, unsigned short* packetid, int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; @@ -130,7 +130,7 @@ int MQTTSNDeserialize_ack(unsigned char* type, unsigned short* packetid, unsigne int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; @@ -166,7 +166,7 @@ int MQTTSNDeserialize_register(unsigned short* topicid, unsigned short* packetid int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; @@ -206,7 +206,7 @@ int MQTTSNDeserialize_regack(unsigned short* topicid, unsigned short* packetid, int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNSearchClient.c b/MQTTSNPacket/src/MQTTSNSearchClient.c index de2f4fe..c55e134 100644 --- a/MQTTSNPacket/src/MQTTSNSearchClient.c +++ b/MQTTSNPacket/src/MQTTSNSearchClient.c @@ -36,7 +36,7 @@ int MQTTSNDeserialize_advertise(unsigned char* gatewayid, unsigned short* durati int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; @@ -105,7 +105,7 @@ int MQTTSNDeserialize_gwinfo(unsigned char* gatewayid, unsigned short* gatewayad int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNSearchServer.c b/MQTTSNPacket/src/MQTTSNSearchServer.c index 6fcc479..c83fd16 100644 --- a/MQTTSNPacket/src/MQTTSNSearchServer.c +++ b/MQTTSNPacket/src/MQTTSNSearchServer.c @@ -68,7 +68,7 @@ int MQTTSNDeserialize_searchgw(unsigned char* radius, unsigned char* buf, int bu int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNSubscribeClient.c b/MQTTSNPacket/src/MQTTSNSubscribeClient.c index 42a34df..97680ed 100644 --- a/MQTTSNPacket/src/MQTTSNSubscribeClient.c +++ b/MQTTSNPacket/src/MQTTSNSubscribeClient.c @@ -115,7 +115,7 @@ int MQTTSNDeserialize_suback(int* qos, unsigned short* topicid, unsigned short* int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNSubscribeServer.c b/MQTTSNPacket/src/MQTTSNSubscribeServer.c index 8e48919..54f6faa 100644 --- a/MQTTSNPacket/src/MQTTSNSubscribeServer.c +++ b/MQTTSNPacket/src/MQTTSNSubscribeServer.c @@ -39,7 +39,7 @@ int MQTTSNDeserialize_subscribe(unsigned char* dup, int* qos, unsigned short* pa int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNUnsubscribeClient.c b/MQTTSNPacket/src/MQTTSNUnsubscribeClient.c index 092ce48..9d952f7 100644 --- a/MQTTSNPacket/src/MQTTSNUnsubscribeClient.c +++ b/MQTTSNPacket/src/MQTTSNUnsubscribeClient.c @@ -97,7 +97,7 @@ int MQTTSNDeserialize_unsuback(unsigned short* packetid, unsigned char* buf, int int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit; diff --git a/MQTTSNPacket/src/MQTTSNUnsubscribeServer.c b/MQTTSNPacket/src/MQTTSNUnsubscribeServer.c index 61c61a0..fe727bc 100644 --- a/MQTTSNPacket/src/MQTTSNUnsubscribeServer.c +++ b/MQTTSNPacket/src/MQTTSNUnsubscribeServer.c @@ -27,7 +27,7 @@ int MQTTSNDeserialize_unsubscribe(unsigned short* packetid, MQTTSN_topicid* topi int mylen = 0; FUNC_ENTRY; - curdata += (rc = MQTTSNPacket_decode(curdata, buflen, &mylen)); /* read length */ + curdata += MQTTSNPacket_decode(curdata, buflen, &mylen); /* read length */ enddata = buf + mylen; if (enddata - curdata > buflen) goto exit;