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 摘要。