diff --git a/src/socketio/async_server.py b/src/socketio/async_server.py index 7e9c872..65dcb38 100644 --- a/src/socketio/async_server.py +++ b/src/socketio/async_server.py @@ -301,6 +301,8 @@ class AsyncServer(base_server.BaseServer): Note: this method is a coroutine. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('%s is entering room %s [%s]', sid, room, namespace) await self.manager.enter_room(sid, namespace, room) @@ -317,6 +319,8 @@ class AsyncServer(base_server.BaseServer): Note: this method is a coroutine. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('%s is leaving room %s [%s]', sid, room, namespace) await self.manager.leave_room(sid, namespace, room) @@ -332,6 +336,8 @@ class AsyncServer(base_server.BaseServer): Note: this method is a coroutine. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('room %s is closing [%s]', room, namespace) await self.manager.close_room(room, namespace) diff --git a/src/socketio/server.py b/src/socketio/server.py index 143d27e..deebd08 100644 --- a/src/socketio/server.py +++ b/src/socketio/server.py @@ -281,6 +281,8 @@ class Server(base_server.BaseServer): :param namespace: The Socket.IO namespace for the event. If this argument is omitted the default namespace is used. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('%s is entering room %s [%s]', sid, room, namespace) self.manager.enter_room(sid, namespace, room) @@ -295,6 +297,8 @@ class Server(base_server.BaseServer): :param namespace: The Socket.IO namespace for the event. If this argument is omitted the default namespace is used. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('%s is leaving room %s [%s]', sid, room, namespace) self.manager.leave_room(sid, namespace, room) @@ -308,6 +312,8 @@ class Server(base_server.BaseServer): :param namespace: The Socket.IO namespace for the event. If this argument is omitted the default namespace is used. """ + if room is None: + raise ValueError('The room name cannot be None') namespace = namespace or '/' self.logger.info('room %s is closing [%s]', room, namespace) self.manager.close_room(room, namespace) diff --git a/tests/async/test_server.py b/tests/async/test_server.py index c3c0903..23f9904 100644 --- a/tests/async/test_server.py +++ b/tests/async/test_server.py @@ -252,6 +252,22 @@ class TestAsyncServer: await s.close_room('room') s.manager.close_room.assert_awaited_once_with('room', '/') + async def test_enter_leave_close_room_none_rejected(self, eio): + # None is the reserved default room that stores the sid<->eio_sid + # mapping; the public room API must reject it instead of corrupting + # the manager's internal data structures (issue #1447). + mgr = self._get_mock_manager() + s = async_server.AsyncServer(client_manager=mgr) + with pytest.raises(ValueError): + await s.enter_room('123', None, namespace='/foo') + with pytest.raises(ValueError): + await s.leave_room('123', None, namespace='/foo') + with pytest.raises(ValueError): + await s.close_room(None, namespace='/foo') + s.manager.enter_room.assert_not_awaited() + s.manager.leave_room.assert_not_awaited() + s.manager.close_room.assert_not_awaited() + async def test_rooms(self, eio): mgr = self._get_mock_manager() s = async_server.AsyncServer(client_manager=mgr) diff --git a/tests/common/test_server.py b/tests/common/test_server.py index d8d80dc..2ebfd3f 100644 --- a/tests/common/test_server.py +++ b/tests/common/test_server.py @@ -254,6 +254,22 @@ class TestServer: s.close_room('room') s.manager.close_room.assert_called_once_with('room', '/') + def test_enter_leave_close_room_none_rejected(self, eio): + # None is the reserved default room that stores the sid<->eio_sid + # mapping; the public room API must reject it instead of corrupting + # the manager's internal data structures (issue #1447). + mgr = mock.MagicMock() + s = server.Server(client_manager=mgr) + with pytest.raises(ValueError): + s.enter_room('123', None, namespace='/foo') + with pytest.raises(ValueError): + s.leave_room('123', None, namespace='/foo') + with pytest.raises(ValueError): + s.close_room(None, namespace='/foo') + s.manager.enter_room.assert_not_called() + s.manager.leave_room.assert_not_called() + s.manager.close_room.assert_not_called() + def test_rooms(self, eio): mgr = mock.MagicMock() s = server.Server(client_manager=mgr)