Browse Source

Reject None as a room name in enter/leave/close_room

The manager stores the sid<->eio_sid mapping in a reserved room keyed by
None (self.rooms[namespace][None]). Passing None to the public room API —
e.g. leave_room(sid, None) when an app inadvertently does
leave_room(data.get('company_id')) with a missing key — removed the client
from that reserved room, corrupting the internal bookkeeping: is_connected()
started returning False and every subsequent room operation crashed with
KeyError: None, for a client that was still connected.

Reject None at the public entry points (Server/AsyncServer
enter_room/leave_room/close_room) with a clear ValueError, so an invalid
argument can no longer corrupt the manager state. The internal reserved-room
handling in connect()/basic_disconnect() goes through basic_* directly and
is unaffected.

Fixes #1447
pull/1589/head
uttam12331 6 days ago
parent
commit
7175fd0cc9
  1. 6
      src/socketio/async_server.py
  2. 6
      src/socketio/server.py
  3. 16
      tests/async/test_server.py
  4. 16
      tests/common/test_server.py

6
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)

6
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)

16
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)

16
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)

Loading…
Cancel
Save