diff --git a/server/app/client.py b/server/app/client.py index 5ebb60e..2236f8a 100644 --- a/server/app/client.py +++ b/server/app/client.py @@ -17,6 +17,8 @@ class ClientConnection: x: int = 20 y: int = 20 last_position_update_ms: int = 0 + movement_window_index: int = -1 + movement_window_steps_used: int = 0 def summary(self) -> dict[str, str | int]: """Return a compact serializable snapshot for logs/diagnostics.""" diff --git a/server/app/server.py b/server/app/server.py index 5450510..98bfa5e 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -576,12 +576,23 @@ class SignalingServer: return 0 <= x < self.grid_size and 0 <= y < self.grid_size - def _max_allowed_position_delta(self, client: ClientConnection, now_ms: int) -> int: - """Compute max allowed movement delta using server-authoritative rate policy.""" + def _movement_window_index(self, now_ms: int) -> int: + """Return current movement rate-limit window index for a server timestamp.""" - elapsed_ms = max(0, now_ms - max(0, client.last_position_update_ms)) - windows = max(1, elapsed_ms // self.movement_tick_ms) - return max(1, windows * self.movement_max_steps_per_tick) + return max(0, now_ms // self.movement_tick_ms) + + def _consume_movement_budget(self, client: ClientConnection, now_ms: int, requested_delta: int) -> bool: + """Consume per-window movement budget; return whether the move is allowed.""" + + window_index = self._movement_window_index(now_ms) + if client.movement_window_index != window_index: + client.movement_window_index = window_index + client.movement_window_steps_used = 0 + remaining = max(0, self.movement_max_steps_per_tick - client.movement_window_steps_used) + if requested_delta > remaining: + return False + client.movement_window_steps_used += requested_delta + return True @staticmethod def _normalize_clock_timezone(value: object) -> str: @@ -699,7 +710,10 @@ class SignalingServer: client = ClientConnection(websocket=websocket, id=str(uuid.uuid4())) client.x = random.randrange(self.grid_size) client.y = random.randrange(self.grid_size) - client.last_position_update_ms = self.item_service.now_ms() + now_ms = self.item_service.now_ms() + client.last_position_update_ms = now_ms + client.movement_window_index = self._movement_window_index(now_ms) + client.movement_window_steps_used = 0 self.clients[websocket] = client LOGGER.info("client connected id=%s total=%d", client.id, len(self.clients)) @@ -827,17 +841,18 @@ class SignalingServer: return now_ms = self.item_service.now_ms() requested_delta = max(abs(packet.x - client.x), abs(packet.y - client.y)) - max_delta = self._max_allowed_position_delta(client, now_ms) - if requested_delta > max_delta: + if not self._consume_movement_budget(client, now_ms, requested_delta): + remaining = max(0, self.movement_max_steps_per_tick - client.movement_window_steps_used) PACKET_LOGGER.warning( - "position rate limit ignored id=%s from=%d,%d to=%d,%d requested_delta=%d max_delta=%d", + "position rate limit ignored id=%s from=%d,%d to=%d,%d requested_delta=%d remaining_budget=%d window=%d", client.id, client.x, client.y, packet.x, packet.y, requested_delta, - max_delta, + remaining, + client.movement_window_index, ) return client.x = packet.x diff --git a/server/tests/test_server_message_handling.py b/server/tests/test_server_message_handling.py index 453843e..1c99c3a 100644 --- a/server/tests/test_server_message_handling.py +++ b/server/tests/test_server_message_handling.py @@ -104,3 +104,32 @@ async def test_item_add_rejects_unknown_type(monkeypatch: pytest.MonkeyPatch) -> assert send_payloads assert send_payloads[-1].ok is False assert "unknown item type" in send_payloads[-1].message.lower() + + +@pytest.mark.asyncio +async def test_update_position_enforces_cumulative_budget_per_tick(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None, grid_size=41, movement_tick_ms=100, movement_max_steps_per_tick=2) + ws = _fake_ws() + client = ClientConnection(websocket=ws, id="u1", nickname="tester", x=5, y=5) + server.clients[ws] = client + + fixed_now = 10_000 + monkeypatch.setattr(server.item_service, "now_ms", lambda: fixed_now) + + broadcast_payloads: list[object] = [] + + async def fake_broadcast(packet: object, exclude: ServerConnection | None = None) -> None: + broadcast_payloads.append(packet) + + monkeypatch.setattr(server, "_broadcast", fake_broadcast) + + # First 1-step move in this tick: allowed. + await server._handle_message(client, json.dumps({"type": "update_position", "x": 6, "y": 5})) + # Second 1-step move in the same tick: allowed (budget now exhausted at 2). + await server._handle_message(client, json.dumps({"type": "update_position", "x": 7, "y": 5})) + # Third 1-step move in the same tick: must be rejected. + await server._handle_message(client, json.dumps({"type": "update_position", "x": 8, "y": 5})) + + assert client.x == 7 + assert client.y == 5 + assert len(broadcast_payloads) == 2