From 483db298a840ce6c2061d5d94a3d2f3f20eae2dd Mon Sep 17 00:00:00 2001
From: Wiebe Cazemier <wiebe@halfgaar.net>
Date: Mon, 27 Jun 2022 22:38:27 +0200
Subject: [PATCH] Separate QoS 2 parsing and handling

---
 mqttpacket.cpp    | 51 +++++++++++++++++++++++++++++++++++++--------------
 mqttpacket.h      |  3 +++
 packetdatatypes.h |  5 ++---
 3 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/mqttpacket.cpp b/mqttpacket.cpp
index 2d9e936..30b0f8d 100644
--- a/mqttpacket.cpp
+++ b/mqttpacket.cpp
@@ -1223,21 +1223,29 @@ void MqttPacket::handlePubAck()
     sender->getSession()->clearQosMessage(packet_id, true);
 }
 
-/**
- * @brief MqttPacket::handlePubRec handles QoS 2 'publish received' packets. The publisher receives these.
- */
-void MqttPacket::handlePubRec()
+PubRecData MqttPacket::parsePubRecData()
 {
-    const uint16_t packet_id = readTwoBytesToUInt16();
-
-    ReasonCodes reasonCode = ReasonCodes::Success; // Default when not specified, or MQTT3
+    setPosToDataStart();
+    this->publishData.qos = 2;
+    this->packet_id = readTwoBytesToUInt16();
+    PubRecData result;
 
     if (!atEnd())
     {
-        reasonCode = static_cast<ReasonCodes>(readByte());
+        result.reasonCode = static_cast<ReasonCodes>(readByte());
     }
 
-    const bool publishTerminatesHere = reasonCode >= ReasonCodes::UnspecifiedError;
+    return result;
+}
+
+/**
+ * @brief MqttPacket::handlePubRec handles QoS 2 'publish received' packets. The publisher receives these.
+ */
+void MqttPacket::handlePubRec()
+{
+    PubRecData data = parsePubRecData();
+
+    const bool publishTerminatesHere = data.reasonCode >= ReasonCodes::UnspecifiedError;
     const bool foundAndRemoved = sender->getSession()->clearQosMessage(packet_id, publishTerminatesHere);
 
     // "If it has sent a PUBREC with a Reason Code of 0x80 or greater, the receiver MUST treat any subsequent PUBLISH packet
@@ -1254,16 +1262,24 @@ void MqttPacket::handlePubRec()
     }
 }
 
+void MqttPacket::parsePubRelData()
+{
+    // MQTT-3.6.1-1, but why do we care, and only care for certain control packets?
+    if (first_byte & 0b1101)
+        throw ProtocolError("PUBREL first byte LSB must be 0010.", ReasonCodes::MalformedPacket);
+
+    setPosToDataStart();
+    this->publishData.qos = 2;
+    this->packet_id = readTwoBytesToUInt16();
+}
+
 /**
  * @brief MqttPacket::handlePubRel handles QoS 2 'publish release'. The publisher sends these.
  */
 void MqttPacket::handlePubRel()
 {
-    // MQTT-3.6.1-1, but why do we care, and only care for certain control packets?
-    if (first_byte & 0b1101)
-        throw ProtocolError("PUBREL first byte LSB must be 0010.", ReasonCodes::MalformedPacket);
+    parsePubRelData();
 
-    const uint16_t packet_id = readTwoBytesToUInt16();
     const bool foundAndRemoved = sender->getSession()->removeIncomingQoS2MessageId(packet_id);
     const ReasonCodes reason = foundAndRemoved ? ReasonCodes::Success : ReasonCodes::PacketIdentifierNotFound;
 
@@ -1272,12 +1288,19 @@ void MqttPacket::handlePubRel()
     sender->writeMqttPacket(response);
 }
 
+void MqttPacket::parsePubComp()
+{
+    setPosToDataStart();
+    this->publishData.qos = 2;
+    this->packet_id = readTwoBytesToUInt16();
+}
+
 /**
  * @brief MqttPacket::handlePubComp handles QoS 2 'publish complete'. The publisher receives these.
  */
 void MqttPacket::handlePubComp()
 {
-    const uint16_t packet_id = readTwoBytesToUInt16();
+    parsePubComp();
     sender->getSession()->removeOutgoingQoS2MessageId(packet_id);
 }
 
diff --git a/mqttpacket.h b/mqttpacket.h
index 46b4436..32c0645 100644
--- a/mqttpacket.h
+++ b/mqttpacket.h
@@ -122,8 +122,11 @@ public:
     void handlePublish();
     void parsePubAckData();
     void handlePubAck();
+    PubRecData parsePubRecData();
     void handlePubRec();
+    void parsePubRelData();
     void handlePubRel();
+    void parsePubComp();
     void handlePubComp();
     SubAckData parseSubAckData();
 
diff --git a/packetdatatypes.h b/packetdatatypes.h
index 2937b32..1aaf7e8 100644
--- a/packetdatatypes.h
+++ b/packetdatatypes.h
@@ -54,10 +54,9 @@ struct SubAckData
     std::vector<uint8_t> subAckCodes;
 };
 
-struct PubAckData
+struct PubRecData
 {
-    uint16_t packet_id;
+    ReasonCodes reasonCode = ReasonCodes::Success; // Default when not specified, or MQTT3;
 };
 
-
 #endif // PACKETDATATYPES_H
--
libgit2 0.21.4