Skip to content

Commit cf0fc1b

Browse files
committed
fix(streamable-http): reserve routing slot before event store priming
In resumable SSE mode the priming event is minted by awaiting EventStore.store_event between the duplicate-id guard and the routing slot registration, so a concurrent POST reusing the id could pass the guard during that await and overwrite the slot. Reserve the slot synchronously with the guard and mint the priming event afterwards, releasing the reservation if the event store raises so the outer 500 path leaves nothing in flight. Adds a regression test that suspends a gated event store mid-priming and asserts the duplicate POST is still rejected.
1 parent 9a2b35e commit cf0fc1b

2 files changed

Lines changed: 109 additions & 8 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -611,19 +611,28 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
611611
finally:
612612
await self._clean_up_memory_streams(request_id)
613613
else:
614-
# Mint the priming event before any per-request state exists:
615-
# `EventStore.store_event` is user code and may raise, in which
616-
# case the outer handler returns a 500 with nothing to clean up.
617-
# Still strictly precedes dispatch, so storage order == wire order.
618-
priming_event = await self._mint_priming_event(request_id, protocol_version)
619-
620-
sse_stream_writer, sse_stream_reader = anyio.create_memory_object_stream[SSEEvent](0)
621-
self._sse_stream_writers[request_id] = sse_stream_writer
614+
# Reserve the routing slot before any await so nothing separates
615+
# the duplicate-id guard above from registration; a concurrent
616+
# POST reusing this id while the event store persists the priming
617+
# event could otherwise pass the guard and overwrite the slot.
622618
self._request_streams[request_id] = anyio.create_memory_object_stream[EventMessage](
623619
REQUEST_STREAM_BUFFER_SIZE
624620
)
625621
request_stream_reader = self._request_streams[request_id][1]
626622

623+
# Mint the priming event before dispatch so storage order matches
624+
# wire order. `EventStore.store_event` is user code and may raise,
625+
# in which case the outer handler returns a 500; release the slot
626+
# reservation on the way out so the id does not stay in flight.
627+
try:
628+
priming_event = await self._mint_priming_event(request_id, protocol_version)
629+
except BaseException:
630+
await self._clean_up_memory_streams(request_id)
631+
raise
632+
633+
sse_stream_writer, sse_stream_reader = anyio.create_memory_object_stream[SSEEvent](0)
634+
self._sse_stream_writers[request_id] = sse_stream_writer
635+
627636
headers = {
628637
"Cache-Control": "no-cache, no-transform",
629638
"Connection": "keep-alive",

tests/shared/test_streamable_http.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,98 @@ async def test_request_id_reuse_after_completion_allowed(basic_app: Starlette) -
812812
assert body["result"]["content"][0]["text"] == "Called test_tool"
813813

814814

815+
@pytest.mark.anyio
816+
async def test_duplicate_in_flight_request_id_rejected_during_priming() -> None:
817+
"""The duplicate-id guard holds while the event store persists the priming event.
818+
819+
With an event store configured, the SSE branch awaits ``EventStore.store_event``
820+
to mint the priming event. The routing slot is reserved before that await, so a
821+
concurrent POST reusing the id during persistence is still rejected rather than
822+
slipping past the guard and overwriting the slot (see #3060).
823+
"""
824+
825+
class GatedEventStore(SimpleEventStore):
826+
"""Blocks the priming write for the gated stream until released."""
827+
828+
def __init__(self) -> None:
829+
super().__init__()
830+
self.entered = anyio.Event()
831+
self.release = anyio.Event()
832+
833+
async def store_event(self, stream_id: StreamId, message: types.JSONRPCMessage | None) -> EventId:
834+
if stream_id == "gated-1" and message is None:
835+
self.entered.set()
836+
await self.release.wait()
837+
return await super().store_event(stream_id, message)
838+
839+
def first_message_sse_data(response: httpx.Response) -> dict[str, Any]:
840+
"""First non-empty SSE data payload; skips the empty-data priming event."""
841+
for line in response.text.splitlines():
842+
if line.startswith("data: ") and line.removeprefix("data: ").strip():
843+
return json.loads(line.removeprefix("data: "))
844+
raise ValueError("No message data event in SSE response")
845+
846+
store = GatedEventStore()
847+
async with running_app(event_store=store) as app:
848+
async with make_client(app) as client:
849+
# Priming events are only minted for protocol >= 2025-11-25, so
850+
# negotiate the latest version rather than INIT_REQUEST's pinned one.
851+
init_request = {
852+
**INIT_REQUEST,
853+
"params": {**INIT_REQUEST["params"], "protocolVersion": types.LATEST_PROTOCOL_VERSION},
854+
}
855+
response = await client.post(
856+
"/mcp",
857+
headers={
858+
"Accept": "application/json, text/event-stream",
859+
"Content-Type": "application/json",
860+
},
861+
json=init_request,
862+
)
863+
assert response.status_code == 200
864+
headers = {
865+
"Accept": "application/json, text/event-stream",
866+
"Content-Type": "application/json",
867+
MCP_SESSION_ID_HEADER: response.headers[MCP_SESSION_ID_HEADER],
868+
MCP_PROTOCOL_VERSION_HEADER: first_message_sse_data(response)["result"]["protocolVersion"],
869+
}
870+
call = {
871+
"jsonrpc": "2.0",
872+
"id": "gated-1",
873+
"method": "tools/call",
874+
"params": {"name": "test_tool", "arguments": {}},
875+
}
876+
results: dict[str, httpx.Response] = {}
877+
878+
async def post_first() -> None:
879+
results["first"] = await client.post("/mcp", headers=headers, json=call)
880+
881+
async with anyio.create_task_group() as tg:
882+
tg.start_soon(post_first)
883+
# The first request is now suspended inside the event store's
884+
# priming write: past the duplicate-id guard, response not started.
885+
with anyio.fail_after(5):
886+
await store.entered.wait()
887+
888+
# Bounded so a regression fails fast: without the early slot
889+
# reservation, this POST would also suspend in the gated event
890+
# store and the test would hang instead of failing.
891+
with anyio.fail_after(5):
892+
duplicate = await client.post("/mcp", headers=headers, json=call)
893+
assert duplicate.status_code == 400
894+
error = duplicate.json()["error"]
895+
assert error["code"] == INVALID_REQUEST
896+
assert "already in flight" in error["message"]
897+
898+
store.release.set()
899+
900+
# The first request is unaffected and completes with its own result.
901+
assert results["first"].status_code == 200
902+
body = first_message_sse_data(results["first"])
903+
assert body["id"] == "gated-1"
904+
assert body["result"]["content"][0]["text"] == "Called test_tool"
905+
906+
815907
@pytest.mark.anyio
816908
async def test_json_response(json_app: Starlette) -> None:
817909
"""With JSON response mode enabled, requests are answered with application/json bodies."""

0 commit comments

Comments
 (0)