fix: return 405 for pre-session GET the server won't serve as SSE#3129
fix: return 405 for pre-session GET the server won't serve as SSE#3129dosvk wants to merge 4 commits into
Conversation
The Streamable HTTP spec requires a GET the server does not serve as an SSE stream to get 405 Method Not Allowed, but in stateful mode a pre-session GET returned 400 (missing session ID) instead. Only-405 is what client transports (e.g. the TypeScript SDK's SSE probe) treat as the graceful fall-through to POST, so stock servers aborted those handshakes before initialize. Return 405 with Allow: GET, POST, DELETE for session-less GETs in stateful mode, before Accept validation, matching the Allow value of _handle_unsupported_request. (The 406-for-wildcard-Accept arm of the report is already fixed on main via check_accept_headers.) Closes modelcontextprotocol#3102
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/server/streamable_http.py">
<violation number="1" location="src/mcp/server/streamable_http.py:670">
P1: A pre-session GET now leaves an unused stateful session behind. The manager creates and registers a transport before this handler can return 405, and the client's fallback initialize creates another one; with the default no idle timeout, each probe retains a server task and session entry indefinitely. Consider rejecting session-less GETs in the manager before provisioning a transport (or explicitly removing and closing the provisional transport).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # Per MCP spec, pre-session GETs that cannot be served as SSE must return | ||
| # 405 Method Not Allowed. A GET without a session ID in stateful mode | ||
| # cannot establish an SSE stream because no session exists yet. | ||
| if self.mcp_session_id and not self._get_session_id(request): |
There was a problem hiding this comment.
P1: A pre-session GET now leaves an unused stateful session behind. The manager creates and registers a transport before this handler can return 405, and the client's fallback initialize creates another one; with the default no idle timeout, each probe retains a server task and session entry indefinitely. Consider rejecting session-less GETs in the manager before provisioning a transport (or explicitly removing and closing the provisional transport).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/streamable_http.py, line 670:
<comment>A pre-session GET now leaves an unused stateful session behind. The manager creates and registers a transport before this handler can return 405, and the client's fallback initialize creates another one; with the default no idle timeout, each probe retains a server task and session entry indefinitely. Consider rejecting session-less GETs in the manager before provisioning a transport (or explicitly removing and closing the provisional transport).</comment>
<file context>
@@ -656,11 +656,29 @@ async def _handle_get_request(self, request: Request, send: Send) -> None:
+ # Per MCP spec, pre-session GETs that cannot be served as SSE must return
+ # 405 Method Not Allowed. A GET without a session ID in stateful mode
+ # cannot establish an SSE stream because no session exists yet.
+ if self.mcp_session_id and not self._get_session_id(request):
+ headers = {
+ "Allow": "GET, POST, DELETE",
</file context>
…s created A pre-session GET (one without a session ID header in stateful mode) was being handled AFTER a transport and session were already registered. The transport's _handle_get_request correctly returned 405, but by then an unused session existed indefinitely (with default no idle timeout). Restructured to reject pre-session GETs at the manager layer BEFORE any transport creation: 1. Manager now checks for GET without session ID first 2. Security validation (DNS rebinding protection) runs before the 405 so malformed/attack requests get their proper 421 response 3. Only after security passes does it return 405 Method Not Allowed The transport-level check remains as defensive code for standalone transport use (the class is public). Tests now cover both paths: - Manager path: test_pre_session_get_rejected_without_creating_transport - Standalone path: test_standalone_transport_pre_session_get_returns_405 Also added test_standalone_transport_get_with_wrong_session_returns_404 to cover the session ID mismatch validation (removed pragma: no cover). Addresses review finding: modelcontextprotocol#3129
|
Good catch on the session leak — fixed in the latest commits. The 405 rejection now happens in The transport-level check remains as a defensive path for standalone (manager-less) transport use, now with its own tests instead of a |
The 405 rejection paths return before the request body is read, so the stub receive callables never execute. Matches the existing convention for unreachable test helpers in this suite.
Closes #3102
Summary
Per the Streamable HTTP transport spec (Listening for Messages from the Server):
In stateful mode, a pre-session GET (no
mcp-session-idheader) returned 400 Bad Request: Missing session ID instead. Since client transports that probe for a standalone SSE stream beforeinitializetreat only 405 as the graceful "no SSE — fall back to POST" signal (e.g. the TypeScript SDK's_startOrAuthSse), any other status aborts the handshake — the interop break described in #3102 (credit to @fgranata for the thorough root-cause analysis).The 406-for-wildcard-Accept arm of the report is already fixed on main via
check_accept_headers; this PR addresses the remaining 400 path.Changes
_handle_get_request: a session-less GET in stateful mode now returns 405 withAllow: GET, POST, DELETE(matching_handle_unsupported_request), before Accept validation — the spec's requirement applies to any GET the server won't serve as SSE, regardless of Accept header.Accept: */*,application/json, absent,text/event-stream), asserting status,Allowheader, and JSON-RPC error code.Backward compatibility
Post-session GETs (valid
mcp-session-id+ SSE Accept) are unchanged and still serve SSE. Stateless mode is unaffected. POST/DELETE session validation is unchanged.