Skip to content

Commit ecb2527

Browse files
committed
fix: remove JSON-RPC ID type coercion for spec-compliant strict matching
Remove the _normalize_request_id method that was introduced in PR #1720 to coerce string response IDs to integers. Per JSON-RPC 2.0 spec, the response ID MUST be the same as the value of the id member in the Request Object, which implies exact matching including type. The type coercion made request ID 1 and "1" interchangeable, which violates the spec. Servers that echo back IDs in a different type are non-compliant and should not be worked around in the SDK. Update tests to verify that type-mismatched IDs are correctly rejected rather than silently coerced. Github-Issue: #1795
1 parent 8f806da commit ecb2527

File tree

2 files changed

+40
-64
lines changed

2 files changed

+40
-64
lines changed

src/mcp/shared/session.py

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -456,26 +456,6 @@ async def _handle_session_message(message: SessionMessage) -> None:
456456
pass
457457
self._response_streams.clear()
458458

459-
def _normalize_request_id(self, response_id: RequestId) -> RequestId:
460-
"""Normalize a response ID to match how request IDs are stored.
461-
462-
Since the client always sends integer IDs, we normalize string IDs
463-
to integers when possible. This matches the TypeScript SDK approach:
464-
https://github.com/modelcontextprotocol/typescript-sdk/blob/a606fb17909ea454e83aab14c73f14ea45c04448/src/shared/protocol.ts#L861
465-
466-
Args:
467-
response_id: The response ID from the incoming message.
468-
469-
Returns:
470-
The normalized ID (int if possible, otherwise original value).
471-
"""
472-
if isinstance(response_id, str):
473-
try:
474-
return int(response_id)
475-
except ValueError:
476-
logging.warning(f"Response ID {response_id!r} cannot be normalized to match pending requests")
477-
return response_id
478-
479459
async def _handle_response(self, message: SessionMessage) -> None:
480460
"""Handle an incoming response or error message.
481461
@@ -495,8 +475,7 @@ async def _handle_response(self, message: SessionMessage) -> None:
495475
logging.warning(f"Received error with null ID: {error.message}")
496476
await self._handle_incoming(MCPError(error.code, error.message, error.data))
497477
return
498-
# Normalize response ID to handle type mismatches (e.g., "0" vs 0)
499-
response_id = self._normalize_request_id(message.message.id)
478+
response_id = message.message.id
500479

501480
# First, check response routers (e.g., TaskResultHandler)
502481
if isinstance(message.message, JSONRPCError):

tests/shared/test_session.py

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -99,44 +99,47 @@ async def make_request(client: Client):
9999

100100

101101
@pytest.mark.anyio
102-
async def test_response_id_type_mismatch_string_to_int():
103-
"""Test that responses with string IDs are correctly matched to requests sent with
104-
integer IDs.
102+
async def test_response_id_type_mismatch_string_to_int_rejected():
103+
"""Verify that a response with a string ID does not match a request sent with
104+
an integer ID.
105105
106-
This handles the case where a server returns "id": "0" (string) but the client
107-
sent "id": 0 (integer). Without ID type normalization, this would cause a timeout.
106+
Per JSON-RPC 2.0, the response ID "MUST be the same as the value of the id
107+
member in the Request Object". Since Python treats 0 != "0", a server that
108+
echoes back "0" instead of 0 is non-compliant and the request should time out.
108109
"""
109-
ev_response_received = anyio.Event()
110-
result_holder: list[types.EmptyResult] = []
110+
ev_timeout = anyio.Event()
111111

112112
async with create_client_server_memory_streams() as (client_streams, server_streams):
113113
client_read, client_write = client_streams
114114
server_read, server_write = server_streams
115115

116116
async def mock_server():
117-
"""Receive a request and respond with a string ID instead of integer."""
118117
message = await server_read.receive()
119118
assert isinstance(message, SessionMessage)
120119
root = message.message
121120
assert isinstance(root, JSONRPCRequest)
122-
# Get the original request ID (which is an integer)
123121
request_id = root.id
124-
assert isinstance(request_id, int), f"Expected int, got {type(request_id)}"
122+
assert isinstance(request_id, int)
125123

126-
# Respond with the ID as a string (simulating a buggy server)
124+
# Respond with the ID as a string (non-compliant server)
127125
response = JSONRPCResponse(
128126
jsonrpc="2.0",
129-
id=str(request_id), # Convert to string to simulate mismatch
127+
id=str(request_id),
130128
result={},
131129
)
132130
await server_write.send(SessionMessage(message=response))
133131

134132
async def make_request(client_session: ClientSession):
135-
nonlocal result_holder
136-
# Send a ping request (uses integer ID internally)
137-
result = await client_session.send_ping()
138-
result_holder.append(result)
139-
ev_response_received.set()
133+
try:
134+
await client_session.send_request(
135+
types.PingRequest(),
136+
types.EmptyResult,
137+
request_read_timeout_seconds=0.5,
138+
)
139+
pytest.fail("Expected timeout") # pragma: no cover
140+
except MCPError as e:
141+
assert "Timed out" in str(e)
142+
ev_timeout.set()
140143

141144
async with (
142145
anyio.create_task_group() as tg,
@@ -146,52 +149,49 @@ async def make_request(client_session: ClientSession):
146149
tg.start_soon(make_request, client_session)
147150

148151
with anyio.fail_after(2): # pragma: no branch
149-
await ev_response_received.wait()
150-
151-
assert len(result_holder) == 1
152-
assert isinstance(result_holder[0], EmptyResult)
152+
await ev_timeout.wait()
153153

154154

155155
@pytest.mark.anyio
156-
async def test_error_response_id_type_mismatch_string_to_int():
157-
"""Test that error responses with string IDs are correctly matched to requests
158-
sent with integer IDs.
156+
async def test_error_response_id_type_mismatch_string_to_int_rejected():
157+
"""Verify that an error response with a string ID does not match a request
158+
sent with an integer ID.
159159
160-
This handles the case where a server returns an error with "id": "0" (string)
161-
but the client sent "id": 0 (integer).
160+
The JSON-RPC spec requires exact ID matching including type.
162161
"""
163-
ev_error_received = anyio.Event()
164-
error_holder: list[MCPError | Exception] = []
162+
ev_timeout = anyio.Event()
165163

166164
async with create_client_server_memory_streams() as (client_streams, server_streams):
167165
client_read, client_write = client_streams
168166
server_read, server_write = server_streams
169167

170168
async def mock_server():
171-
"""Receive a request and respond with an error using a string ID."""
172169
message = await server_read.receive()
173170
assert isinstance(message, SessionMessage)
174171
root = message.message
175172
assert isinstance(root, JSONRPCRequest)
176173
request_id = root.id
177174
assert isinstance(request_id, int)
178175

179-
# Respond with an error, using the ID as a string
176+
# Respond with an error using the ID as a string (non-compliant)
180177
error_response = JSONRPCError(
181178
jsonrpc="2.0",
182-
id=str(request_id), # Convert to string to simulate mismatch
179+
id=str(request_id),
183180
error=ErrorData(code=-32600, message="Test error"),
184181
)
185182
await server_write.send(SessionMessage(message=error_response))
186183

187184
async def make_request(client_session: ClientSession):
188-
nonlocal error_holder
189185
try:
190-
await client_session.send_ping()
191-
pytest.fail("Expected MCPError to be raised") # pragma: no cover
186+
await client_session.send_request(
187+
types.PingRequest(),
188+
types.EmptyResult,
189+
request_read_timeout_seconds=0.5,
190+
)
191+
pytest.fail("Expected timeout") # pragma: no cover
192192
except MCPError as e:
193-
error_holder.append(e)
194-
ev_error_received.set()
193+
assert "Timed out" in str(e)
194+
ev_timeout.set()
195195

196196
async with (
197197
anyio.create_task_group() as tg,
@@ -201,16 +201,13 @@ async def make_request(client_session: ClientSession):
201201
tg.start_soon(make_request, client_session)
202202

203203
with anyio.fail_after(2): # pragma: no branch
204-
await ev_error_received.wait()
205-
206-
assert len(error_holder) == 1
207-
assert "Test error" in str(error_holder[0])
204+
await ev_timeout.wait()
208205

209206

210207
@pytest.mark.anyio
211208
async def test_response_id_non_numeric_string_no_match():
212-
"""Test that responses with non-numeric string IDs don't incorrectly match
213-
integer request IDs.
209+
"""Test that responses with non-numeric string IDs don't match integer
210+
request IDs.
214211
215212
If a server returns "id": "abc" (non-numeric string), it should not match
216213
a request sent with "id": 0 (integer).

0 commit comments

Comments
 (0)