Skip to content

mcp: make the ioConn frame cap independent of read boundaries - #1510

Merged
guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
Tangerg:fix/ioconn-frame-limit
Nov 6, 2026
Merged

guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
Tangerg:fix/ioconn-frame-limit

Conversation

@Tangerg

@Tangerg Tangerg commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

frameLimitReader reset its count after each message, but json.Decoder
reads ahead: when it finishes a message it may already hold bytes of the
next one, and those bytes were counted toward the finished message. The
next message could then exceed MaxLineLength by as many bytes as had
been read ahead, so whether it was rejected depended on how the reader
split the stream.

The reader now counts the bytes it has read, and each frame's budget
starts at the decoder's InputOffset, so whether a message fits depends
only on the message. That is what the MaxLineLength documentation
already describes, so the documentation is unchanged. Line terminators
and blank lines before a message count toward it, since they follow the
previous message.

Without the change, four cases of the new table test fail, and the fuzz
test finds a counterexample within a second.

Fixes #1509

AI assistance: prepared with Claude; I reviewed the change and can
explain it.

frameLimitReader reset its count after each message, but json.Decoder
reads ahead: when it finishes a message it may already hold bytes of the
next one, and those bytes were counted toward the finished message. The
next message could then exceed MaxLineLength by as many bytes as had
been read ahead, so whether it was rejected depended on how the reader
split the stream.

The reader now counts the bytes it has read, and each frame's budget
starts at the decoder's InputOffset, so whether a message fits depends
only on the message. That is what the MaxLineLength documentation
already describes, so the documentation is unchanged. Line terminators
and blank lines before a message count toward it, since they follow the
previous message.

Without the change, four cases of the new table test fail, and the fuzz
test finds a counterexample within a second.

Fixes modelcontextprotocol#1509

AI assistance: prepared with Claude; I reviewed the change and can
explain it.
@guglielmo-san
guglielmo-san merged commit df0f295 into modelcontextprotocol:main Nov 6, 2026
9 checks passed
@Tangerg
Tangerg deleted the fix/ioconn-frame-limit branch November 6, 2026 18:57
Tangerg added a commit to Tangerg/go-sdk that referenced this pull request Nov 6, 2026
Sync 6 upstream commits through df0f295: the new
ParseURLElicitationRequiredError (modelcontextprotocol#1495), the tools/call retry on
HeaderMismatch (modelcontextprotocol#1502), notifications without Mcp-Method (modelcontextprotocol#1504), the
cancelable ioConn write lock (modelcontextprotocol#1499), invalid_token in 401 challenges
(modelcontextprotocol#1507) and the ioConn frame cap that no longer depends on read
boundaries (modelcontextprotocol#1510).

modelcontextprotocol#1499 and modelcontextprotocol#1510 are the upstream form of the fork's transport fixes, so
ioConn now matches upstream except that canceling a write already in
progress still closes the connection, because the peer may hold part of
the frame. Upstream's tests replace the fork's duplicates of them; the
fork keeps its test for the in-progress cancellation. New pkg.go.dev
links in the docs use the fork's module path.
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.

mcp: whether MaxLineLength rejects a message depends on how the stream is read

2 participants