fix(core): resync ReadBuffer at the next message boundary after an oversized message - #2793
Draft
BerkantACUN wants to merge 1 commit into
Draft
Conversation
…ersized message On overflow the buffer was cleared and reading continued, but the rest of the oversized message was still arriving: it landed in the empty buffer and was fed to the parser as if it were a new message, and a large enough remainder overflowed a second time. The remainder is now dropped, unbuffered, up to and including its newline, and parsing resumes with whatever follows. One oversized message, one error. Second point of modelcontextprotocol#2775.
🦋 Changeset detectedLatest commit: 2b0fed1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
9 tasks
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
mysleekdesigns
added a commit
to mysleekdesigns/crawlforge-mcp
that referenced
this pull request
Sep 13, 2026
…audit the real tool R23 live testing found three defects, all reachable from ordinary use. A full-page screenshot of a long page produced a resource that exceeded the MCP stdio ReadBuffer, and since SDK 1.30 an oversized message CLOSES the transport: the whole session died, every tool with it, and the overflow is not recoverable (modelcontextprotocol/typescript-sdk#2793). en.wikipedia.org/wiki/World_War_II is 61,341px tall and shoots an 18.5 MB PNG; JPEG q80 only reaches 11.3 MB, so neither format saved the caller. ResourceRegistry now budgets the blob a read may emit at three-quarters of the ceiling less the envelope (7,077,888 bytes, CRAWLFORGE_MAX_RESOURCE_BLOB_BYTES) and refuses an oversized read with the size, the limit and the remedy; storeScreenshot reports the size so the screenshot result carries `bytes` and, over budget, a warning at capture time rather than one wasted read later. ActionExecutor awaited browserPreflight and threw away what it returned, so the warning the shared respect_robots description promises ("returns a warning in the response") never reached a browser_session or scrape_with_actions caller, though `scrape` published it. The warnings are now stamped on the page beside __crawlforgeNavigation and published by open and act. The same call site hard-coded tool: 'scrape_with_actions', so every session's robots override was filed against the wrong tool in the G5 audit — the record of the customer's decision. The tool name now travels in browserOptions. Verified live: the full-page shot warns and its read is refused while the next call still answers; a viewport shot (322,476 bytes) still reads; the override returns its warning and audits as browser_session. Gates: test:unit 2,280/0 (+6 regression tests), MCP protocol 100%/0 errors, cost parity 31 tools/0 mismatches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BkdGiyFKeav7BC4HBVP4R
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After an oversized message,
ReadBuffercleared itself and kept reading — but the rest of that message was still arriving. It landed in the empty buffer and was fed to the parser as if it were the start of a new message. The remainder is now dropped, unbuffered, up to and including the newline that ends it, and parsing resumes with whatever follows.Motivation and Context
Second point of #2775. On overflow,
append()calledclear()and threw. Two things followed from that:SyntaxErrorskip hides it; onv1.xit surfaces as a secondonerrorfor the same message (Unexpected token 'A', "AAAAAAAAAA"... is not valid JSONin the issue's output), pointing at nothing real.maxBufferSizeof memory for a message already rejected. ThroughStdioClientTransportthis is usually masked, sinceclose()ends the child before the second half lands (I checked with the issue's 20 MB repro: oneonerroronmaintoo), so the evidence for it is at theReadBufferlevel, in the tests.A parser that resumes at a message boundary has neither problem: one oversized message, one error, and the next real message parses.
How it works
_discardingToNewlineis set and subsequent chunks are skipped without being stored until one contains it.maxBufferSizereaches the parser, as before.clear()also cancels the resync, so a transport that resets the buffer on close does not skip the first line of a new stream.onerror, thenclose()) is unchanged. No public API change;ReadBufferiscore-internal.How Has This Been Tested?
Five unit tests on
ReadBuffer: resumes at the next boundary rather than mid-message; drops a multi-chunk remainder without buffering it (the second-overflow case — fails onmain); keeps what follows the boundary inside the chunk that overflowed (fails onmain); never hands the parser more than the limit even after the boundary (fails onmain);clear()abandons the resync. The existing overflow tests still pass, includingshould clear buffer before throwing on overflow, which exercises the same "append again afterwards" contract.core-internal, client and server stdio suites pass;pnpm typecheck:allandpnpm lint:allpass.Companion to #2792, which carries the overflow error into the caller's rejection; independent of it.
Breaking Changes
None.
Types of changes
Checklist
Disclosure: written with Claude Code; I reviewed the diff and ran the suites myself.