fix(client): validate Content-Type on GET/resume SSE responses#2520
Draft
lntutor wants to merge 1 commit into
Draft
fix(client): validate Content-Type on GET/resume SSE responses#2520lntutor wants to merge 1 commit into
lntutor wants to merge 1 commit into
Conversation
The standalone-GET and resume paths (_startOrAuthSse) piped the response body into the SSE parser without checking Content-Type, while the POST path already validates it via mediaTypeEssence and throws ClientHttpUnexpectedContent. A proxy/captive-portal/misconfigured server answering the GET with 200 and a non-SSE body (e.g. text/html) was therefore silently swallowed: the parser yielded no events, onerror never fired, and the caller believed it had a live stream. Apply the same content-type check on the GET path and throw ClientHttpUnexpectedContent when the media type is not text/event-stream, matching the Streamable HTTP spec (GET returns text/event-stream or 405). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N6RtoHuxrDqTUo9Mw9h4Cv
🦋 Changeset detectedLatest commit: 8fc12d4 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 |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
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
1 participant
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.
What's broken
StreamableHTTPClientTransport's standalone-GET and resume paths (_startOrAuthSse) pipe the response body into the SSE parser after only checkingresponse.okand handling405— with noContent-Typecheck. The POST path already validates this viamediaTypeEssenceand throwsClientHttpUnexpectedContent.Observable failure
A proxy, captive portal, or misconfigured server answering the GET with
200and a non-SSE body (e.g.text/html) is silently swallowed:EventSourceParserStreamyields no events,onerrornever fires, and the caller (initial listen,resumeStream(), or a resumed request) believes it has a live stream. Per the Streamable HTTP spec the server "MUST either returnContent-Type: text/event-stream... or else return HTTP 405".Fix
Apply the same
mediaTypeEssencecheck on the GET path before_handleSseStream, throwingSdkError(ClientHttpUnexpectedContent)when the media type is nottext/event-stream. The throw flows through the existing catch, surfacing viaonerror(and rejecting the resume promise) instead of being lost.Tests
New test in
packages/client/test/client/streamableHttp.test.ts: a GET returning200 text/htmlnow makes_startOrAuthSsereject withClientHttpUnexpectedContent(before the fix it resolved silently). Full@modelcontextprotocol/clientsuite passes (749 tests), typecheck and lint clean. Changeset included.🤖 Generated with Claude Code