Repository navigation
fix(server): read HTTP request bodies with a size limit and bound JSON-RPC batch length #2698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| '@modelcontextprotocol/server': patch | ||
| '@modelcontextprotocol/node': patch | ||
| '@modelcontextprotocol/hono': patch | ||
| --- | ||
|
|
||
| Read Streamable HTTP request bodies with a size limit. Every SDK-owned body read — | ||
| `WebStandardStreamableHTTPServerTransport` (and the Node transport built on it), | ||
| `createMcpHandler`, `toNodeHandler`, and `createMcpHonoApp`'s JSON pre-parse — now stops at | ||
| 4 MiB (the limit the legacy SSE transport already uses; the Express adapter and stdio bound | ||
| their reads too) and answers `413 Payload Too Large` before anything is parsed. | ||
| `toWebRequest` (when it reads the Node stream itself) now rejects once the body exceeds the | ||
| limit, and `toNodeHandler` answers that with `413`; hand-wired callers of `toWebRequest` | ||
| should handle the rejection or pass a pre-parsed body, and `isLegacyRequest` reports such a | ||
| request as non-legacy 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. `createMcpHonoApp` also runs its Host/Origin validation before the JSON | ||
| pre-parse, so a request from a disallowed Host with an invalid JSON body is now answered | ||
| `403` rather than `400`. | ||
|
|
||
| Hosts that need to accept larger JSON-RPC bodies can parse the body themselves and pass it | ||
| as `parsedBody`, in which case the SDK reads nothing and applies no size limit; the batch | ||
| bound applies either way. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,15 @@ | |
| */ | ||
| import type { AuthInfo, McpHandlerRequestOptions } from '@modelcontextprotocol/server'; | ||
|
|
||
| /** Same limit the Streamable HTTP transport applies to request bodies. */ | ||
| const MAX_REQUEST_BODY_SIZE = 4 * 1024 * 1024; | ||
|
|
||
| class RequestBodyTooLargeError extends Error { | ||
|
Check warning on line 34 in packages/middleware/node/src/toNodeHandler.ts
|
||
|
claude[bot] marked this conversation as resolved.
|
||
| constructor() { | ||
| super(`Payload Too Large: Request body must not exceed ${MAX_REQUEST_BODY_SIZE} bytes`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Minimal duck-typed shape of a Node.js `IncomingMessage` accepted by | ||
| * {@linkcode toNodeHandler}. Kept structural so the adapter stays free of | ||
|
|
@@ -121,12 +130,22 @@ | |
| ...(parsedBody !== undefined && { parsedBody }) | ||
| }); | ||
| } catch (error) { | ||
| try { | ||
| opts?.onerror?.(error instanceof Error ? error : new Error(String(error))); | ||
| } catch { | ||
| // Reporting must never alter the response. | ||
| if (error instanceof RequestBodyTooLargeError) { | ||
| // A normal answer, not an adapter failure. `connection: close` has | ||
| // an HTTP/1.1 server end the socket after answering instead of | ||
| // idling a connection whose request stream was left partly unread. | ||
| response = Response.json( | ||
| { jsonrpc: '2.0', error: { code: -32_000, message: error.message }, id: null }, | ||
| { status: 413, headers: { connection: 'close' } } | ||
| ); | ||
| } else { | ||
| try { | ||
| opts?.onerror?.(error instanceof Error ? error : new Error(String(error))); | ||
| } catch { | ||
| // Reporting must never alter the response. | ||
| } | ||
| response = internalServerErrorResponse(echoableRequestId(parsedBody)); | ||
| } | ||
| response = internalServerErrorResponse(echoableRequestId(parsedBody)); | ||
| } | ||
|
|
||
| const headers: Record<string, string> = {}; | ||
|
|
@@ -203,10 +222,11 @@ | |
| * await ((await isLegacyRequest(probe)) ? legacy(req, res) : modern(req, res, req.body)); | ||
| * ``` | ||
| * | ||
| * With no `parsedBody` the Node stream is read to completion — read the body | ||
| * from the returned `Request` afterwards, not from `req`. When a body parser | ||
| * already consumed the stream (`express.json()`), pass the parsed value as | ||
| * `parsedBody` and nothing is read from `req`. | ||
| * With no `parsedBody` the Node stream is read to completion (up to 4 MiB — a | ||
| * longer body rejects) — read the body from the returned `Request` afterwards, | ||
| * not from `req`. When a body parser already consumed the stream | ||
| * (`express.json()`), pass the parsed value as `parsedBody` and nothing is read | ||
| * from `req`. | ||
| */ | ||
| export async function toWebRequest(req: NodeIncomingMessageLike, parsedBody?: unknown, options?: ToWebRequestOptions): Promise<Request> { | ||
| const method = (req.method ?? 'GET').toUpperCase(); | ||
|
|
@@ -237,9 +257,17 @@ | |
| let body: string | undefined; | ||
| if (method !== 'GET' && method !== 'HEAD') { | ||
| if (parsedBody === undefined) { | ||
| if (Number(singleHeaderValue(req.headers['content-length'])) > MAX_REQUEST_BODY_SIZE) { | ||
| throw new RequestBodyTooLargeError(); | ||
| } | ||
| const decoder = new TextDecoder(); | ||
| let collected = ''; | ||
| let received = 0; | ||
| 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(); | ||
| } | ||
|
Check failure on line 270 in packages/middleware/node/src/toNodeHandler.ts
|
||
|
Comment on lines
300
to
+304
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 toWebRequest throws RequestBodyTooLargeError from inside 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 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(); } |
||
| collected += typeof chunk === 'string' ? chunk : decoder.decode(chunk as Uint8Array, { stream: true }); | ||
| } | ||
| collected += decoder.decode(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.