Skip to content

fix(core): resync ReadBuffer at the next message boundary after an oversized message - #2793

Draft
BerkantACUN wants to merge 1 commit into
modelcontextprotocol:mainfrom
BerkantACUN:fix/readbuffer-resync-after-overflow
Draft

fix(core): resync ReadBuffer at the next message boundary after an oversized message#2793
BerkantACUN wants to merge 1 commit into
modelcontextprotocol:mainfrom
BerkantACUN:fix/readbuffer-resync-after-overflow

Conversation

@BerkantACUN

@BerkantACUN BerkantACUN commented Sep 11, 2026

Copy link
Copy Markdown

Draft only because non-collaborators may have one non-draft PR open at a time on this repository; the change is complete and ready for review.

After an oversized message, ReadBuffer cleared 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() called clear() and threw. Two things followed from that:

  • A mid-message tail went through the parser. The next chunk of the same message started a fresh buffer, and its first newline produced a "line" that was the tail of a string nobody asked for. On this branch the SyntaxError skip hides it; on v1.x it surfaces as a second onerror for the same message (Unexpected token 'A', "AAAAAAAAAA"... is not valid JSON in the issue's output), pointing at nothing real.
  • The remainder was re-buffered. A remainder larger than the limit accumulated in the cleared buffer until that overflowed too — a second error and another maxBufferSize of memory for a message already rejected. Through StdioClientTransport this is usually masked, since close() ends the child before the second half lands (I checked with the issue's 20 MB repro: one onerror on main too), so the evidence for it is at the ReadBuffer level, 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

  • On overflow the buffer is dropped and the chunk is scanned for the newline that ends the oversized message. If it is there, whatever follows becomes the start of the next message; if not, _discardingToNewline is set and subsequent chunks are skipped without being stored until one contains it.
  • If what follows the boundary is itself over the limit — two oversized messages in one chunk — it is dropped the same way rather than kept as an unchecked buffer larger than the limit. Nothing larger than maxBufferSize reaches 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.
  • The error thrown is unchanged, so the transports' handling (onerror, then close()) is unchanged. No public API change; ReadBuffer is core-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 on main); keeps what follows the boundary inside the chunk that overflowed (fails on main); never hands the parser more than the limit even after the boundary (fails on main); clear() abandons the resync. The existing overflow tests still pass, including should clear buffer before throwing on overflow, which exercises the same "append again afterwards" contract. core-internal, client and server stdio suites pass; pnpm typecheck:all and pnpm lint:all pass.

Companion to #2792, which carries the overflow error into the caller's rejection; independent of it.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Disclosure: written with Claude Code; I reviewed the diff and ran the suites myself.

…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-bot

changeset-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2b0fed1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/core-internal Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch

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

@pkg-pr-new

pkg-pr-new Bot commented Sep 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2793

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2793

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2793

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2793

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2793

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2793

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2793

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2793

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2793

commit: 2b0fed1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant