Skip to content

Commit 8804063

Browse files
committed
prevent non-canonical response ID collisions
1 parent 62575ed commit 8804063

2 files changed

Lines changed: 65 additions & 4 deletions

File tree

src/mcp/shared/session.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,10 @@ async def _receive_loop(self) -> None:
431431
def _normalize_request_id(self, response_id: RequestId) -> RequestId:
432432
"""Normalize a response ID to match how request IDs are stored.
433433
434-
Since the client always sends integer IDs, we normalize string IDs
435-
to integers when possible. This matches the TypeScript SDK approach:
436-
https://github.com/modelcontextprotocol/typescript-sdk/blob/a606fb17909ea454e83aab14c73f14ea45c04448/src/shared/protocol.ts#L861
434+
Since the client always sends integer IDs, we normalize canonical numeric
435+
string IDs (e.g. ``"0"``, ``"42"``) to integers. Non-canonical numeric
436+
strings (e.g. ``"01"``, ``"+1"``) are left as strings to avoid collisions
437+
with integer IDs.
437438
438439
Args:
439440
response_id: The response ID from the incoming message.
@@ -443,9 +444,18 @@ def _normalize_request_id(self, response_id: RequestId) -> RequestId:
443444
"""
444445
if isinstance(response_id, str):
445446
try:
446-
return int(response_id)
447+
int_id = int(response_id)
447448
except ValueError:
448449
logging.warning(f"Response ID {response_id!r} cannot be normalized to match pending requests")
450+
return response_id
451+
452+
if str(int_id) == response_id:
453+
return int_id
454+
455+
logging.warning(
456+
"Response ID %r is numeric but non-canonical; not normalizing to avoid ID collisions",
457+
response_id,
458+
)
449459
return response_id
450460

451461
async def _handle_response(self, message: SessionMessage) -> None:

tests/shared/test_session.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,57 @@ async def make_request(client_session: ClientSession):
258258
await ev_timeout.wait()
259259

260260

261+
@pytest.mark.anyio
262+
async def test_response_id_non_canonical_numeric_string_no_match():
263+
"""Test that non-canonical numeric IDs don't collide with integer request IDs.
264+
265+
If a server returns ``"id": "01"``, it should not match a pending request with
266+
integer ID ``1``.
267+
"""
268+
ev_timeout = anyio.Event()
269+
270+
async with create_client_server_memory_streams() as (client_streams, server_streams):
271+
client_read, client_write = client_streams
272+
server_read, server_write = server_streams
273+
274+
async def mock_server():
275+
"""Receive two requests and reply with a colliding non-canonical ID."""
276+
await server_read.receive() # request id 0
277+
await server_read.receive() # request id 1
278+
279+
response = JSONRPCResponse(
280+
jsonrpc="2.0",
281+
id="01", # Non-canonical representation of 1
282+
result={},
283+
)
284+
await server_write.send(SessionMessage(message=response))
285+
286+
async def make_requests(client_session: ClientSession):
287+
# First request consumes request ID 0 so the second request uses ID 1.
288+
await client_session.send_ping()
289+
290+
try:
291+
await client_session.send_request(
292+
types.PingRequest(),
293+
types.EmptyResult,
294+
request_read_timeout_seconds=0.5,
295+
)
296+
pytest.fail("Expected timeout") # pragma: no cover
297+
except MCPError as e:
298+
assert "Timed out" in str(e)
299+
ev_timeout.set()
300+
301+
async with (
302+
anyio.create_task_group() as tg,
303+
ClientSession(read_stream=client_read, write_stream=client_write) as client_session,
304+
):
305+
tg.start_soon(mock_server)
306+
tg.start_soon(make_requests, client_session)
307+
308+
with anyio.fail_after(2): # pragma: no branch
309+
await ev_timeout.wait()
310+
311+
261312
@pytest.mark.anyio
262313
async def test_connection_closed():
263314
"""Test that pending requests are cancelled when the connection is closed remotely."""

0 commit comments

Comments
 (0)