modelcontextprotocol / modelcontextprotocol/python-sdk

Dispatcher mints a request id already used by a completed caller-supplied request (spec: ids MUST NOT be reused in a session)

未關閉
#3,126 2 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視

還沒有人認領這個 Issue。

bug needs confirmation P2 v2
主要語言
Python
星號
24.3k
分支
4k
平均合併
1 天 1 小時
30 天內合併 PR
31

描述

Initial Checks
Description

On current main (v2, tested at 3a6f2996), when a caller supplies its own request id via CallOptions["request_id"] (added in #3046), the dispatcher's minted-id sequence can later land on that same id after the supplied request completes. The session then sends two different requests with the same id, which the spec forbids:

The request ID MUST NOT have been previously used by the requestor within the same session.

(https://modelcontextprotocol.io/specification/2025-11-25/basic#requests, same wording since 2025-03-26. The SDK pins this itself as protocol:request-id:unique in tests/interaction/_requirements.py: "ids are never reused within the session".)

Root cause

In JSONRPCDispatcher.send_raw_request (src/mcp/shared/jsonrpc_dispatcher.py, around line 338):

else:
    # Mint past any key a supplied id occupies: the collision error is
    # reserved for the caller who actually chose the id.
    request_id = self._allocate_id()
    while request_id in self._pending:
        request_id = self._allocate_id()

The comment says minting should skip past any supplied id, but the guard only checks self._pending, and pending entries are popped when a request completes (line ~428). So for a supplied integer id (or numeric string, since coerce_request_id folds "2" and 2 into one key): once that request finishes, the monotonic counter eventually reaches the same value and reuses it on the wire.

DirectDispatcher._dispatch_request (src/mcp/shared/direct_dispatcher.py, around line 253) has the same pattern with self._in_flight_ids, which is discarded in the finally.

Why it matters

The receiving side is allowed to assume per-session id uniqueness. The SDK's own stateful Streamable HTTP server already mishandles duplicate ids (#3060, response cross-wiring), so an innocent client that supplies small integer ids for a few calls and then keeps using the same session can hit that server-side behavior through no fault of its own. The SDK-internal user of this feature (the subscriptions/listen driver's "listen-N" ids) is immune because non-numeric string ids never collide with the minted integer sequence, so this only bites users of the new public CallOptions["request_id"] with integer or numeric-string ids.

Note this is only about the dispatcher's own minting crossing a completed supplied id. The reverse direction (a caller re-supplying an id it already used before) is currently allowed on purpose and asserted by tests, so I left that alone.

Proposed fix

When accepting a supplied id, advance the mint counter past its coerced key so minted ids can never revisit it:

# jsonrpc_dispatcher.py, after computing pending_key for a supplied id:
if isinstance(pending_key, int):
    self._next_id = max(self._next_id, pending_key)

and the same in direct_dispatcher.py with in_flight_key. Happy to open a PR with regression tests for both dispatchers if this looks right.

Example Code
import anyio
from mcp_types import JSONRPCRequest, JSONRPCResponse

from mcp.shared.jsonrpc_dispatcher import JSONRPCDispatcher
from mcp.shared.message import SessionMessage


async def main() -> None:
    c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
    s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
    client = JSONRPCDispatcher(s2c_recv, c2s_send)

    async def on_request(ctx, method, params):
        return {}

    async def on_notify(ctx, method, params):
        pass

    wire_ids = []

    async def respond() -> None:
        async for wire in c2s_recv:
            if isinstance(wire, SessionMessage) and isinstance(wire.message, JSONRPCRequest):
                wire_ids.append(wire.message.id)
                await s2c_send.send(
                    SessionMessage(JSONRPCResponse(jsonrpc="2.0", id=wire.message.id, result={}))
                )

    async with anyio.create_task_group() as tg:
        await tg.start(client.run, on_request, on_notify)
        tg.start_soon(respond)

        await client.send_raw_request("ping", None, {"request_id": 2})
        await client.send_raw_request("ping", None)
        await client.send_raw_request("ping", None)

        tg.cancel_scope.cancel()

    print("wire request ids in order:", wire_ids)
    # prints: wire request ids in order: [2, 1, 2]
    # the third request (dispatcher-minted) reuses the completed supplied id 2


anyio.run(main)
Python & MCP Python SDK
python: 3.14.5
mcp: main @ 3a6f2996 (v2 development line)

AI disclosure, since it's policy here: I leaned on Claude Code to help navigate the codebase and pressure-test this analysis, but I ran and verified the repro myself and I understand the fix I'm proposing. Freshman in college trying to contribute something genuinely useful, so if I've misjudged the intent behind the mint loop I'd honestly love the correction :)

貢獻指南

開啟貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

研究方向

從 src/mcp/shared/jsonrpc_dispatcher.py 中的 JSONRPCDispatcher.send_raw_request 和 src/mcp/shared/direct_dispatcher.py 中的 DirectDispatcher._dispatch_request 開始,然後檢查 tests/interaction/_requirements.py 中對 request-id 的要求。重現提供的 ID 序列,並為兩個 dispatcher 新增回歸覆蓋;完成標準是,產生的 wire ID 在一個工作階段中絕不重複使用呼叫方提供且已完成的整數 ID。

由索引模型根據 Issue 內容生成。

評估

技術堆疊
python
領域
api, testing
Issue 類型
缺陷
難度
3/5
預估耗時
1-2 天
活躍度
活躍
描述清晰度
描述清楚
新手友好度
86/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。