From a79c0f34e83904e501f1b92f925581f8b82abdff Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:03:28 +0000 Subject: [PATCH 1/2] [v1.x] Apply the request body limit to the SSE and OAuth endpoints 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. --- docs/server.md | 8 +- src/mcp/server/auth/routes.py | 43 ++++---- src/mcp/server/fastmcp/server.py | 2 + src/mcp/server/sse.py | 28 ++++- src/mcp/server/streamable_http_manager.py | 6 +- tests/server/auth/test_error_handling.py | 57 ++++++++++ tests/server/fastmcp/test_server.py | 15 +++ tests/server/test_sse_security.py | 104 +++++++++++++++++-- tests/server/test_streamable_http_manager.py | 35 +++++++ 9 files changed, 260 insertions(+), 38 deletions(-) diff --git a/docs/server.md b/docs/server.md index 6402596a57..75516f699a 100644 --- a/docs/server.md +++ b/docs/server.md @@ -1253,7 +1253,7 @@ The FastMCP server instance accessible via `ctx.fastmcp` provides access to serv - `host` and `port` - Server network configuration - `mount_path`, `sse_path`, `streamable_http_path` - Transport paths - `stateless_http` - Whether the server operates in stateless mode - - `max_request_body_size` - Maximum Streamable HTTP POST body size in bytes + - `max_request_body_size` - Maximum HTTP request body size in bytes (Streamable HTTP and SSE) - And other configuration options ```python @@ -1418,9 +1418,9 @@ Note that `uv run mcp run` or `uv run mcp dev` only supports server using FastMC > **Note**: Streamable HTTP transport is the recommended transport for production deployments. Use `stateless_http=True` and `json_response=True` for optimal scalability. -Streamable HTTP POST bodies are limited to 4 MiB by default. Larger requests receive HTTP 413 -before parsing or session creation. If your server intentionally accepts larger MCP messages, -configure the smallest suitable byte limit: +HTTP request bodies (Streamable HTTP and SSE) are limited to 4 MiB by default. Larger requests +receive HTTP 413 before parsing or session creation. If your server intentionally accepts larger MCP +messages, configure the smallest suitable byte limit: ```python mcp = FastMCP("Large messages", max_request_body_size=8 * 1024 * 1024) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 71a9c8b165..1e934ff8c9 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -18,6 +18,7 @@ from mcp.server.auth.provider import OAuthAuthorizationServerProvider from mcp.server.auth.settings import ClientRegistrationOptions, RevocationOptions from mcp.server.streamable_http import MCP_PROTOCOL_VERSION_HEADER +from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware from mcp.shared.auth import OAuthMetadata @@ -53,17 +54,24 @@ def validate_issuer_url(url: AnyHttpUrl): REVOCATION_PATH = "/revoke" -def cors_middleware( - handler: Callable[[Request], Response | Awaitable[Response]], - allow_methods: list[str], -) -> ASGIApp: - cors_app = CORSMiddleware( - app=request_response(handler), +def _cors(app: ASGIApp, allow_methods: list[str]) -> ASGIApp: + return CORSMiddleware( + app=app, allow_origins="*", allow_methods=allow_methods, allow_headers=[MCP_PROTOCOL_VERSION_HEADER], ) - return cors_app + + +def _body_limited(app: ASGIApp) -> ASGIApp: + return RequestBodyLimitMiddleware(app, DEFAULT_MAX_REQUEST_BODY_SIZE) + + +def cors_middleware( + handler: Callable[[Request], Response | Awaitable[Response]], + allow_methods: list[str], +) -> ASGIApp: + return _cors(request_response(handler), allow_methods) def create_auth_routes( @@ -84,11 +92,13 @@ def create_auth_routes( revocation_options, ) client_authenticator = ClientAuthenticator(provider) + token_handler = TokenHandler(provider, client_authenticator) # Create routes # Allow CORS requests for endpoints meant to be hit by the OAuth client # (with the client secret). This is intended to support things like MCP Inspector, - # where the client runs in a web browser. + # where the client runs in a web browser. CORS is the outermost wrapper so that + # responses produced by inner layers (such as a 413) still carry CORS headers. routes = [ Route( "/.well-known/oauth-authorization-server", @@ -102,15 +112,12 @@ def create_auth_routes( AUTHORIZATION_PATH, # do not allow CORS for authorization endpoint; # clients should just redirect to this - endpoint=AuthorizationHandler(provider).handle, + endpoint=_body_limited(request_response(AuthorizationHandler(provider).handle)), methods=["GET", "POST"], ), Route( TOKEN_PATH, - endpoint=cors_middleware( - TokenHandler(provider, client_authenticator).handle, - ["POST", "OPTIONS"], - ), + endpoint=_cors(_body_limited(request_response(token_handler.handle)), ["POST", "OPTIONS"]), methods=["POST", "OPTIONS"], ), ] @@ -123,10 +130,7 @@ def create_auth_routes( routes.append( Route( REGISTRATION_PATH, - endpoint=cors_middleware( - registration_handler.handle, - ["POST", "OPTIONS"], - ), + endpoint=_cors(_body_limited(request_response(registration_handler.handle)), ["POST", "OPTIONS"]), methods=["POST", "OPTIONS"], ) ) @@ -136,10 +140,7 @@ def create_auth_routes( routes.append( Route( REVOCATION_PATH, - endpoint=cors_middleware( - revocation_handler.handle, - ["POST", "OPTIONS"], - ), + endpoint=_cors(_body_limited(request_response(revocation_handler.handle)), ["POST", "OPTIONS"]), methods=["POST", "OPTIONS"], ) ) diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index e915a12bfd..1bcc0a8c03 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -107,6 +107,7 @@ class Settings(BaseSettings, Generic[LifespanResultT]): stateless_http: bool """Define if the server should create a new transport per request.""" max_request_body_size: int + """Maximum request body size in bytes for the Streamable HTTP endpoint and the SSE message endpoint.""" # resource settings warn_on_duplicate_resources: bool @@ -835,6 +836,7 @@ def sse_app(self, mount_path: str | None = None) -> Starlette: sse = SseServerTransport( normalized_message_endpoint, security_settings=self.settings.transport_security, + max_request_body_size=self.settings.max_request_body_size, ) async def handle_sse(scope: Scope, receive: Receive, send: Send): # pragma: no cover diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py index 489785c4c9..f68d75c189 100644 --- a/src/mcp/server/sse.py +++ b/src/mcp/server/sse.py @@ -53,6 +53,7 @@ async def handle_sse(request): import mcp.types as types from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser, AuthorizationContext, authorization_context +from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware from mcp.server.transport_security import ( TransportSecurityMiddleware, TransportSecuritySettings, @@ -81,7 +82,12 @@ class SseServerTransport: _session_owners: dict[UUID, AuthorizationContext] _security: TransportSecurityMiddleware - def __init__(self, endpoint: str, security_settings: TransportSecuritySettings | None = None) -> None: + def __init__( + self, + endpoint: str, + security_settings: TransportSecuritySettings | None = None, + max_request_body_size: int = DEFAULT_MAX_REQUEST_BODY_SIZE, + ) -> None: """ Creates a new SSE server transport, which will direct the client to POST messages to the relative path given. @@ -90,6 +96,9 @@ def __init__(self, endpoint: str, security_settings: TransportSecuritySettings | endpoint: A relative path where messages should be posted (e.g., "/messages/"). security_settings: Optional security settings for DNS rebinding protection. + max_request_body_size: Maximum size in bytes for POSTed message bodies. Requests that + declare or stream a larger body receive HTTP 413. Defaults to 4 MiB, matching + `StreamableHTTPSessionManager`. Note: We use relative paths instead of full URLs for several reasons: @@ -106,6 +115,9 @@ def __init__(self, endpoint: str, security_settings: TransportSecuritySettings | super().__init__() + if max_request_body_size <= 0: + raise ValueError("max_request_body_size must be a positive number of bytes") + # Validate that endpoint is a relative path and not a full URL if "://" in endpoint or endpoint.startswith("//") or "?" in endpoint or "#" in endpoint: raise ValueError( @@ -121,6 +133,7 @@ def __init__(self, endpoint: str, security_settings: TransportSecuritySettings | self._read_stream_writers = {} self._session_owners = {} self._security = TransportSecurityMiddleware(security_settings) + self._post_message_app = RequestBodyLimitMiddleware(self._handle_post_message, max_request_body_size) logger.debug(f"SseServerTransport initialized with endpoint: {endpoint}") @asynccontextmanager @@ -214,7 +227,18 @@ async def response_wrapper(scope: Scope, receive: Receive, send: Send): self._read_stream_writers.pop(session_id, None) self._session_owners.pop(session_id, None) - async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) -> None: # pragma: no cover + async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) -> None: + """ASGI application for the message endpoint. + + 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": + response = Response(status_code=405, headers={"Allow": "POST"}) + return await response(scope, receive, send) + await self._post_message_app(scope, receive, send) + + async def _handle_post_message(self, scope: Scope, receive: Receive, send: Send) -> None: # pragma: no cover logger.debug("Handling POST message") request = Request(scope, receive) diff --git a/src/mcp/server/streamable_http_manager.py b/src/mcp/server/streamable_http_manager.py index 0ee6d362b5..69fe9788c8 100644 --- a/src/mcp/server/streamable_http_manager.py +++ b/src/mcp/server/streamable_http_manager.py @@ -29,7 +29,7 @@ logger = logging.getLogger(__name__) DEFAULT_MAX_REQUEST_BODY_SIZE: Final = 4 * 1024 * 1024 -"""Default maximum Streamable HTTP request body size in bytes (4 MiB).""" +"""Default maximum HTTP request body size in bytes (4 MiB).""" class StreamableHTTPSessionManager: @@ -65,7 +65,7 @@ class StreamableHTTPSessionManager: retry_interval is also configured, ensure the idle timeout comfortably exceeds the retry interval to avoid reaping sessions during normal SSE polling gaps. Default is None (no timeout). A value of 1800 (30 minutes) is recommended for most deployments. - max_request_body_size: Maximum size in bytes for Streamable HTTP POST request bodies. Requests that + max_request_body_size: Maximum size in bytes for Streamable HTTP request bodies. Requests that exceed this limit receive a 413 response before parsing or session creation. Defaults to 4 MiB. """ @@ -371,7 +371,7 @@ def __init__(self, app: ASGIApp, max_body_size: int) -> None: self.max_body_size = max_body_size async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: - if scope["type"] != "http" or scope["method"] != "POST": + if scope["type"] != "http": await self.app(scope, receive, send) return diff --git a/tests/server/auth/test_error_handling.py b/tests/server/auth/test_error_handling.py index f331b2cb2d..eb7db64748 100644 --- a/tests/server/auth/test_error_handling.py +++ b/tests/server/auth/test_error_handling.py @@ -14,6 +14,7 @@ from mcp.server.auth.provider import AuthorizeError, RegistrationError, TokenError from mcp.server.auth.routes import create_auth_routes +from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE # TODO(Marcelo): This TYPE_CHECKING shouldn't be here, but pytest doesn't seem to get the module correctly. if TYPE_CHECKING: @@ -302,3 +303,59 @@ async def test_token_error_handling_refresh_token( data = refresh_response.json() assert data["error"] == "invalid_scope" assert data["error_description"] == "The requested scope is invalid" + + +_FORM = "application/x-www-form-urlencoded" + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("method", "path", "content_type"), + [ + ("POST", "/token", _FORM), + ("POST", "/revoke", _FORM), + ("POST", "/register", "application/json"), + ("POST", "/authorize", _FORM), + # The other methods these routes accept reach the same body-reading handlers. + ("OPTIONS", "/token", _FORM), + ("OPTIONS", "/revoke", _FORM), + ("OPTIONS", "/register", "application/json"), + ("HEAD", "/authorize", _FORM), + ], +) +async def test_oversized_request_body_returns_413(client: httpx.AsyncClient, method: str, path: str, content_type: str): + """Each endpoint that reads a request body rejects one over 4 MiB before parsing it, whatever the method.""" + response = await client.request( + method, path, content=b"x" * (DEFAULT_MAX_REQUEST_BODY_SIZE + 1), headers={"Content-Type": content_type} + ) + assert response.status_code == 413 + + +@pytest.mark.anyio +async def test_request_body_within_the_limit_is_still_parsed(client: httpx.AsyncClient): + """A small body is passed through to the handler intact: the form is parsed and its fields validated.""" + response = await client.post("/token", data={"grant_type": "authorization_code"}) + assert response.status_code == 401 + assert response.json() == {"error": "unauthorized_client", "error_description": "Missing client_id"} + + +@pytest.mark.anyio +async def test_cors_preflight_is_still_answered(client: httpx.AsyncClient): + """A CORS preflight to a body-limited endpoint is answered by the CORS layer as before.""" + response = await client.options( + "/token", headers={"Origin": "https://client.example.com", "Access-Control-Request-Method": "POST"} + ) + assert response.status_code == 200 + assert response.headers["access-control-allow-origin"] == "*" + + +@pytest.mark.anyio +async def test_oversized_cross_origin_request_gets_413_with_cors_headers(client: httpx.AsyncClient): + """The 413 is produced inside the CORS layer, so a browser client can still read it.""" + response = await client.post( + "/token", + content=b"x" * (DEFAULT_MAX_REQUEST_BODY_SIZE + 1), + headers={"Content-Type": _FORM, "Origin": "https://client.example.com"}, + ) + assert response.status_code == 413 + assert response.headers["access-control-allow-origin"] == "*" diff --git a/tests/server/fastmcp/test_server.py b/tests/server/fastmcp/test_server.py index b134489bc5..eb3091e764 100644 --- a/tests/server/fastmcp/test_server.py +++ b/tests/server/fastmcp/test_server.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING, Any from unittest.mock import patch +import httpx import pytest from pydantic import AnyUrl, BaseModel from starlette.routing import Mount, Route @@ -1499,3 +1500,17 @@ def test_streamable_http_app_passes_the_configured_request_body_limit_to_its_man mcp.streamable_http_app() assert mcp.session_manager.max_request_body_size == 8 + + +@pytest.mark.anyio +async def test_sse_app_applies_the_configured_request_body_limit() -> None: + """FastMCP forwards its request-body setting to the SSE message endpoint: larger POSTs get HTTP 413.""" + mcp = FastMCP(host="0.0.0.0", max_request_body_size=8) + transport = httpx.ASGITransport(app=mcp.sse_app()) + async with httpx.AsyncClient(transport=transport, base_url="http://localhost") as http: + response = await http.post( + "/messages/?session_id=12345678123456781234567812345678", + content=b"123456789", + headers={"Content-Type": "application/json"}, + ) + assert response.status_code == 413 diff --git a/tests/server/test_sse_security.py b/tests/server/test_sse_security.py index 0978b8a150..42fe944af2 100644 --- a/tests/server/test_sse_security.py +++ b/tests/server/test_sse_security.py @@ -21,6 +21,7 @@ from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser from mcp.server.auth.provider import AccessToken from mcp.server.sse import SseServerTransport +from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE from mcp.server.transport_security import TransportSecuritySettings from mcp.types import Tool from tests.test_helpers import wait_for_server @@ -323,14 +324,14 @@ def _authenticated_user(client_id: str, subject: str | None = None, issuer: str return AuthenticatedUser(AccessToken(token="token", client_id=client_id, scopes=[], subject=subject, claims=claims)) -def _sse_scope(method: str, path: str, user: AuthenticatedUser | None) -> dict[str, Any]: +def _sse_scope(method: str, path: str, user: AuthenticatedUser | None, *, query_string: bytes = b"") -> dict[str, Any]: """Build an ASGI scope for a request to the SSE transport.""" scope: dict[str, Any] = { "type": "http", "method": method, "path": path, "root_path": "", - "query_string": b"", + "query_string": query_string, "headers": [(b"content-type", b"application/json")], } if user is not None: @@ -338,24 +339,44 @@ def _sse_scope(method: str, path: str, user: AuthenticatedUser | None) -> dict[s return scope -async def _post_message(transport: SseServerTransport, session_id: str, user: AuthenticatedUser | None) -> int: - """POST a message to an SSE session as `user` and return the response status.""" - body = b'{"jsonrpc": "2.0", "id": 1, "method": "ping", "params": null}' - scope = _sse_scope("POST", "/messages/", user) - scope["query_string"] = f"session_id={session_id}".encode() +async def _call_message_endpoint( + transport: SseServerTransport, scope: dict[str, Any], body: bytes | list[bytes] +) -> list[Message]: + """Send a request to the transport's message endpoint and return the ASGI messages it sent. + + `body` may be a list of chunks to deliver the request body over several `http.request` messages; + no Content-Length header is set either way. + """ sent: list[Message] = [] + chunks = list(body) if isinstance(body, list) else [body] async def receive() -> Message: - return {"type": "http.request", "body": body, "more_body": False} + chunk = chunks.pop(0) + return {"type": "http.request", "body": chunk, "more_body": bool(chunks)} async def send(message: Message) -> None: sent.append(message) await transport.handle_post_message(scope, receive, send) + return sent + + +def _response_status(sent: list[Message]) -> int: response_start = next(msg for msg in sent if msg["type"] == "http.response.start") return response_start["status"] +def _response_body(sent: list[Message]) -> bytes: + return b"".join(msg.get("body", b"") for msg in sent if msg["type"] == "http.response.body") + + +async def _post_message(transport: SseServerTransport, session_id: str, user: AuthenticatedUser | None) -> int: + """POST a message to an SSE session as `user` and return the response status.""" + body = b'{"jsonrpc": "2.0", "id": 1, "method": "ping", "params": null}' + scope = _sse_scope("POST", "/messages/", user, query_string=f"session_id={session_id}".encode()) + return _response_status(await _call_message_endpoint(transport, scope, body)) + + _Principal = tuple[str] | tuple[str, str] | tuple[str, str, str] @@ -426,3 +447,70 @@ async def hold_sse_connection() -> None: # Once the connection is gone the session is no longer routable. assert await _post_message(transport, session_ids[0], creator_user) == 404 + + +# A well-formed session ID that no live session owns. +_UNKNOWN_SESSION = b"session_id=12345678123456781234567812345678" + + +@pytest.mark.anyio +async def test_sse_post_body_over_the_limit_returns_413(): + """A POST body larger than max_request_body_size is answered with 413 before any session handling.""" + transport = SseServerTransport("/messages/", max_request_body_size=8) + scope = _sse_scope("POST", "/messages/", None, query_string=_UNKNOWN_SESSION) + + sent = await _call_message_endpoint(transport, scope, b"123456789") + assert _response_status(sent) == 413 + assert _response_body(sent) == b"Request body too large" + + +@pytest.mark.anyio +async def test_sse_post_body_limit_defaults_to_four_mib(): + """Without an explicit limit, a body one byte over 4 MiB (and no Content-Length) is answered with 413.""" + transport = SseServerTransport("/messages/") + scope = _sse_scope("POST", "/messages/", None, query_string=_UNKNOWN_SESSION) + + sent = await _call_message_endpoint(transport, scope, b"x" * (DEFAULT_MAX_REQUEST_BODY_SIZE + 1)) + assert _response_status(sent) == 413 + + +@pytest.mark.anyio +async def test_sse_post_streamed_body_over_the_limit_returns_413(): + """The limit counts bytes across body chunks, not just a declared Content-Length.""" + transport = SseServerTransport("/messages/", max_request_body_size=8) + scope = _sse_scope("POST", "/messages/", None, query_string=_UNKNOWN_SESSION) + + sent = await _call_message_endpoint(transport, scope, [b"1234", b"56789"]) + assert _response_status(sent) == 413 + + +@pytest.mark.anyio +async def test_sse_post_within_the_limit_reaches_session_lookup(): + """A body within the limit is passed on intact: an unknown session still gets its 404.""" + transport = SseServerTransport("/messages/", max_request_body_size=64) + scope = _sse_scope("POST", "/messages/", None, query_string=_UNKNOWN_SESSION) + + sent = await _call_message_endpoint(transport, scope, [b'{"jsonrpc": ', b'"2.0"}']) + assert _response_status(sent) == 404 + assert _response_body(sent) == b"Could not find session" + + +@pytest.mark.anyio +@pytest.mark.parametrize("method", ["GET", "PUT"]) +async def test_sse_message_endpoint_answers_405_to_non_post(method: str): + """The message endpoint only accepts POST; other methods get 405 with an Allow header.""" + transport = SseServerTransport("/messages/") + scope = _sse_scope(method, "/messages/", None, query_string=_UNKNOWN_SESSION) + + sent = await _call_message_endpoint(transport, scope, b"{}") + assert _response_status(sent) == 405 + response_start = next(msg for msg in sent if msg["type"] == "http.response.start") + assert (b"allow", b"POST") in response_start["headers"] + + +@pytest.mark.parametrize("max_request_body_size", [0, -1]) +def test_sse_transport_rejects_a_non_positive_body_limit(max_request_body_size: int): + """The body limit must be a positive number of bytes, matching StreamableHTTPSessionManager.""" + with pytest.raises(ValueError) as exc_info: + SseServerTransport("/messages/", max_request_body_size=max_request_body_size) + assert str(exc_info.value) == "max_request_body_size must be a positive number of bytes" diff --git a/tests/server/test_streamable_http_manager.py b/tests/server/test_streamable_http_manager.py index 9deeeeb37a..31d828e546 100644 --- a/tests/server/test_streamable_http_manager.py +++ b/tests/server/test_streamable_http_manager.py @@ -194,6 +194,41 @@ async def app(scope: Scope, receive: Receive, send: Send) -> None: assert received_messages == [disconnect] +@pytest.mark.anyio +@pytest.mark.parametrize("method", ["GET", "PUT", "OPTIONS", "HEAD", "DELETE"]) +async def test_request_body_limit_applies_to_every_method(method: str) -> None: + """SDK-defined: the limit is a property of the request body, not of the method that carries it.""" + app = AsyncMock() + sent_messages: list[Message] = [] + receive = AsyncMock(return_value={"type": "http.request", "body": b"123456789", "more_body": False}) + + async def send(message: Message) -> None: + sent_messages.append(message) + + scope: Scope = {"type": "http", "method": method, "path": "/mcp", "headers": []} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, send) + + assert [message["status"] for message in sent_messages if message["type"] == "http.response.start"] == [413] + app.assert_not_awaited() + + +@pytest.mark.anyio +async def test_request_body_limit_leaves_non_http_scopes_alone() -> None: + """SDK-defined: only HTTP requests carry a body to limit; other ASGI scopes go straight to the app.""" + app = AsyncMock() + receive = AsyncMock() + send = AsyncMock() + scope: Scope = {"type": "lifespan"} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, send) + + app.assert_awaited_once_with(scope, receive, send) + receive.assert_not_awaited() + + def test_request_body_limit_defaults_to_four_mib() -> None: """SDK-defined: Streamable HTTP request bodies are limited to 4 MiB by default.""" manager = StreamableHTTPSessionManager(app=Server("test-default-size-limit")) From d838cd35197d19007421ed6a7c87f84b8c4a68aa Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:28:52 +0000 Subject: [PATCH 2/2] [v1.x] Move RequestBodyLimitMiddleware out of the Streamable HTTP manager 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. --- src/mcp/server/auth/routes.py | 2 +- src/mcp/server/fastmcp/server.py | 4 +- src/mcp/server/sse.py | 3 +- src/mcp/server/streamable_http_manager.py | 77 ++---------- src/mcp/server/transport_security.py | 69 ++++++++++- tests/server/auth/test_error_handling.py | 2 +- tests/server/test_sse_security.py | 3 +- tests/server/test_streamable_http_manager.py | 89 +------------- tests/server/test_transport_security.py | 122 +++++++++++++++++++ 9 files changed, 207 insertions(+), 164 deletions(-) create mode 100644 tests/server/test_transport_security.py diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 1e934ff8c9..0a98c38419 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -18,7 +18,7 @@ from mcp.server.auth.provider import OAuthAuthorizationServerProvider from mcp.server.auth.settings import ClientRegistrationOptions, RevocationOptions from mcp.server.streamable_http import MCP_PROTOCOL_VERSION_HEADER -from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware +from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware from mcp.shared.auth import OAuthMetadata diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index 1bcc0a8c03..67b9b0c595 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -62,8 +62,8 @@ from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.server.streamable_http import EventStore -from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE, StreamableHTTPSessionManager -from mcp.server.transport_security import TransportSecuritySettings +from mcp.server.streamable_http_manager import StreamableHTTPSessionManager +from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, TransportSecuritySettings from mcp.shared.context import LifespanContextT, RequestContext, RequestT from mcp.types import Annotations, AnyFunction, ContentBlock, GetPromptResult, Icon, ToolAnnotations from mcp.types import Prompt as MCPPrompt diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py index f68d75c189..89384bfc02 100644 --- a/src/mcp/server/sse.py +++ b/src/mcp/server/sse.py @@ -53,8 +53,9 @@ async def handle_sse(request): import mcp.types as types from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser, AuthorizationContext, authorization_context -from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware from mcp.server.transport_security import ( + DEFAULT_MAX_REQUEST_BODY_SIZE, + RequestBodyLimitMiddleware, TransportSecurityMiddleware, TransportSecuritySettings, ) diff --git a/src/mcp/server/streamable_http_manager.py b/src/mcp/server/streamable_http_manager.py index 69fe9788c8..dd8c2ae245 100644 --- a/src/mcp/server/streamable_http_manager.py +++ b/src/mcp/server/streamable_http_manager.py @@ -4,17 +4,15 @@ import contextlib import logging -from collections import deque from collections.abc import AsyncIterator -from typing import Any, Final +from typing import Any from uuid import uuid4 import anyio from anyio.abc import TaskStatus -from starlette.datastructures import Headers from starlette.requests import Request from starlette.responses import Response -from starlette.types import ASGIApp, Message, Receive, Scope, Send +from starlette.types import Receive, Scope, Send from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser, AuthorizationContext, authorization_context from mcp.server.lowlevel.server import Server as MCPServer @@ -23,13 +21,16 @@ EventStore, StreamableHTTPServerTransport, ) -from mcp.server.transport_security import TransportSecuritySettings +from mcp.server.transport_security import ( + DEFAULT_MAX_REQUEST_BODY_SIZE, + RequestBodyLimitMiddleware, + TransportSecuritySettings, +) from mcp.types import INVALID_REQUEST, ErrorData, JSONRPCError -logger = logging.getLogger(__name__) +__all__ = ["DEFAULT_MAX_REQUEST_BODY_SIZE", "RequestBodyLimitMiddleware", "StreamableHTTPSessionManager"] -DEFAULT_MAX_REQUEST_BODY_SIZE: Final = 4 * 1024 * 1024 -"""Default maximum HTTP request body size in bytes (4 MiB).""" +logger = logging.getLogger(__name__) class StreamableHTTPSessionManager: @@ -361,63 +362,3 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE body.model_dump_json(by_alias=True, exclude_none=True), status_code=404, media_type="application/json" ) await response(scope, receive, send) - - -class RequestBodyLimitMiddleware: - """Reject oversized HTTP request bodies before invoking an ASGI application.""" - - def __init__(self, app: ASGIApp, max_body_size: int) -> None: - self.app = app - self.max_body_size = max_body_size - - async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: - if scope["type"] != "http": - await self.app(scope, receive, send) - return - - headers = Headers(scope=scope) - content_length = headers.get("content-length") - if content_length is not None: - try: - declared_size = int(content_length) - except ValueError: - pass - else: - if declared_size > self.max_body_size: - response = Response("Request body too large", status_code=413) - return await response(scope, receive, send) - - received_body = bytearray() - received_request = False - body_complete = False - trailing_message: Message | None = None - while True: - message = await receive() - if message["type"] != "http.request": - trailing_message = message - break - - received_request = True - body = message.get("body", b"") - if len(received_body) + len(body) > self.max_body_size: - response = Response("Request body too large", status_code=413) - return await response(scope, receive, send) - received_body.extend(body) - if not message.get("more_body", False): - body_complete = True - break - - cached_messages: deque[Message] = deque() - if received_request: - cached_messages.append( - {"type": "http.request", "body": bytes(received_body), "more_body": not body_complete} - ) - if trailing_message is not None: - cached_messages.append(trailing_message) - - async def replay() -> Message: - if cached_messages: - return cached_messages.popleft() - return await receive() - - await self.app(scope, replay, send) diff --git a/src/mcp/server/transport_security.py b/src/mcp/server/transport_security.py index 5022a1a2fe..37abfb5ac0 100644 --- a/src/mcp/server/transport_security.py +++ b/src/mcp/server/transport_security.py @@ -1,13 +1,20 @@ -"""DNS rebinding protection for MCP server transports.""" +"""Request checks shared by the HTTP server transports: Host/Origin header validation and body size limits.""" import logging +from collections import deque +from typing import Final from pydantic import BaseModel, Field +from starlette.datastructures import Headers from starlette.requests import HTTPConnection from starlette.responses import Response +from starlette.types import ASGIApp, Message, Receive, Scope, Send logger = logging.getLogger(__name__) +DEFAULT_MAX_REQUEST_BODY_SIZE: Final = 4 * 1024 * 1024 +"""Default maximum HTTP request body size in bytes (4 MiB).""" + class TransportSecuritySettings(BaseModel): """Settings for MCP transport security features. @@ -125,3 +132,63 @@ async def validate_request(self, request: HTTPConnection, is_post: bool = False) return Response("Invalid Origin header", status_code=403) # pragma: no cover return None # pragma: no cover + + +class RequestBodyLimitMiddleware: + """Reject oversized HTTP request bodies before invoking an ASGI application.""" + + def __init__(self, app: ASGIApp, max_body_size: int) -> None: + self.app = app + self.max_body_size = max_body_size + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + headers = Headers(scope=scope) + content_length = headers.get("content-length") + if content_length is not None: + try: + declared_size = int(content_length) + except ValueError: + pass + else: + if declared_size > self.max_body_size: + response = Response("Request body too large", status_code=413) + return await response(scope, receive, send) + + received_body = bytearray() + received_request = False + body_complete = False + trailing_message: Message | None = None + while True: + message = await receive() + if message["type"] != "http.request": + trailing_message = message + break + + received_request = True + body = message.get("body", b"") + if len(received_body) + len(body) > self.max_body_size: + response = Response("Request body too large", status_code=413) + return await response(scope, receive, send) + received_body.extend(body) + if not message.get("more_body", False): + body_complete = True + break + + cached_messages: deque[Message] = deque() + if received_request: + cached_messages.append( + {"type": "http.request", "body": bytes(received_body), "more_body": not body_complete} + ) + if trailing_message is not None: + cached_messages.append(trailing_message) + + async def replay() -> Message: + if cached_messages: + return cached_messages.popleft() + return await receive() + + await self.app(scope, replay, send) diff --git a/tests/server/auth/test_error_handling.py b/tests/server/auth/test_error_handling.py index eb7db64748..30bdac14bb 100644 --- a/tests/server/auth/test_error_handling.py +++ b/tests/server/auth/test_error_handling.py @@ -14,7 +14,7 @@ from mcp.server.auth.provider import AuthorizeError, RegistrationError, TokenError from mcp.server.auth.routes import create_auth_routes -from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE +from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE # TODO(Marcelo): This TYPE_CHECKING shouldn't be here, but pytest doesn't seem to get the module correctly. if TYPE_CHECKING: diff --git a/tests/server/test_sse_security.py b/tests/server/test_sse_security.py index 42fe944af2..68a96f1bd1 100644 --- a/tests/server/test_sse_security.py +++ b/tests/server/test_sse_security.py @@ -21,8 +21,7 @@ from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser from mcp.server.auth.provider import AccessToken from mcp.server.sse import SseServerTransport -from mcp.server.streamable_http_manager import DEFAULT_MAX_REQUEST_BODY_SIZE -from mcp.server.transport_security import TransportSecuritySettings +from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, TransportSecuritySettings from mcp.types import Tool from tests.test_helpers import wait_for_server diff --git a/tests/server/test_streamable_http_manager.py b/tests/server/test_streamable_http_manager.py index 31d828e546..e7f76419a7 100644 --- a/tests/server/test_streamable_http_manager.py +++ b/tests/server/test_streamable_http_manager.py @@ -7,7 +7,7 @@ import anyio import pytest -from starlette.types import Message, Receive, Scope, Send +from starlette.types import Message, Scope from mcp.server import streamable_http_manager from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser @@ -16,7 +16,6 @@ from mcp.server.streamable_http import MCP_SESSION_ID_HEADER, StreamableHTTPServerTransport from mcp.server.streamable_http_manager import ( DEFAULT_MAX_REQUEST_BODY_SIZE, - RequestBodyLimitMiddleware, StreamableHTTPSessionManager, ) from mcp.types import INVALID_REQUEST @@ -143,92 +142,6 @@ async def send(message: Message) -> None: assert response_start["status"] == 413 -@pytest.mark.anyio -async def test_request_body_chunks_are_replayed_as_one_message() -> None: - """SDK-defined: raw ASGI proves chunk overhead is discarded before the body reaches the transport.""" - request_messages: Iterator[Message] = iter( - [ - {"type": "http.request", "body": b"12", "more_body": True}, - {"type": "http.request", "body": b"34", "more_body": True}, - {"type": "http.request", "body": b"56", "more_body": False}, - {"type": "http.disconnect"}, - ] - ) - received_messages: list[Message] = [] - - async def receive() -> Message: - return next(request_messages) - - async def app(scope: Scope, receive: Receive, send: Send) -> None: - received_messages.append(await receive()) - received_messages.append(await receive()) - - scope: Scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} - middleware = RequestBodyLimitMiddleware(app, max_body_size=8) - - await middleware(scope, receive, AsyncMock()) - - assert received_messages == [ - {"type": "http.request", "body": b"123456", "more_body": False}, - {"type": "http.disconnect"}, - ] - - -@pytest.mark.anyio -async def test_disconnect_before_request_body_is_replayed() -> None: - """SDK-defined: raw ASGI proves a disconnect before the first body message reaches the transport.""" - disconnect: Message = {"type": "http.disconnect"} - received_messages: list[Message] = [] - - async def receive() -> Message: - return disconnect - - async def app(scope: Scope, receive: Receive, send: Send) -> None: - received_messages.append(await receive()) - - scope: Scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} - middleware = RequestBodyLimitMiddleware(app, max_body_size=8) - - await middleware(scope, receive, AsyncMock()) - - assert received_messages == [disconnect] - - -@pytest.mark.anyio -@pytest.mark.parametrize("method", ["GET", "PUT", "OPTIONS", "HEAD", "DELETE"]) -async def test_request_body_limit_applies_to_every_method(method: str) -> None: - """SDK-defined: the limit is a property of the request body, not of the method that carries it.""" - app = AsyncMock() - sent_messages: list[Message] = [] - receive = AsyncMock(return_value={"type": "http.request", "body": b"123456789", "more_body": False}) - - async def send(message: Message) -> None: - sent_messages.append(message) - - scope: Scope = {"type": "http", "method": method, "path": "/mcp", "headers": []} - middleware = RequestBodyLimitMiddleware(app, max_body_size=8) - - await middleware(scope, receive, send) - - assert [message["status"] for message in sent_messages if message["type"] == "http.response.start"] == [413] - app.assert_not_awaited() - - -@pytest.mark.anyio -async def test_request_body_limit_leaves_non_http_scopes_alone() -> None: - """SDK-defined: only HTTP requests carry a body to limit; other ASGI scopes go straight to the app.""" - app = AsyncMock() - receive = AsyncMock() - send = AsyncMock() - scope: Scope = {"type": "lifespan"} - middleware = RequestBodyLimitMiddleware(app, max_body_size=8) - - await middleware(scope, receive, send) - - app.assert_awaited_once_with(scope, receive, send) - receive.assert_not_awaited() - - def test_request_body_limit_defaults_to_four_mib() -> None: """SDK-defined: Streamable HTTP request bodies are limited to 4 MiB by default.""" manager = StreamableHTTPSessionManager(app=Server("test-default-size-limit")) diff --git a/tests/server/test_transport_security.py b/tests/server/test_transport_security.py new file mode 100644 index 0000000000..19522092ed --- /dev/null +++ b/tests/server/test_transport_security.py @@ -0,0 +1,122 @@ +"""Tests for the request checks shared by the HTTP server transports.""" + +from collections.abc import Iterator +from unittest.mock import AsyncMock + +import pytest +from starlette.types import Message, Receive, Scope, Send + +from mcp.server.transport_security import RequestBodyLimitMiddleware + + +@pytest.mark.anyio +async def test_request_body_chunks_are_replayed_as_one_message() -> None: + """SDK-defined: raw ASGI proves chunk overhead is discarded before the body reaches the transport.""" + request_messages: Iterator[Message] = iter( + [ + {"type": "http.request", "body": b"12", "more_body": True}, + {"type": "http.request", "body": b"34", "more_body": True}, + {"type": "http.request", "body": b"56", "more_body": False}, + {"type": "http.disconnect"}, + ] + ) + received_messages: list[Message] = [] + + async def receive() -> Message: + return next(request_messages) + + async def app(scope: Scope, receive: Receive, send: Send) -> None: + received_messages.append(await receive()) + received_messages.append(await receive()) + + scope: Scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, AsyncMock()) + + assert received_messages == [ + {"type": "http.request", "body": b"123456", "more_body": False}, + {"type": "http.disconnect"}, + ] + + +@pytest.mark.anyio +async def test_client_disconnect_while_streaming_request_body_is_replayed() -> None: + """SDK-defined: raw ASGI is required to prove a disconnect before body completion reaches the transport.""" + disconnect: Message = {"type": "http.disconnect"} + request_messages: Iterator[Message] = iter( + [{"type": "http.request", "body": b"1234", "more_body": True}, disconnect] + ) + received_messages: list[Message] = [] + + async def receive() -> Message: + return next(request_messages) + + async def app(scope: Scope, receive: Receive, send: Send) -> None: + received_messages.append(await receive()) + received_messages.append(await receive()) + + scope: Scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, AsyncMock()) + + assert received_messages == [ + {"type": "http.request", "body": b"1234", "more_body": True}, + disconnect, + ] + + +@pytest.mark.anyio +async def test_disconnect_before_request_body_is_replayed() -> None: + """SDK-defined: raw ASGI proves a disconnect before the first body message reaches the transport.""" + disconnect: Message = {"type": "http.disconnect"} + received_messages: list[Message] = [] + + async def receive() -> Message: + return disconnect + + async def app(scope: Scope, receive: Receive, send: Send) -> None: + received_messages.append(await receive()) + + scope: Scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, AsyncMock()) + + assert received_messages == [disconnect] + + +@pytest.mark.anyio +@pytest.mark.parametrize("method", ["GET", "PUT", "OPTIONS", "HEAD", "DELETE"]) +async def test_request_body_limit_applies_to_every_method(method: str) -> None: + """SDK-defined: the limit is a property of the request body, not of the method that carries it.""" + app = AsyncMock() + sent_messages: list[Message] = [] + receive = AsyncMock(return_value={"type": "http.request", "body": b"123456789", "more_body": False}) + + async def send(message: Message) -> None: + sent_messages.append(message) + + scope: Scope = {"type": "http", "method": method, "path": "/mcp", "headers": []} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, send) + + assert [message["status"] for message in sent_messages if message["type"] == "http.response.start"] == [413] + app.assert_not_awaited() + + +@pytest.mark.anyio +async def test_request_body_limit_leaves_non_http_scopes_alone() -> None: + """SDK-defined: only HTTP requests carry a body to limit; other ASGI scopes go straight to the app.""" + app = AsyncMock() + receive = AsyncMock() + send = AsyncMock() + scope: Scope = {"type": "lifespan"} + middleware = RequestBodyLimitMiddleware(app, max_body_size=8) + + await middleware(scope, receive, send) + + app.assert_awaited_once_with(scope, receive, send) + receive.assert_not_awaited()