Correct use of a trailing comma in Socket.IO packets with no id or data (Fixes #671)

Author: miguelgrinberg

socketio/packet.py

@@ -15,8 +15,7 @@ class Packet(object):
     # packet type: 1 byte, values 0-6
     # num_attachments: ASCII encoded, only if num_attachments != 0
     # '-': only if num_attachments != 0
-    # namespace: only if namespace != '/'
-    # ',': only if namespace and one of id and data are defined in this packet
+    # namespace, followed by a ',': only if namespace != '/'
     # id: ASCII encoded, only if id is not None
     # data: JSON dump of data payload
 
@@ -54,18 +53,11 @@ def encode(self):
         else:
             data = self.data
             attachments = None
-        needs_comma = False
         if self.namespace is not None and self.namespace != '/':
-            encoded_packet += self.namespace
-            needs_comma = True
+            encoded_packet += self.namespace + ','
         if self.id is not None:
-            if needs_comma:
-                encoded_packet += ','
-                needs_comma = False
             encoded_packet += str(self.id)
         if data is not None:
-            if needs_comma:
-                encoded_packet += ','
             encoded_packet += self.json.dumps(data, separators=(',', ':'))
         if attachments is not None:
             encoded_packet = [encoded_packet] + attachments

tests/asyncio/test_asyncio_server.py

@@ -431,7 +431,7 @@ def test_handle_connect_namespace(self, eio):
         handler = mock.MagicMock()
         s.on('connect', handler, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.mock.assert_called_once_with('123', '0/foo,{"sid":"1"}')
@@ -471,7 +471,7 @@ def test_handle_connect_namespace_rejected(self, eio):
         handler = mock.MagicMock(return_value=False)
         s.on('connect', handler, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.mock.assert_any_call(
@@ -498,7 +498,7 @@ def test_handle_connect_namespace_rejected_always_connect(self, eio):
         handler = mock.MagicMock(return_value=False)
         s.on('connect', handler, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.mock.assert_any_call('123', '0/foo,{"sid":"1"}')
@@ -545,7 +545,7 @@ def test_handle_connect_namespace_rejected_with_exception(self, eio):
         )
         s.on('connect', handler, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.mock.assert_called_once_with(
@@ -560,7 +560,7 @@ def test_handle_connect_namespace_rejected_with_empty_exception(self, eio):
         )
         s.on('connect', handler, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.mock.assert_called_once_with(
@@ -588,7 +588,7 @@ def test_handle_disconnect_namespace(self, eio):
         handler_namespace = mock.MagicMock()
         s.on('disconnect', handler_namespace, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         _run(s._handle_eio_disconnect('123'))
         handler.assert_not_called()
         handler_namespace.assert_called_once_with('1')
@@ -602,8 +602,8 @@ def test_handle_disconnect_only_namespace(self, eio):
         handler_namespace = mock.MagicMock()
         s.on('disconnect', handler_namespace, namespace='/foo')
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
-        _run(s._handle_eio_message('123', '1/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
+        _run(s._handle_eio_message('123', '1/foo,'))
         assert handler.call_count == 0
         handler_namespace.assert_called_once_with('1')
         assert s.environ == {'123': 'environ'}
@@ -756,7 +756,7 @@ def test_send_with_ack_namespace(self, eio):
         eio.return_value.send = AsyncMock()
         s = asyncio_server.AsyncServer()
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         cb = mock.MagicMock()
         id = s.manager._generate_ack_id('1', cb)
         _run(
@@ -833,9 +833,9 @@ def test_disconnect_namespace(self, eio):
         eio.return_value.disconnect = AsyncMock()
         s = asyncio_server.AsyncServer()
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         _run(s.disconnect('1', namespace='/foo'))
-        s.eio.send.mock.assert_any_call('123', '1/foo')
+        s.eio.send.mock.assert_any_call('123', '1/foo,')
         assert not s.manager.is_connected('1', '/foo')
 
     def test_disconnect_twice(self, eio):
@@ -854,7 +854,7 @@ def test_disconnect_twice_namespace(self, eio):
         eio.return_value.send = AsyncMock()
         s = asyncio_server.AsyncServer()
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         _run(s.disconnect('1', namespace='/foo'))
         calls = s.eio.send.mock.call_count
         assert not s.manager.is_connected('1', '/foo')
@@ -884,7 +884,7 @@ async def on_baz(self, sid, data1, data2):
         s = asyncio_server.AsyncServer(async_handlers=False)
         s.register_namespace(MyNamespace('/foo'))
         _run(s._handle_eio_connect('123', 'environ'))
-        _run(s._handle_eio_message('123', '0/foo'))
+        _run(s._handle_eio_message('123', '0/foo,'))
         assert result['result'] == ('1', 'environ')
         _run(s._handle_eio_message('123', '2/foo,["foo","a"]'))
         assert result['result'] == ('1', 'a')

tests/common/test_packet.py

@@ -111,12 +111,13 @@ def test_decode_namespace_with_query_string(self):
 
     def test_encode_namespace_no_data(self):
         pkt = packet.Packet(packet_type=packet.EVENT, namespace='/bar')
-        assert pkt.encode() == '2/bar'
+        assert pkt.encode() == '2/bar,'
 
     def test_decode_namespace_no_data(self):
-        pkt = packet.Packet(encoded_packet='2/bar')
+        pkt = packet.Packet(encoded_packet='2/bar,')
         assert pkt.namespace == '/bar'
-        assert pkt.encode() == '2/bar'
+        assert pkt.data is None
+        assert pkt.encode() == '2/bar,'
 
     def test_encode_namespace_with_hyphens(self):
         pkt = packet.Packet(

tests/common/test_server.py

@@ -359,7 +359,7 @@ def test_handle_connect_namespace(self, eio):
         handler = mock.MagicMock()
         s.on('connect', handler, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.assert_called_once_with('123', '0/foo,{"sid":"1"}')
@@ -396,7 +396,7 @@ def test_handle_connect_namespace_rejected(self, eio):
         handler = mock.MagicMock(return_value=False)
         s.on('connect', handler, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         assert not s.manager.is_connected('1', '/foo')
@@ -422,7 +422,7 @@ def test_handle_connect_namespace_rejected_always_connect(self, eio):
         handler = mock.MagicMock(return_value=False)
         s.on('connect', handler, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert not s.manager.is_connected('1', '/foo')
         handler.assert_called_once_with('1', 'environ')
         s.eio.send.assert_any_call('123', '0/foo,{"sid":"1"}')
@@ -464,7 +464,7 @@ def test_handle_connect_namespace_rejected_with_exception(self, eio):
         )
         s.on('connect', handler, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert not s.manager.is_connected('1', '/foo')
         s.eio.send.assert_called_once_with(
             '123', '4/foo,{"message":"fail_reason","data":1}'
@@ -478,7 +478,7 @@ def test_handle_connect_namespace_rejected_with_empty_exception(self, eio):
         )
         s.on('connect', handler, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert not s.manager.is_connected('1', '/foo')
         s.eio.send.assert_called_once_with(
             '123', '4/foo,{"message":"Connection rejected by server"}')
@@ -503,7 +503,7 @@ def test_handle_disconnect_namespace(self, eio):
         handler_namespace = mock.MagicMock()
         s.on('disconnect', handler_namespace, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         s._handle_eio_disconnect('123')
         handler.assert_not_called()
         handler_namespace.assert_called_once_with('1')
@@ -516,8 +516,8 @@ def test_handle_disconnect_only_namespace(self, eio):
         handler_namespace = mock.MagicMock()
         s.on('disconnect', handler_namespace, namespace='/foo')
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
-        s._handle_eio_message('123', '1/foo')
+        s._handle_eio_message('123', '0/foo,')
+        s._handle_eio_message('123', '1/foo,')
         assert handler.call_count == 0
         handler_namespace.assert_called_once_with('1')
         assert s.environ == {'123': 'environ'}
@@ -651,7 +651,7 @@ def test_send_with_ack(self, eio):
     def test_send_with_ack_namespace(self, eio):
         s = server.Server()
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         cb = mock.MagicMock()
         id = s.manager._generate_ack_id('1', cb)
         s._emit_internal('123', 'my event', ['foo'], namespace='/foo', id=id)
@@ -711,9 +711,9 @@ def test_disconnect_ignore_queue(self, eio):
     def test_disconnect_namespace(self, eio):
         s = server.Server()
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         s.disconnect('1', namespace='/foo')
-        s.eio.send.assert_any_call('123', '1/foo')
+        s.eio.send.assert_any_call('123', '1/foo,')
 
     def test_disconnect_twice(self, eio):
         s = server.Server()
@@ -727,7 +727,7 @@ def test_disconnect_twice(self, eio):
     def test_disconnect_twice_namespace(self, eio):
         s = server.Server()
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         s.disconnect('123', namespace='/foo')
         calls = s.eio.send.call_count
         s.disconnect('123', namespace='/foo')
@@ -755,7 +755,7 @@ def on_baz(self, sid, data1, data2):
         s = server.Server(async_handlers=False)
         s.register_namespace(MyNamespace('/foo'))
         s._handle_eio_connect('123', 'environ')
-        s._handle_eio_message('123', '0/foo')
+        s._handle_eio_message('123', '0/foo,')
         assert result['result'] == ('1', 'environ')
         s._handle_eio_message('123', '2/foo,["foo","a"]')
         assert result['result'] == ('1', 'a')