[v1.x] Add Streamable HTTP request body limits#3101
Conversation
📚 Documentation preview
|
There was a problem hiding this comment.
Beyond the inline pragma nit, a few other candidates were examined and ruled out: (1) the middleware buffering the POST body before origin/session validation — the buffer is capped at max_request_body_size itself, so memory exposure is bounded by design; (2) the plain-text 413 body being inconsistent with the manager's JSON-RPC error responses — the rejection happens before JSON-RPC parsing, so there is no request id to correlate and a plain HTTP response is appropriate; (3) FastMCP not validating max_request_body_size at construction — the manager's constructor validates it when streamable_http_app() runs, so the error still surfaces, just later.
Extended reasoning...
Bugs-found run: one nit (stale pragma) posted inline. This note records the candidate issues that finder agents raised and verifiers refuted, plus my own check that the transport's DNS-rebinding/origin validation is header-only, so the middleware's bounded body buffering ahead of it does not weaken that protection. No verdict on overall approval is implied — the PR adds a new middleware to the streamable HTTP request path and warrants human review as a transport behavior change.
| def test_streamable_http_app_passes_the_configured_request_body_limit_to_its_manager() -> None: | ||
| """SDK-defined: FastMCP forwards its public request-body setting to the Streamable HTTP manager.""" | ||
| mcp = FastMCP(max_request_body_size=8) | ||
|
|
||
| mcp.streamable_http_app() | ||
|
|
||
| assert mcp.session_manager.max_request_body_size == 8 |
There was a problem hiding this comment.
🟡 The new test test_streamable_http_app_passes_the_configured_request_body_limit_to_its_manager accesses mcp.session_manager, which makes the # pragma: no cover on the return self._session_manager line (src/mcp/server/fastmcp/server.py:280) stale — the line is now executed by the suite but still excluded from coverage measurement. Since this PR already removes newly-stale pragmas elsewhere (the mock_receive functions in test_streamable_http_manager.py), drop this one too; the None-check raise branch above it remains genuinely uncovered and keeps its pragma.
Extended reasoning...
What the issue is
The FastMCP.session_manager property in src/mcp/server/fastmcp/server.py ends with:
return self._session_manager # pragma: no coverBefore this PR, no test in the suite accessed the session_manager property directly, so this pragma accurately marked an unexecuted line. This PR adds test_streamable_http_app_passes_the_configured_request_body_limit_to_its_manager (tests/server/fastmcp/test_server.py:1495-1501), which is now the only test that reads mcp.session_manager — and it does so after calling streamable_http_app(), so the None guard does not fire and the return line executes.
The specific code path
FastMCP(max_request_body_size=8)is constructed withself._session_manager = None.mcp.streamable_http_app()runs; the lazy-init branch creates theStreamableHTTPSessionManagerand assigns it toself._session_manager.- The test's assertion evaluates
mcp.session_manager, entering the property.self._session_manager is NoneisFalse, so theraise RuntimeError(...)branch is skipped andreturn self._session_managerat line 280 executes.
So line 280 is now exercised on every test run while still being excluded from coverage measurement by the pragma.
Why nothing catches this
This repo enforces fail_under = 100 with "pragma: no cover" in exclude_lines (pyproject.toml). Coverage.py silently ignores excluded lines whether or not they execute — it never fails on an excluded-but-executed line — so CI stays green and the stale pragma persists unnoticed.
Impact
Zero runtime impact; this is purely coverage hygiene. The cost is that a genuinely-covered line is invisible to coverage reporting, which erodes the value of the 100%-coverage convention: pragmas are supposed to mark only lines the suite truly cannot reach. Notably, this PR itself follows that convention elsewhere — it removes the now-stale # pragma: no cover comments from several mock_receive functions in tests/server/test_streamable_http_manager.py that became covered by the same change — so leaving this one behind is an internal inconsistency within the PR.
How to fix
Delete the trailing comment on line 280:
return self._session_managerThe raise RuntimeError branch at lines 273-279 (the self._session_manager is None case) remains genuinely unexecuted by the suite and should keep its pragma.
Step-by-step proof
- Run
grep -rn "\.session_manager" tests/— the only hit is the new test at tests/server/fastmcp/test_server.py:1501, confirming the pragma was accurate before this PR. - Run the new test:
FastMCP(max_request_body_size=8)→streamable_http_app()sets_session_manager→mcp.session_manager.max_request_body_size == 8passes, which requires the property to have returnedself._session_manager— i.e., line 280 executed. - Run coverage: line 280 shows as excluded (not counted covered or missing) because of the pragma, even though step 2 proved it executes. Removing the pragma and re-running keeps coverage at 100%, confirming the pragma is stale and its removal is safe.
Summary
max_request_body_sizeoption to the V1 Streamable HTTP serverFastMCPand document how to configure larger messagesBackport of #3095.
Validation
./scripts/test— 1,156 passed, 95 skipped, 1 xfailed, 100% coverageuv run --frozen pre-commit run --all-files