SUBACK with wildcard subscriptions not supported if so.

eerimoq · May 11, 2019 · b3c0a6c · b3c0a6c
1 parent 3f586de
commit b3c0a6c
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 17 deletions.
diff --git a/mqttools/broker.py b/mqttools/broker.py
@@ -43,8 +43,8 @@ def clean(self):
         self.client = None
 
 
-def is_valid_topic(topic):
-    return '#' not in topic and '+' not in topic
+def is_wildcards_in_topic(topic):
+    return '#' in topic or '+' in topic
 
 
 class Client(object):
@@ -142,8 +142,8 @@ def on_connect(self, payload):
     def on_publish(self, payload):
         topic, message, _ = unpack_publish(payload, 0)
 
-        if not is_valid_topic(topic):
-            raise MalformedPacketError('Invalid topic in publish.')
+        if is_wildcards_in_topic(topic):
+            raise MalformedPacketError(f'Invalid topic {topic} in publish.')
 
         for session in self._broker.iter_subscribers(topic):
             session.client.publish(topic, message)
@@ -153,12 +153,12 @@ def on_subscribe(self, payload):
         reasons = bytearray()
 
         for topic in topics:
-            if is_valid_topic(topic):
+            if is_wildcards_in_topic(topic):
+                reason = SubackReasonCode.WILDCARD_SUBSCRIPTIONS_NOT_SUPPORTED
+            else:
                 self._session.subscribes.add(topic)
                 self._broker.add_subscriber(topic, self._session)
                 reason = SubackReasonCode.GRANTED_QOS_0
-            else:
-                reason = SubackReasonCode.IMPLEMENTATION_SPECIFIC_ERROR
 
             reasons.append(reason)
 
@@ -169,15 +169,12 @@ def on_unsubscribe(self, payload):
         reasons = bytearray()
 
         for topic in topics:
-            if is_valid_topic(topic):
-                if topic in self._session.subscribes:
-                    self._session.subscribes.remove(topic)
-                    self._broker.remove_subscriber(topic, self._session)
-                    reason = UnsubackReasonCode.SUCCESS
-                else:
-                    reason = UnsubackReasonCode.NO_SUBSCRIPTION_EXISTED
+            if topic in self._session.subscribes:
+                self._session.subscribes.remove(topic)
+                self._broker.remove_subscriber(topic, self._session)
+                reason = UnsubackReasonCode.SUCCESS
             else:
-                reason = UnsubackReasonCode.IMPLEMENTATION_SPECIFIC_ERROR
+                reason = UnsubackReasonCode.NO_SUBSCRIPTION_EXISTED
 
             reasons.append(reason)
 

diff --git a/tests/test_broker.py b/tests/test_broker.py
@@ -79,7 +79,7 @@ async def tester():
             subscribe = b'\x82\x0a\x00\x01\x00\x00\x04/+/b\x00'
             writer_2.write(subscribe)
             suback = await reader_2.readexactly(6)
-            self.assertEqual(suback, b'\x90\x04\x00\x01\x00\x83')
+            self.assertEqual(suback, b'\x90\x04\x00\x01\x00\xa2')
 
             writer_2.close()
             writer_3.close()
@@ -170,7 +170,7 @@ async def tester():
             unsubscribe = b'\xa2\x09\x00\x02\x00\x00\x04/a/#'
             writer_1.write(unsubscribe)
             unsuback = await reader_1.readexactly(6)
-            self.assertEqual(unsuback, b'\xb0\x04\x00\x02\x00\x83')
+            self.assertEqual(unsuback, b'\xb0\x04\x00\x02\x00\x11')
 
             # Publish /a/b and then /a/d, /a/b should not be received
             # by the subscriber.