fix(server): read HTTP request bodies with a size limit and bound JSON-RPC batch length - #2698
fix(server): read HTTP request bodies with a size limit and bound JSON-RPC batch length#2698maxisbey wants to merge 3 commits into
Conversation
…N-RPC batch length The Streamable HTTP transport, createMcpHandler, the Node adapter's toWebRequest and the Hono app's JSON pre-parse each read the whole POST body before doing anything with it. Read it with a 4 MiB limit instead (the value the legacy SSE transport uses; the Express adapter and stdio already bound their reads) and answer 413 past that, and cap JSON-RPC batch arrays at 100 messages. The Node adapter answers its 413 with `connection: close` so the partly read socket is not kept alive. createMcpHonoApp now runs its Host/Origin validation before the JSON pre-parse.
🦋 Changeset detectedLatest commit: 0fbc28c The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| for await (const chunk of req) { | ||
| received += typeof chunk === 'string' ? new TextEncoder().encode(chunk).byteLength : (chunk as Uint8Array).byteLength; | ||
| if (received > MAX_REQUEST_BODY_SIZE) { | ||
| throw new RequestBodyTooLargeError(); | ||
| } |
There was a problem hiding this comment.
🔴 toWebRequest throws RequestBodyTooLargeError from inside for await (const chunk of req); early exit from a for-await over a Node Readable invokes the iterator's return(), which destroys the stream, and destroying an http.IncomingMessage whose body is not fully read destroys the underlying socket. The 413 + connection:close response that toNodeHandler's catch (lines 133-140) builds for exactly this error is then written to a dead socket and never reaches the client. The new test masks this by feeding Readable.from(chunks) and a mock res instead of a real HTTP request/response pair. Fix: stop reading without destroying (e.g. collect via 'data' events and pause/unpipe on limit, as raw-body does) so the 413 can be written before the connection closes.
Extended reasoning...
A client sends a >4 MiB POST with Transfer-Encoding: chunked (no Content-Length — e.g. a streaming HTTP client or an intermediary that strips Content-Length) to the documented Node mounting app.all('/mcp', toNodeHandler(handler)) or plain node:http. Once 4 MiB+1 bytes arrive, the throw at line 269 aborts the for-await, Node destroys req and its socket (IncomingMessage._destroy destroys the socket for an incomplete request), and the res.writeHead(413)/res.end() in the catch writes into the destroyed socket. The client observes ECONNRESET / empty reply instead of the documented 413 JSON-RPC error ({code:-32000, 'Payload Too Large...'}), so it cannot distinguish payload-too-large from a server crash; the changeset's and PR's promised 413 behavior is never delivered on the chunked path (the Content-Length pre-check path at line 260 works because it throws before any read starts).
Verification: normal — the mid-stream throw destroys the real request socket before the 413 is written, so the response the catch block builds never reaches the client on the chunked path. Mechanics, from /home/claude/typescript-sdk/packages/middleware/node/src/toNodeHandler.ts: - Lines 266-270: `for await (const chunk of req) { ... if (received > MAX_REQUEST_BODY_SIZE) { throw new RequestBodyTooLargeError(); }
…tBodySize Add a maxRequestBodySize option (bytes, default 4 MiB) to WebStandardStreamableHTTPServerTransport, createMcpHandler (forwarded to its stateless legacy leg; isLegacyRequest and legacyStatelessFallback take the same option), createMcpHonoApp, and toNodeHandler/toWebRequest, so hosts that accept larger JSON-RPC bodies can raise the bound instead of pre-parsing the body themselves. Invalid values (0, negative, NaN, non-finite) throw a RangeError at construction. DEFAULT_MAX_REQUEST_BODY_SIZE and the bounded readRequestBody are exported from @modelcontextprotocol/server; the Hono adapter uses them instead of its private copy and no longer keeps the decoded body text alive across next(). The rejection toWebRequest produces for an over-limit body now carries name 'RequestBodyTooLargeError' and status 413, so hand-wired callers can recognise it without matching message text. createMcpExpressApp runs its Host/Origin validation before express.json(), the ordering createMcpHonoApp already uses, so a request from a disallowed origin is answered 403 without its body being read. Changeset bumped to minor for server/node/hono (new option), patch for express.
There was a problem hiding this comment.
Beyond the inline findings on this push, two other candidates were examined and ruled out: the 400/-32700 "Parse error" mapping in handlePostRequest's catch around the new bounded body read predates this PR (the rewrite preserves existing behavior rather than introducing the miscoding), and the 100-message batch cap remaining non-configurable while the byte limit gained maxRequestBodySize matches the PR's stated scope (batching left the protocol in 2025-06-18), so it was not flagged.
Extended reasoning...
This is a findings-present run on a previously reviewed PR: three new nits are posted inline (unvalidated maxBytes in the newly public readRequestBody, missing docs/ guide updates for the new 413/400 behavior and maxRequestBodySize option, and the option-validation logic copy-pasted across three packages). The note above records only what was newly examined and ruled out this run — the pre-existing -32700 parse-error mapping in packages/server/src/server/streamableHttp.ts and the deliberate non-configurability of MAX_BATCH_SIZE — without restating the inline findings or the prior review's comments. The new commit making the limit configurable responds to the earlier configurability finding; remaining prior comments (e.g. the throw-from-for-await read path in toWebRequest, which the new commit keeps) were already posted and are not repeated.
| export async function readRequestBody( | ||
| request: Request, | ||
| maxBytes: number = DEFAULT_MAX_REQUEST_BODY_SIZE | ||
| ): Promise<{ tooLarge: true } | { tooLarge: false; text: string }> { | ||
| if (Number(request.headers.get('content-length')) > maxBytes) { |
There was a problem hiding this comment.
🟡 nit: the newly exported public readRequestBody never validates maxBytes, so a NaN (or Infinity) bound silently disables the size limit instead of failing fast. Both guards — Number(request.headers.get('content-length')) > maxBytes (line 37) and received > maxBytes (line 54) — evaluate false for NaN/Infinity, so the loop reads the entire body unbounded. Every sibling entry point that accepts the same knob (WebStandardStreamableHTTPServerTransport ctor, createMcpHandler, legacyStatelessFallback, isLegacyRequest, createMcpHonoApp, toNodeHandler/toWebRequest) throws RangeError for non-finite values via resolveMaxRequestBodySize, but that resolver is deliberately not applied inside readRequestBody itself, and it is not exported — yet readRequestBody IS exported from…
Extended reasoning...
An adapter author follows the index.ts guidance and calls the public readRequestBody(request, maxBytes) with a config-derived bound, e.g. Number(process.env.MAX_BODY) when the env var is unset or malformed, yielding NaN. Unlike every other maxRequestBodySize entry point in this PR (which throws RangeError at construction for the same input), readRequestBody raises no error and applies no bound: the Content-Length fast path and the streamed received > maxBytes check are both false against NaN, so a multi-gigabyte POST body is buffered into the text string in full — the exact memory-exhaustion DoS this new API exists to prevent — while the author believes a limit is in force.
Verification: nit — the described behavior is real in the code. readRequestBody (/home/claude/typescript-sdk/packages/server/src/server/requestBody.ts:33-63) performs no validation of maxBytes: with maxBytes = NaN, both guards — line 37 if (Number(request.headers.get('content-length')) > maxBytes) and line 54 if (received > maxBytes) — evaluate false (any comparison against NaN is false), so the loop
| so the modern handler answers it. JSON-RPC batch arrays are limited to 100 messages; a longer | ||
| batch is answered `400` / `-32600` and none of it is dispatched. | ||
|
|
||
| The limit is configurable with a new `maxRequestBodySize` option (bytes, default |
There was a problem hiding this comment.
🟡 nit: new user-facing behavior (default 4 MiB body cap answering 413, 100-message batch cap answering 400) and the new maxRequestBodySize option on four public surfaces are documented only in this changeset and JSDoc — no docs/ guide page is updated. REVIEW.md's checklist explicitly requires prose documentation (not just JSDoc) for new features and a check that existing docs don't omit new behavior; the serving guides are exactly where the sibling knobs already live: docs/serving/legacy-clients.md:96 tells users to raise jsonLimit to '4mb' because "the SSE transport itself accepts messages up to 4mb", and docs/serving/express.md… [also at: packages/server/src/index.ts:78 - nit: new public feature ships with zero prose documentation under docs/ — the maxRequestBodySize option (added to…]
Extended reasoning...
An operator whose MCP server accepts large tool arguments (e.g. base64 images) follows docs/serving/express.md or docs/serving/http.md to deploy after upgrading; clients start receiving 413 for >4 MiB POSTs. The guides they were told to follow never mention the limit, the maxRequestBodySize option, or that on the toNodeHandler path both the adapter's and the handler's bound must be raised in step, so they either misdiagnose the 413 as a proxy problem or raise only CreateMcpHandlerOptions.maxRequestBodySize and still get 413 from the adapter's default bound. The only discoverable documentation is a CHANGELOG entry and per-symbol JSDoc.
Verification: nit. The omission is real. The diff stat shows no docs/** file is touched (13 changed files, all under .changeset/ and packages/), yet the change introduces user-facing behavior and a new public option on four surfaces: .changeset/request-body-size-limit.md states "now stops at 4 MiB by default ... and answers 413 Payload Too Large", "JSON-RPC batch arrays are limited to 100 messages;
| } | ||
|
|
||
| function resolveMaxRequestBodySize(value: number | undefined): number { | ||
| if (value === undefined) { | ||
| return DEFAULT_MAX_REQUEST_BODY_SIZE; | ||
| } | ||
| if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) { | ||
| throw new RangeError(`maxRequestBodySize must be a positive number of bytes, got ${String(value)}`); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
🟡 nit: maxRequestBodySize validation logic copy-pasted across three packages instead of exporting the canonical helper sweep:maxRequestBodySize must be a positive number of bytes — packages/server/src/server/requestBody.ts exports resolveMaxRequestBodySize internally, but @ modelcontextprotocol/node re-implements the whole function (toNodeHandler.ts:44-51) and @ modelcontextprotocol/hono inlines the same check (hono.ts:80-83); export resolveMaxRequestBodySize from @ modelcontextprotocol/server (alongside the already-exported readRequestBody/DEFAULT_MAX_REQUEST_BODY_SIZE) and delete the copies.
Extended reasoning...
The three copies drift: e.g. the server later tightens resolveMaxRequestBodySize (say, rejecting non-integers or changing the error type/message that tests and callers match on) while the node and hono copies keep the old behavior, so createMcpHonoApp/toNodeHandler accept a value the paired WebStandardStreamableHTTPServerTransport rejects with RangeError at construction — an app that passes the same option to adapter and transport throws from one layer but not the other, and the adapter's documented validation contract silently diverges from the SDK's.
Verification: nit — the duplication is real and verbatim. The canonical helper lives in /home/claude/typescript-sdk/packages/server/src/server/requestBody.ts:17-25 (export function resolveMaxRequestBodySize(...) throwing RangeError('maxRequestBodySize must be a positive number of bytes, got ...')), but it is deliberately kept off the public surface — packages/server/src/index.ts:78 exports only `DEFAULT_MAX
A minor on @modelcontextprotocol/server would bump every adapter package that peer-depends on it to a new major under the current changesets config; additive options have shipped as patch here before (stdio maxBufferSize, isJsonContentType).
| if (body.tooLarge) { | ||
| const message = requestBodyTooLargeMessage(this._maxRequestBodySize); | ||
| this.onerror?.(new Error(message)); | ||
| return this.createJsonErrorResponse(413, -32_000, message); | ||
| } |
There was a problem hiding this comment.
🟡 nit: the new 413 path in handlePost calls this.onerror inside the same try whose catch hard-codes 400/-32700 'Parse error: Invalid JSON', so a throwing user onerror callback converts the documented 413 Payload Too Large answer into a misleading 400 Parse error; the sibling adapter (toNodeHandler.ts) wraps its onerror call in try/catch with the comment 'Reporting must never alter the response', but this new site does not.
Extended reasoning...
An operator sets transport.onerror to a logger that can throw (e.g. a reporting call that rejects/throws synchronously). A client posts a >4 MiB body: readRequestBody returns tooLarge, onerror(new Error('Payload Too Large...')) throws, the surrounding catch fires, onerror is invoked a second time with the secondary error, and the client receives 400 -32700 'Parse error: Invalid JSON' instead of the 413/-32000 the changeset and JSDoc promise — the client mis-diagnoses its request as malformed JSON rather than oversized.
Verification: nit: Real but edge-case. streamableHttp.ts lines 777-788 (added by this diff): this.onerror?.(new Error(message)); at line 781 sits inside the inner try whose catch (785-787) returns this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON') — a throwing user onerror callback aborts before the return ...413... at line 782 and the client receives 400/-32700 instead of the documen
2 participants
The Streamable HTTP transport, the
createMcpHandlerentry, the Node adapter'stoWebRequest, and the Hono app's JSON pre-parse each read the whole POST body into memory before doing anything with it, and the transport accepts a JSON-RPC batch array of any length. This reads bodies with a 4 MiB limit at all four (answering413past it) and caps batch arrays at 100 messages (400).Motivation and Context
Every other body read in the SDK already has a ceiling:
createMcpExpressAppgoes throughexpress.json()(100 kb default), the Fastify adapter through Fastify's 1 MiBbodyLimit, the legacy SSE transport reads at'4mb', and stdio framing stops atSTDIO_DEFAULT_MAX_BUFFER_SIZE. The web-standard transport and the 2.x entry points had none, so how much a single request could make the server buffer depended on which adapter it arrived through. They now use the SSE transport's 4 MiB.WebStandardStreamableHTTPServerTransport(and the Node transport wrapping it) reads through a small private helper: a declaredContent-Lengthover the limit is refused without reading; otherwise reading stops once 4 MiB is crossed, so chunked bodies are covered too. Unchanged whenparsedBodyis supplied.400/-32600before parsing any element (read path andparsedBody). Batching left the protocol in 2025-06-18; 100 leaves older clients plenty of room.createMcpHandleruses the same helper, so an over-limit body is answered before a server instance is created;isLegacyRequestreturnsfalsefor it.toWebRequestapplies the same limit to what it buffers from the Node stream and rejects past it;toNodeHandleranswers that with413andConnection: closeinstead of its generic500.createMcpHonoAppnow validates Host/Origin before its JSON pre-parse, and the pre-parse does the same bounded read (its own copy of the ~20-line reader, since nothing new is exported;hono/body-limitre-wrapsc.req.raw, which fails under@hono/node-serverwithoverrideGlobalObjects: false).Requests under the limits behave exactly as before. A host that needs to accept larger bodies can already parse the body itself and pass
parsedBody; the SDK then reads nothing and applies no size limit (the batch bound still applies).Making the limit configurable (deliberately not in this PR)
The limits are private constants for now. If a knob is wanted, the shape I'd suggest:
maxRequestBodySize?: number(bytes, default 4 MiB, same JSDoc everywhere) onWebStandardStreamableHTTPServerTransportOptions(the Node transport shares it),CreateMcpHandlerOptions(forwarded to its stateless fallback the waykeepAliveMsis), andCreateMcpHonoAppOptions(the counterpart of Express'sjsonLimit). Plus an exportedDEFAULT_MAX_REQUEST_BODY_SIZE, mirroringSTDIO_DEFAULT_MAX_BUFFER_SIZE. Name follows stdio'smaxBufferSize; reject0/negative/NaNat construction.ToNodeHandlerOptionsthat users would have to raise in step, havetoWebRequestpass the Node stream through as theRequestbody instead of buffering it to a string (what@hono/node-serveralready does for the sibling transport). The adapter then reads nothing and the handler's limit is the only one on that path.isJsonContentTypewas in fix(server): validate Content-Type by parsed media type instead of substring match #2441, so the hono package can drop its copy.Purely additive; roughly +50 lines of src.
How Has This Been Tested?
streamableHttp.test.ts,createMcpHandler.test.ts,toNodeHandler.test.ts,hono.test.ts), each failing onmainfirst: declared and streamed over-limit bodies get413without the stream being pulled further; a 101-message batch gets400with nothing dispatched (read path andparsedBody); the413arrives before the server factory /handler.fetchis called; the Hono app rejects a disallowedHostwithout reading the body.@hono/node-serverwith and withoutoverrideGlobalObjects): exactly 4194304 bytes served, 4194305 refused, under bothContent-Lengthand chunked framing; batch of 100 served, 101 refused; memory flat while a client keeps streaming after the413.pnpm check:all, every package's unit suite,test-e2e, and the server/client conformance runs pass locally.Breaking Changes
No API changes: no new exports, options, or signatures. Behavioural differences, all for requests no SDK client sends:
413from the transports (when they read the body themselves),createMcpHandler,toNodeHandler, andcreateMcpHonoApp's JSON pre-parse;toWebRequestrejects instead of resolving, so hand-wired callers should catch that or pass a pre-parsed body.400, including arrays passed asparsedBody(so through the Express and Fastify adapters too).createMcpHonoApp: a disallowedHost/Originwith an invalid JSON body is now403rather than400, since validation runs first.Types of changes
Checklist
AI Disclaimer