From 6a6bfe47f0b10405de38e87941ff565befe234d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Sat, 3 Dec 2022 20:40:19 +0100 Subject: [PATCH 1/6] make _wait_for_msg() more readable --- adafruit_minimqtt/adafruit_minimqtt.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index b01a0780..445dbdfd 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -47,6 +47,7 @@ # MQTT Commands MQTT_PINGREQ = b"\xc0\0" MQTT_PINGRESP = const(0xD0) +MQTT_PUBLISH = const(0x30) MQTT_SUB = b"\x82" MQTT_UNSUB = b"\xA2" MQTT_DISCONNECT = b"\xe0\0" @@ -879,7 +880,9 @@ def loop(self, timeout=0): def _wait_for_msg(self, timeout=0.1): # pylint: disable = too-many-return-statements - """Reads and processes network events.""" + """Reads and processes network events. + Return the packet type or None if there is nothing to be received. + """ # CPython socket module contains a timeout attribute if hasattr(self._socket_pool, "timeout"): try: @@ -909,8 +912,11 @@ def _wait_for_msg(self, timeout=0.1): "Unexpected PINGRESP returned from broker: {}.".format(sz) ) return MQTT_PINGRESP - if res[0] & 0xF0 != 0x30: + + if res[0] & 0xF0 != MQTT_PUBLISH: return res[0] + + # Handle only the PUBLISH packet type from now on. sz = self._recv_len() # topic length MSB & LSB topic_len = self._sock_exact_recv(2) @@ -923,12 +929,13 @@ def _wait_for_msg(self, timeout=0.1): pid = self._sock_exact_recv(2) pid = pid[0] << 0x08 | pid[1] sz -= 0x02 + # read message contents raw_msg = self._sock_exact_recv(sz) msg = raw_msg if self._use_binary_mode else str(raw_msg, "utf-8") if self.logger is not None: self.logger.debug( - "Receiving SUBSCRIBE \nTopic: %s\nMsg: %s\n", topic, raw_msg + "Receiving PUBLISH \nTopic: %s\nMsg: %s\n", topic, raw_msg ) self._handle_on_message(self, topic, msg) if res[0] & 0x06 == 0x02: @@ -937,6 +944,7 @@ def _wait_for_msg(self, timeout=0.1): self._sock.send(pkt) elif res[0] & 6 == 4: assert 0 + return res[0] def _recv_len(self): From 2d7491d5c9787ab304b77eb82d3bb6fa98bf0893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Sat, 3 Dec 2022 21:03:10 +0100 Subject: [PATCH 2/6] avoid the reserved bits --- adafruit_minimqtt/adafruit_minimqtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index 445dbdfd..eca7ff0c 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -903,7 +903,7 @@ def _wait_for_msg(self, timeout=0.1): if res in [None, b"", b"\x00"]: # If we get here, it means that there is nothing to be received return None - if res[0] == MQTT_PINGRESP: + if res[0] & 0xF0 == MQTT_PINGRESP: if self.logger is not None: self.logger.debug("Got PINGRESP") sz = self._sock_exact_recv(1)[0] From 4e38f9ee96cfee6a997a8bcadf207f35fa370700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Sat, 3 Dec 2022 21:41:41 +0100 Subject: [PATCH 3/6] check topic length against remaining length --- adafruit_minimqtt/adafruit_minimqtt.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index eca7ff0c..c79ae15b 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -921,6 +921,12 @@ def _wait_for_msg(self, timeout=0.1): # topic length MSB & LSB topic_len = self._sock_exact_recv(2) topic_len = (topic_len[0] << 8) | topic_len[1] + + if topic_len > sz - 2: + raise MMQTTException( + f"Topic length {topic_len} in PUBLISH packet exceeds remaining length {sz} - 2" + ) + topic = self._sock_exact_recv(topic_len) topic = str(topic, "utf-8") sz -= topic_len + 2 From 94589a86635d1406f23863a8f9d033abd2633192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Sun, 4 Dec 2022 12:05:34 +0100 Subject: [PATCH 4/6] remove duplicate initialization --- adafruit_minimqtt/adafruit_minimqtt.py | 1 - 1 file changed, 1 deletion(-) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index c79ae15b..367181c1 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -211,7 +211,6 @@ def __init__( # LWT self._lw_topic = None self._lw_qos = 0 - self._lw_topic = None self._lw_msg = None self._lw_retain = False From 29e60e9a8e050803675a70cb14ba9fc52b845dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Mon, 5 Dec 2022 17:10:23 +0100 Subject: [PATCH 5/6] use const for the packet type mask --- adafruit_minimqtt/adafruit_minimqtt.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index 367181c1..b2e6aea1 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -52,6 +52,8 @@ MQTT_UNSUB = b"\xA2" MQTT_DISCONNECT = b"\xe0\0" +MQTT_PKT_TYPE_MASK = const(0xF0) + # Variable CONNECT header [MQTT 3.1.2] MQTT_HDR_CONNECT = bytearray(b"\x04MQTT\x04\x02\0\0") @@ -902,7 +904,7 @@ def _wait_for_msg(self, timeout=0.1): if res in [None, b"", b"\x00"]: # If we get here, it means that there is nothing to be received return None - if res[0] & 0xF0 == MQTT_PINGRESP: + if res[0] & MQTT_PKT_TYPE_MASK == MQTT_PINGRESP: if self.logger is not None: self.logger.debug("Got PINGRESP") sz = self._sock_exact_recv(1)[0] @@ -912,7 +914,7 @@ def _wait_for_msg(self, timeout=0.1): ) return MQTT_PINGRESP - if res[0] & 0xF0 != MQTT_PUBLISH: + if res[0] & MQTT_PKT_TYPE_MASK != MQTT_PUBLISH: return res[0] # Handle only the PUBLISH packet type from now on. From b76d6bbfbd6a78d48b661f9e8892e8756260524a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Kotal?= Date: Mon, 5 Dec 2022 17:11:14 +0100 Subject: [PATCH 6/6] use MQTT_PUBLISH instead of literal --- adafruit_minimqtt/adafruit_minimqtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py index b2e6aea1..99b0c7e0 100644 --- a/adafruit_minimqtt/adafruit_minimqtt.py +++ b/adafruit_minimqtt/adafruit_minimqtt.py @@ -632,7 +632,7 @@ def publish(self, topic, msg, retain=False, qos=0): ), "Quality of Service Level 2 is unsupported by this library." # fixed header. [3.3.1.2], [3.3.1.3] - pub_hdr_fixed = bytearray([0x30 | retain | qos << 1]) + pub_hdr_fixed = bytearray([MQTT_PUBLISH | retain | qos << 1]) # variable header = 2-byte Topic length (big endian) pub_hdr_var = bytearray(struct.pack(">H", len(topic.encode("utf-8"))))