From 8d1aeb2e401713758a0a2576d0d1ea8eaf14896e Mon Sep 17 00:00:00 2001 From: Miguel Grinberg Date: Mon, 12 Apr 2021 23:48:37 +0100 Subject: [PATCH] Correct use of a trailing comma in Socket.IO packets with no id or data (Fixes #671) --- socketio/packet.py | 12 ++---------- tests/asyncio/test_asyncio_server.py | 26 +++++++++++++------------- tests/common/test_packet.py | 7 ++++--- tests/common/test_server.py | 26 +++++++++++++------------- 4 files changed, 32 insertions(+), 39 deletions(-) diff --git a/socketio/packet.py b/socketio/packet.py index 49f210e..280f5f7 100644 --- a/socketio/packet.py +++ b/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 @@ class Packet(object): 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 diff --git a/tests/asyncio/test_asyncio_server.py b/tests/asyncio/test_asyncio_server.py index a58f319..911cb3b 100644 --- a/tests/asyncio/test_asyncio_server.py +++ b/tests/asyncio/test_asyncio_server.py @@ -431,7 +431,7 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): ) 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 @@ class TestAsyncServer(unittest.TestCase): ) 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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 @@ class TestAsyncServer(unittest.TestCase): 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') diff --git a/tests/common/test_packet.py b/tests/common/test_packet.py index 0623817..ac080ba 100644 --- a/tests/common/test_packet.py +++ b/tests/common/test_packet.py @@ -111,12 +111,13 @@ class TestPacket(unittest.TestCase): 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( diff --git a/tests/common/test_server.py b/tests/common/test_server.py index e6be2ac..05eefff 100644 --- a/tests/common/test_server.py +++ b/tests/common/test_server.py @@ -359,7 +359,7 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): ) 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 @@ class TestServer(unittest.TestCase): ) 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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 @@ class TestServer(unittest.TestCase): 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')