Validate connection data integrity in ConnectionManager
Fix audit warnings about empty string defaults hiding data corruption: 1. get_connection_info(): Validate required fields (user_id, connected_at, last_seen) exist before creating ConnectionInfo. Return None and log warning for corrupted records instead of returning invalid data. 2. unregister_connection(): Log warning if user_id is missing during cleanup. Existing code safely handles this (skips cleanup), but now we have visibility into data corruption. Test added for corrupted data case. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
1346853869
commit
0a9cd73daf
@ -230,6 +230,10 @@ class ConnectionManager:
|
||||
user_id = data.get("user_id", "")
|
||||
game_id = data.get("game_id", "")
|
||||
|
||||
# Warn if user_id is missing - indicates corrupted data
|
||||
if not user_id:
|
||||
logger.warning(f"Connection {sid} has no user_id - data may be corrupted")
|
||||
|
||||
# Remove from game connection set if in a game
|
||||
if game_id:
|
||||
game_conns_key = self._game_conns_key(game_id)
|
||||
@ -392,12 +396,25 @@ class ConnectionManager:
|
||||
if not data:
|
||||
return None
|
||||
|
||||
# Validate required fields - if missing, data is corrupted
|
||||
user_id = data.get("user_id")
|
||||
connected_at = data.get("connected_at")
|
||||
last_seen = data.get("last_seen")
|
||||
|
||||
if not user_id or not connected_at or not last_seen:
|
||||
logger.warning(
|
||||
f"Corrupted connection record for {sid}: "
|
||||
f"missing required fields (user_id={bool(user_id)}, "
|
||||
f"connected_at={bool(connected_at)}, last_seen={bool(last_seen)})"
|
||||
)
|
||||
return None
|
||||
|
||||
return ConnectionInfo(
|
||||
sid=sid,
|
||||
user_id=data.get("user_id", ""),
|
||||
user_id=user_id,
|
||||
game_id=data.get("game_id") or None,
|
||||
connected_at=datetime.fromisoformat(data["connected_at"]),
|
||||
last_seen=datetime.fromisoformat(data["last_seen"]),
|
||||
connected_at=datetime.fromisoformat(connected_at),
|
||||
last_seen=datetime.fromisoformat(last_seen),
|
||||
)
|
||||
|
||||
async def get_user_connection(self, user_id: str | UUID) -> ConnectionInfo | None:
|
||||
|
||||
@ -462,6 +462,30 @@ class TestQueryMethods:
|
||||
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_connection_returns_none_for_corrupted_data(
|
||||
self,
|
||||
manager: ConnectionManager,
|
||||
mock_redis: AsyncMock,
|
||||
) -> None:
|
||||
"""Test that get_connection returns None for corrupted records.
|
||||
|
||||
If a connection record is missing required fields (user_id,
|
||||
connected_at, last_seen), it indicates data corruption. The
|
||||
method should return None and log a warning rather than
|
||||
returning a ConnectionInfo with invalid/empty data.
|
||||
"""
|
||||
# Record missing user_id
|
||||
mock_redis.hgetall.return_value = {
|
||||
"game_id": "game-123",
|
||||
"connected_at": "2024-01-01T00:00:00+00:00",
|
||||
"last_seen": "2024-01-01T00:00:00+00:00",
|
||||
}
|
||||
|
||||
result = await manager.get_connection("corrupted-sid")
|
||||
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_is_user_online_returns_true_for_connected_user(
|
||||
self,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user