Skip to content

Commit 0f12dae

Browse files
committed
fix(streamable-http): reject GET/DELETE without session-id before allocating
Before this patch, any request to the stateful streamable_http_manager without an MCP-Session-Id header entered the "new session" branch and: 1. allocated a StreamableHTTPServerTransport 2. registered it in `_server_instances` 3. spawned a background `run_server` task waiting on `serve_loop` / `app.run()` for messages that never come The subsequent transport-level rejection (400 "Missing session ID" for GET/DELETE, 406 "Not Acceptable" for GET with missing Accept header) returned the correct HTTP response but did not tear the allocated session or its task down. The `finally` cleanup in the background task only fires when the loop completes, and without the (opt-in, off-by-default) `session_idle_timeout` the task blocks forever. Under a real deployment we hit this via a Docker healthcheck polling `GET /mcp` every 30s on a FastMCP-based server: ~2 sessions/min leaked, ~5.3 MiB/day RAM growth, 28 800 accumulated sessions with 0 teardown events over 10 days. The fix reorders `_handle_stateful_request` so that: * security validation runs first (preserves the 421 DNS-rebinding behaviour tested in test_streamable_http_security_get_request), * GET and DELETE with no session-id return 400 "Missing session ID" at the manager layer without touching `_server_instances` and without spawning any task, * POST without session-id continues to initialize a new session exactly as before, * PUT/PATCH/OPTIONS with no session-id continue to reach the transport and get the existing 405 "Method Not Allowed". A regression test asserts both counters (`_server_instances`, `_task_group._tasks`) stay at zero after 300 bad requests.
1 parent 0e3a604 commit 0f12dae

2 files changed

Lines changed: 121 additions & 1 deletion

File tree

src/mcp/server/streamable_http_manager.py

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
EventStore,
2222
StreamableHTTPServerTransport,
2323
)
24-
from mcp.server.transport_security import TransportSecuritySettings
24+
from mcp.server.transport_security import TransportSecurityMiddleware, TransportSecuritySettings
2525
from mcp.types import INVALID_REQUEST, ErrorData, JSONRPCError
2626

2727
logger = logging.getLogger(__name__)
@@ -85,6 +85,15 @@ def __init__(
8585
self.retry_interval = retry_interval
8686
self.session_idle_timeout = session_idle_timeout
8787

88+
# Pre-check middleware: the manager runs security validation *before*
89+
# deciding whether to allocate a session. Without this, a request that
90+
# fails security (DNS rebinding, bad Host) would either be rejected too
91+
# late (after a session has already been created and would leak) or with
92+
# the wrong status code (400 "Missing session ID" instead of 421
93+
# "Invalid Host header"). Using the same middleware the transport uses
94+
# keeps the two validation paths consistent.
95+
self._security = TransportSecurityMiddleware(security_settings)
96+
8897
# Session tracking (only used if not stateless)
8998
self._session_creation_lock = anyio.Lock()
9099
self._server_instances: dict[str, StreamableHTTPServerTransport] = {}
@@ -229,6 +238,16 @@ async def _handle_stateful_request(
229238
send: ASGI send function
230239
"""
231240
request = Request(scope, receive)
241+
242+
# Run security validation FIRST so DNS-rebinding / bad-Host requests
243+
# are rejected with 421 (or the transport-level Content-Type 400)
244+
# regardless of whether a session existed or not — and so bad-Host
245+
# requests can never trigger a session allocation.
246+
security_error = await self._security.validate_request(request, is_post=(request.method == "POST"))
247+
if security_error is not None:
248+
await security_error(scope, receive, send)
249+
return
250+
232251
request_mcp_session_id = request.headers.get(MCP_SESSION_ID_HEADER)
233252

234253
user = scope.get("user")
@@ -262,6 +281,36 @@ async def _handle_stateful_request(
262281
return
263282

264283
if request_mcp_session_id is None:
284+
# Only POST may initialize a new session (per MCP spec: a session is
285+
# opened by the response to the ``initialize`` JSON-RPC request,
286+
# which is always a POST). GET and DELETE without a session-id are
287+
# protocol errors and must be rejected here — the transport-layer
288+
# rejection happens too late to matter: by the time
289+
# `StreamableHTTPServerTransport.handle_request()` sees the request,
290+
# the session has already been allocated in `_server_instances`
291+
# AND a background `run_server` task has been spawned, waiting on a
292+
# stream that will never receive anything. Both leak forever unless
293+
# `session_idle_timeout` is set (which is opt-in and off by default).
294+
# Other unsupported methods (PUT/PATCH/OPTIONS/...) are intentionally
295+
# let through to the existing ``_handle_unsupported_request`` path,
296+
# which returns 405 — preserved for API stability.
297+
if request.method in ("GET", "DELETE"):
298+
error_response = JSONRPCError(
299+
jsonrpc="2.0",
300+
id="server-error",
301+
error=ErrorData(
302+
code=INVALID_REQUEST,
303+
message="Bad Request: Missing session ID",
304+
),
305+
)
306+
response = Response(
307+
content=error_response.model_dump_json(by_alias=True, exclude_none=True),
308+
status_code=400,
309+
media_type="application/json",
310+
)
311+
await response(scope, receive, send)
312+
return
313+
265314
# New session case
266315
logger.debug("Creating new transport")
267316
async with self._session_creation_lock:

tests/server/test_streamable_http_manager.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,77 @@ async def capture_send(message: Message):
382382
assert response_start["status"] == 404
383383

384384

385+
@pytest.mark.anyio
386+
@pytest.mark.parametrize("method", ["GET", "DELETE"])
387+
async def test_non_post_without_session_id_does_not_allocate_session(method: str):
388+
"""Regression test: GET/DELETE without a session-id must return 400 without
389+
allocating a session or spawning a background task.
390+
391+
Before this fix, any request without a session id — including GET/DELETE —
392+
entered the "new session" branch of the manager, created a transport,
393+
registered it in ``_server_instances``, and launched a ``run_server`` task
394+
that would wait forever for messages that never come. The transport-level
395+
validation then rejected the request (with 400 or 406), but the allocated
396+
session + task were leaked. Under a Docker healthcheck polling /mcp every
397+
30 seconds this accumulated ~2 sessions/min indefinitely (~1 GiB/week).
398+
"""
399+
app = Server("test-non-post-no-session")
400+
manager = StreamableHTTPSessionManager(app=app)
401+
402+
async with manager.run():
403+
sent_messages: list[Message] = []
404+
response_body = b""
405+
406+
async def mock_send(message: Message):
407+
nonlocal response_body
408+
sent_messages.append(message)
409+
if message["type"] == "http.response.body":
410+
response_body += message.get("body", b"")
411+
412+
scope: Scope = {
413+
"type": "http",
414+
"method": method,
415+
"path": "/mcp",
416+
"headers": [
417+
(b"accept", b"application/json, text/event-stream"),
418+
],
419+
}
420+
421+
async def mock_receive(): # pragma: no cover
422+
return {"type": "http.request", "body": b"", "more_body": False}
423+
424+
# Snapshot before
425+
assert len(manager._server_instances) == 0
426+
427+
await manager.handle_request(scope, mock_receive, mock_send)
428+
429+
# Give any accidentally-spawned background task a chance to register
430+
await anyio.sleep(0.05)
431+
432+
# No session, no task
433+
assert len(manager._server_instances) == 0, (
434+
f"{method} without session-id must not allocate a session — leaked {len(manager._server_instances)}"
435+
)
436+
assert manager._task_group is not None
437+
# anyio TaskGroup internals: no live tasks belonging to run_server
438+
assert len(manager._task_group._tasks) == 0, (
439+
f"{method} without session-id must not spawn a background task — leaked {len(manager._task_group._tasks)}"
440+
)
441+
442+
# Response should be a well-formed JSON-RPC error at status 400
443+
response_start = next(
444+
(msg for msg in sent_messages if msg["type"] == "http.response.start"),
445+
None,
446+
)
447+
assert response_start is not None, "Should have sent a response"
448+
assert response_start["status"] == 400
449+
450+
error_data = json.loads(response_body)
451+
assert error_data["jsonrpc"] == "2.0"
452+
assert error_data["error"]["code"] == INVALID_REQUEST
453+
assert "Missing session ID" in error_data["error"]["message"]
454+
455+
385456
def test_session_idle_timeout_rejects_non_positive():
386457
with pytest.raises(ValueError, match="positive number"):
387458
StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=-1)

0 commit comments

Comments
 (0)