Browse Source

[2922] Unsubscribe from group not subscribed to

Make sure msgq does not send notification when a client tries to
unsubscribe from a group it did not subscribe to.
Michal 'vorner' Vaner 12 years ago
parent
commit
9d30cc97c9
2 changed files with 18 additions and 8 deletions
  1. 9 7
      src/bin/msgq/msgq.py.in
  2. 9 1
      src/bin/msgq/tests/msgq_test.py

+ 9 - 7
src/bin/msgq/msgq.py.in

@@ -124,6 +124,8 @@ class SubscriptionManager:
         if target in self.subscriptions:
             if socket in self.subscriptions[target]:
                 self.subscriptions[target].remove(socket)
+                return True
+        return False
 
     def unsubscribe_all(self, socket):
         """Remove the socket from all subscriptions."""
@@ -703,13 +705,13 @@ class MsgQ:
         instance = routing[CC_HEADER_INSTANCE]
         if group == None or instance == None:
             return  # ignore invalid packets entirely
-        self.subs.unsubscribe(group, instance, sock)
-        lname = self.fd_to_lname[sock.fileno()]
-        self.members_notify('unsubscribed',
-                            {
-                                'client': lname,
-                                'group': group
-                            })
+        if self.subs.unsubscribe(group, instance, sock):
+            lname = self.fd_to_lname[sock.fileno()]
+            self.members_notify('unsubscribed',
+                                {
+                                    'client': lname,
+                                    'group': group
+                                })
 
     def run(self):
         """Process messages.  Forever.  Mostly."""

+ 9 - 1
src/bin/msgq/tests/msgq_test.py

@@ -63,7 +63,9 @@ class TestSubscriptionManager(unittest.TestCase):
         socks = [ 's1', 's2', 's3', 's4', 's5' ]
         for s in socks:
             self.sm.subscribe("a", "*", s)
-        self.sm.unsubscribe("a", "*", 's3')
+        self.assertTrue(self.sm.unsubscribe("a", "*", 's3'))
+        # Unsubscribe from group it is not in
+        self.assertFalse(self.sm.unsubscribe("a", "*", 's42'))
         self.assertEqual(self.sm.find_sub("a", "*"),
                          [ 's1', 's2', 's4', 's5' ])
 
@@ -300,6 +302,12 @@ class MsgQTest(unittest.TestCase):
                          notifications)
         notifications.clear()
 
+        # Unsubscription from a group it isn't subscribed to
+        self.__msgq.process_command_unsubscribe(sock, {'group': 'H',
+                                                       'instance': '*'},
+                                                None)
+        self.assertEqual([], notifications)
+
         # And, finally, for removal of client
         self.__msgq.kill_socket(sock.fileno(), sock)
         self.assertEqual([('disconnected', {'client': lname})], notifications)