Skip to content

Commit 6ccd6e1

Browse files
committed
Fix concurrent requests sharing a JSON-RPC id hijacking each other's responses
_handle_post_request unconditionally overwrote self._request_streams[request_id], so a second concurrent POST reusing an in-flight request's id would silently take over that request's stream and both callers would race for a single response. Reject the second request with a 409 duplicate-request-id error instead of overwriting the existing in-flight entry, for both the JSON and SSE response modes. Fixes #3137
1 parent 629ca29 commit 6ccd6e1

2 files changed

Lines changed: 179 additions & 1 deletion

File tree

src/mcp/server/streamable_http.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,13 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
550550
request_id = str(message.id)
551551

552552
if self.is_json_response_enabled:
553+
if request_id in self._request_streams:
554+
response = self._create_error_response(
555+
"Duplicate request id: a request with this id is already in flight on this session.",
556+
HTTPStatus.CONFLICT,
557+
)
558+
await response(scope, receive, send)
559+
return
553560
self._request_streams[request_id] = anyio.create_memory_object_stream[EventMessage](
554561
REQUEST_STREAM_BUFFER_SIZE
555562
)
@@ -570,7 +577,7 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
570577
response_message = event_message.message
571578
break
572579
# For notifications and requests, keep waiting
573-
else: # pragma: no cover
580+
else:
574581
logger.debug(f"received: {event_message.message.method}")
575582

576583
# At this point we should have a response
@@ -597,6 +604,14 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
597604
finally:
598605
await self._clean_up_memory_streams(request_id)
599606
else:
607+
if request_id in self._request_streams:
608+
response = self._create_error_response(
609+
"Duplicate request id: a request with this id is already in flight on this session.",
610+
HTTPStatus.CONFLICT,
611+
)
612+
await response(scope, receive, send)
613+
return
614+
600615
# Mint the priming event before any per-request state exists:
601616
# `EventStore.store_event` is user code and may raise, in which
602617
# case the outer handler returns a 500 with nothing to clean up.

tests/shared/test_streamable_http.py

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,169 @@ async def test_get_sse_stream(basic_app: Starlette) -> None:
809809
assert second_get.status_code == 409
810810

811811

812+
@pytest.mark.anyio
813+
async def test_post_duplicate_request_id_rejected_while_first_still_in_flight(basic_app: Starlette) -> None:
814+
"""A second POST reusing an in-flight request's JSON-RPC id is rejected; the first still completes."""
815+
async with make_client(basic_app) as client:
816+
init_response = await client.post(
817+
"/mcp",
818+
headers={
819+
"Accept": "application/json, text/event-stream",
820+
"Content-Type": "application/json",
821+
},
822+
json=INIT_REQUEST,
823+
)
824+
assert init_response.status_code == 200
825+
negotiated_version = extract_protocol_version_from_sse(init_response)
826+
session_id = init_response.headers.get(MCP_SESSION_ID_HEADER)
827+
assert session_id is not None
828+
829+
headers = {
830+
"Accept": "application/json, text/event-stream",
831+
"Content-Type": "application/json",
832+
MCP_SESSION_ID_HEADER: session_id,
833+
MCP_PROTOCOL_VERSION_HEADER: negotiated_version,
834+
}
835+
shared_id = "shared-request-id"
836+
837+
first_request_blocked = anyio.Event()
838+
first_response_lines: list[str] = []
839+
840+
async def run_first_request() -> None:
841+
async with client.stream(
842+
"POST",
843+
"/mcp",
844+
headers=headers,
845+
json={
846+
"jsonrpc": "2.0",
847+
"method": "tools/call",
848+
"params": {"name": "wait_for_lock_with_notification", "arguments": {}},
849+
"id": shared_id,
850+
},
851+
) as first_response:
852+
assert first_response.status_code == 200
853+
async for line in first_response.aiter_lines(): # pragma: no branch
854+
first_response_lines.append(line)
855+
if line.startswith("data: ") and "First notification before lock" in line:
856+
first_request_blocked.set()
857+
if line.startswith("data: ") and '"result"' in line:
858+
break
859+
860+
async with anyio.create_task_group() as tg:
861+
tg.start_soon(run_first_request)
862+
863+
with anyio.fail_after(5):
864+
await first_request_blocked.wait()
865+
866+
# The first request is still in flight (blocked on the lock), holding `shared_id`.
867+
duplicate_response = await client.post(
868+
"/mcp",
869+
headers=headers,
870+
json={
871+
"jsonrpc": "2.0",
872+
"method": "tools/call",
873+
"params": {"name": "test_tool", "arguments": {}},
874+
"id": shared_id,
875+
},
876+
)
877+
assert duplicate_response.status_code == 409
878+
assert "Duplicate request id" in duplicate_response.text
879+
880+
# Release the lock so the first request can finish.
881+
release_response = await client.post(
882+
"/mcp",
883+
headers=headers,
884+
json={
885+
"jsonrpc": "2.0",
886+
"method": "tools/call",
887+
"params": {"name": "release_lock", "arguments": {}},
888+
"id": "release-lock-1",
889+
},
890+
)
891+
assert release_response.status_code == 200
892+
893+
final_data_line = next(line for line in reversed(first_response_lines) if line.startswith("data: "))
894+
final_payload = json.loads(final_data_line.removeprefix("data: "))
895+
assert final_payload["id"] == shared_id
896+
assert final_payload["result"]["content"][0]["text"] == "Completed"
897+
898+
899+
@pytest.mark.anyio
900+
async def test_json_response_duplicate_request_id_rejected_while_first_still_in_flight(json_app: Starlette) -> None:
901+
"""In JSON response mode, a second POST reusing an in-flight id is rejected; the first still completes."""
902+
async with make_client(json_app) as client:
903+
init_response = await client.post(
904+
"/mcp",
905+
headers={
906+
"Accept": "application/json, text/event-stream",
907+
"Content-Type": "application/json",
908+
},
909+
json=INIT_REQUEST,
910+
)
911+
assert init_response.status_code == 200
912+
session_id = init_response.headers.get(MCP_SESSION_ID_HEADER)
913+
assert session_id is not None
914+
negotiated_version = init_response.json()["result"]["protocolVersion"]
915+
916+
headers = {
917+
"Accept": "application/json, text/event-stream",
918+
"Content-Type": "application/json",
919+
MCP_SESSION_ID_HEADER: session_id,
920+
MCP_PROTOCOL_VERSION_HEADER: negotiated_version,
921+
}
922+
shared_id = "shared-json-request-id"
923+
first_result: dict[str, Any] = {}
924+
925+
async def run_first_request() -> None:
926+
response = await client.post(
927+
"/mcp",
928+
headers=headers,
929+
json={
930+
"jsonrpc": "2.0",
931+
"method": "tools/call",
932+
"params": {"name": "wait_for_lock_with_notification", "arguments": {}},
933+
"id": shared_id,
934+
},
935+
)
936+
assert response.status_code == 200
937+
first_result.update(response.json())
938+
939+
async with anyio.create_task_group() as tg:
940+
tg.start_soon(run_first_request)
941+
942+
# The first request registers its request stream and blocks on the lock before
943+
# anything else in this task group can run.
944+
await anyio.wait_all_tasks_blocked()
945+
946+
duplicate_response = await client.post(
947+
"/mcp",
948+
headers=headers,
949+
json={
950+
"jsonrpc": "2.0",
951+
"method": "tools/call",
952+
"params": {"name": "test_tool", "arguments": {}},
953+
"id": shared_id,
954+
},
955+
)
956+
assert duplicate_response.status_code == 409
957+
assert "Duplicate request id" in duplicate_response.text
958+
959+
release_response = await client.post(
960+
"/mcp",
961+
headers=headers,
962+
json={
963+
"jsonrpc": "2.0",
964+
"method": "tools/call",
965+
"params": {"name": "release_lock", "arguments": {}},
966+
"id": "release-lock-json-1",
967+
},
968+
)
969+
assert release_response.status_code == 200
970+
971+
assert first_result["id"] == shared_id
972+
assert first_result["result"]["content"][0]["text"] == "Completed"
973+
974+
812975
@pytest.mark.anyio
813976
async def test_get_validation(basic_app: Starlette) -> None:
814977
"""A GET without an Accept header covering text/event-stream is rejected with 406."""

0 commit comments

Comments
 (0)