[v1.x] Apply the request body limit to the SSE and OAuth endpoints - #3344
[v1.x] Apply the request body limit to the SSE and OAuth endpoints#3344maxisbey wants to merge 2 commits into
Conversation
SseServerTransport now takes max_request_body_size (default 4 MiB, the same default and validation as StreamableHTTPSessionManager) and answers 413 before session lookup or parsing when a request declares or streams a larger body. The message endpoint only ever handled POST bodies, so it now answers 405 (Allow: POST) to other methods instead of treating them like a POST. FastMCP forwards its existing max_request_body_size setting to the SSE transport, so one setting governs both HTTP transports. The create_auth_routes endpoints (/token, /revoke, /register and /authorize) are wrapped in RequestBodyLimitMiddleware at the route declarations and answer 413 to bodies over the 4 MiB default before any form or JSON parsing. On the CORS-enabled routes the limit sits inside the CORS wrapper so a 413 still carries CORS headers; cors_middleware itself is unchanged. The middleware no longer special-cases POST, since some of these routes also accept OPTIONS or HEAD and their handlers read the body either way. Differences from the main change: - FastMCP reuses its existing max_request_body_size setting for the SSE app instead of adding keywords to sse_app()/run(); no new FastMCP parameter. - /register reads its body via request.json() on this line; the same wrapper applies unchanged. - Tests use httpx and this line's dict-based SSE scope helper.
…ager module The middleware and DEFAULT_MAX_REQUEST_BODY_SIZE are now used by the SSE transport and the OAuth routes as well, so they move next to the other shared HTTP request checks in mcp.server.transport_security. Both names remain importable from mcp.server.streamable_http_manager (listed in its __all__). The middleware's own unit tests move with it, plus the mid-stream disconnect replay case main already carries; no behaviour change.
📚 Documentation preview
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline note, a few other candidate issues were checked and ruled out: the middleware's removal of the POST-only special case against Streamable HTTP GET/DELETE handling — the replay wrapper falls through to the live receive, so disconnect delivery on long-lived streams is unaffected; the now-default-on 4 MiB limit for the SSE message endpoint — it is the same FastMCP setting already governing Streamable HTTP and remains configurable on SseServerTransport; and the auth routes' switch from callable endpoints to wrapped ASGI instances — Starlette's Route treats non-function endpoints as ASGI apps, so dispatch and the CORS-outermost ordering behave as described.
Extended reasoning...
One inline finding (the pre-existing-equivalent KeyError on non-HTTP scopes in handle_post_message) is posted separately; this note only records what else was examined. Since the diff touches OAuth endpoint wiring in src/mcp/server/auth/routes.py and changes default behavior on the SSE message endpoint (413 on oversized bodies, 405 on non-POST), a human look remains worthwhile, but the specific concerns above — receive-replay semantics after dropping the POST guard, the newly-enforced SSE default limit, and Starlette endpoint-type handling for the restructured auth routes — were each verified against the code and are not problems.
| Only POST is accepted (other methods get 405), and bodies larger than | ||
| `max_request_body_size` are answered with 413 before the message is handled. | ||
| """ | ||
| if scope["method"] != "POST": |
There was a problem hiding this comment.
🟡 nit (pre-existing-equivalent): handle_post_message reads scope["method"] without a scope["type"] == "http" guard, so non-HTTP scopes (e.g. a websocket handshake routed through the Mount at the message path) raise KeyError before RequestBodyLimitMiddleware's own non-http passthrough can run
Extended reasoning...
A client opens a websocket connection to the mounted message endpoint (Starlette Mount matches websocket scopes); scope has no "method" key, so handle_post_message raises KeyError and the server returns a 500/unhandled-exception instead of cleanly rejecting the request. Before this change the same request also crashed (AssertionError in Request(scope)), so the user-visible outcome is unchanged — flagged only because the new code sits in front of a middleware that deliberately checks scope type.
Verification: nit. The new code at src/mcp/server/sse.py:237 (if scope["method"] != "POST":) reads scope["method"] with no scope["type"] == "http" guard. ASGI websocket scopes have no "method" key, and the callable is reachable by websocket handshakes: FastMCP mounts it via Mount(self.settings.message_path, app=sse.handle_post_message) (src/mcp/server/fastmcp/server.py:912-915, 931-935), and
1 participant
v1.x backport of #3336.
#3101 added
RequestBodyLimitMiddlewareand applied it to the Streamable HTTP endpoint on this line. This does the same for the other two places that accept request bodies, so every HTTP entry point shares the one 4 MiB default.Motivation and Context
SseServerTransporttakesmax_request_body_size(default 4 MiB, same validation asStreamableHTTPSessionManager).FastMCPforwards its existingmax_request_body_sizesetting to the SSE transport, so one setting governs both HTTP transports. The message endpoint now answers 405 to anything that isn't a POST instead of treating it as one.create_auth_routesendpoints (/token,/revoke,/register, POST/authorize) use the default limit. On the CORS-enabled routes it sits inside the CORS wrapper, so a 413 still carries CORS headers and preflights are untouched. These endpoints use the 4 MiB default rather than a configurable value; their payloads are a few kilobytes.OPTIONS/HEADand their handlers read the body either way.RequestBodyLimitMiddlewareandDEFAULT_MAX_REQUEST_BODY_SIZEmove tomcp.server.transport_security, next to the other shared HTTP request checks. Both remain importable frommcp.server.streamable_http_manager.Nothing changes for requests under the limit.
Differences from #3336
sse_app()/run()keywords:FastMCPalready has amax_request_body_sizesetting on this line, and it now applies to the SSE message endpoint as well.mcp.server.streamable_http_managerkeeps the two names importable via__all__rather than import aliases (this line's lint configuration flags redundant aliases).docs/server.mdwording for the existing setting now covers both HTTP transports; no other documentation changes.httpxand this line's SSE test helpers.How Has This Been Tested?
New tests in
tests/server/test_sse_security.py,tests/server/auth/test_error_handling.py,tests/server/test_transport_security.pyandtests/server/fastmcp/test_server.py: over-limit bodies (declared and streamed, across methods) get 413, bodies under the limit still reach session lookup / form parsing, CORS preflights are still answered and a 413 on a CORS route keeps its CORS headers, non-POST to the message endpoint gets 405, andsse_app()applies the configured setting. Full suite, pyright and ruff pass locally.Breaking Changes
None. The new
SseServerTransportkeyword is optional and defaults to the limit the Streamable HTTP transport already uses; the observable differences are a 413 for request bodies over 4 MiB on these endpoints and a 405 for non-POST requests to the SSE message endpoint.Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
None.
AI Disclaimer