Peter M. Groen / FlashMQ

Commit fc709a32b4a19670d5b2b9223bc87f80481bc348

Authored by Wiebe Cazemier
1 parent ab7c79a7

Fix occasional uint16 parsing bug 

Shifting signed integers is undefined and only sometimes produced
unexpected results.

Was detected in packet identifiers in QoS testing.
Inline Side-by-side
Showing 3 changed files with 25 additions and 1 deletions
FlashMQTests/tst_maintests.cpp
  View file @fc709a3
@@ -81,6 +81,8 @@ private slots: @@ -81,6 +81,8 @@ private slots:
81 void test_validUtf8(); 81 void test_validUtf8();
82 void test_validUtf8Sse(); 82 void test_validUtf8Sse();
83 83
  84 + void testPacketInt16Parse();
  85 +
84 }; 86 };
85 87
86 MainTests::MainTests() 88 MainTests::MainTests()
@@ -755,6 +757,21 @@ void MainTests::test_validUtf8Sse() @@ -755,6 +757,21 @@ void MainTests::test_validUtf8Sse()
755 QVERIFY(!data.isValidUtf8(e)); 757 QVERIFY(!data.isValidUtf8(e));
756 } 758 }
757 759
  760 +void MainTests::testPacketInt16Parse()
  761 +{
  762 + std::vector<uint64_t> tests {128, 300, 64, 65550, 32000};
  763 +
  764 + for (const uint16_t id : tests)
  765 + {
  766 + Publish pub("hallo", "content", 1);
  767 + MqttPacket packet(pub);
  768 + packet.setPacketId(id);
  769 + packet.pos -= 2;
  770 + uint16_t idParsed = packet.readTwoBytesToUInt16();
  771 + QVERIFY(id == idParsed);
  772 + }
  773 +}
  774 +
758 QTEST_GUILESS_MAIN(MainTests) 775 QTEST_GUILESS_MAIN(MainTests)
759 776
760 #include "tst_maintests.moc" 777 #include "tst_maintests.moc"
mqttpacket.cpp
  View file @fc709a3
@@ -143,6 +143,7 @@ MqttPacket::MqttPacket(const PubAck &amp;pubAck) : @@ -143,6 +143,7 @@ MqttPacket::MqttPacket(const PubAck &amp;pubAck) :
143 writeByte(2); // length is always 2. 143 writeByte(2); // length is always 2.
144 char topicLenMSB = (pubAck.packet_id & 0xFF00) >> 8; 144 char topicLenMSB = (pubAck.packet_id & 0xFF00) >> 8;
145 char topicLenLSB = (pubAck.packet_id & 0x00FF); 145 char topicLenLSB = (pubAck.packet_id & 0x00FF);
  146 + packet_id_pos = pos;
146 writeByte(topicLenMSB); 147 writeByte(topicLenMSB);
147 writeByte(topicLenLSB); 148 writeByte(topicLenLSB);
148 } 149 }
@@ -641,7 +642,9 @@ uint16_t MqttPacket::readTwoBytesToUInt16() @@ -641,7 +642,9 @@ uint16_t MqttPacket::readTwoBytesToUInt16()
641 if (pos + 2 > bites.size()) 642 if (pos + 2 > bites.size())
642 throw ProtocolError("Invalid packet: header specifies invalid length."); 643 throw ProtocolError("Invalid packet: header specifies invalid length.");
643 644
644 - uint16_t i = bites[pos] << 8 | bites[pos+1]; 645 + uint8_t a = bites[pos];
  646 + uint8_t b = bites[pos+1];
  647 + uint16_t i = a << 8 | b;
645 pos += 2; 648 pos += 2;
646 return i; 649 return i;
647 } 650 }
mqttpacket.h
  View file @fc709a3
@@ -43,6 +43,10 @@ public: @@ -43,6 +43,10 @@ public:
43 43
44 class MqttPacket 44 class MqttPacket
45 { 45 {
  46 +#ifdef TESTING
  47 + friend class MainTests;
  48 +#endif
  49 +
46 std::string topic; 50 std::string topic;
47 std::vector<std::string> *subtopics; // comes from local thread storage. See std::vector<std::string> *ThreadData::splitTopic(std::string &topic) 51 std::vector<std::string> *subtopics; // comes from local thread storage. See std::vector<std::string> *ThreadData::splitTopic(std::string &topic)
48 std::vector<char> bites; 52 std::vector<char> bites;