fix: use negotiated protocol version in GET SSE reconnect after initialize#931
Open
suryateja-g13 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
added 2 commits
April 21, 2026 21:40
All three servlet-based server transports called request.getReader() without first setting the character encoding. Per the Jakarta Servlet spec, getReader() defaults to ISO-8859-1 when the Content-Type header has no explicit charset parameter. Since application/json without a charset is valid per RFC 8259, non-ASCII characters in tool names, argument values, and notification data were silently corrupted. The analogous server-side fix was applied to StdioServerTransportProvider in modelcontextprotocol#826 and to the HTTP response path in modelcontextprotocol#881. This commit completes the fix by applying request.setCharacterEncoding("UTF-8") before getReader() in HttpServletStreamableServerTransportProvider, HttpServletSseServerTransportProvider, and HttpServletStatelessServerTransport. Fixes modelcontextprotocol#880
…alize When the server negotiates down to an older protocol version during the initialize handshake, the subsequent GET request to open the SSE stream was still sending the client's latest supported version in the MCP-Protocol-Version header. This caused servers that strictly validate the header (e.g. rmcp) to reject the GET with 400 Bad Request. Root cause: the GET reconnect is triggered inside sendMessage() via reconnect(null).contextWrite(deliveredSink.contextView()) at the point where markInitialized() returns true. At that moment the Reactor context has not yet been populated with NEGOTIATED_PROTOCOL_VERSION by LifecycleInitializer (which runs after sendMessage() completes), so reconnect() falls back to latestSupportedProtocolVersion. Fix: add a negotiatedProtocolVersion AtomicReference to the transport. When markInitialized() returns true, extract the protocolVersion from the initialize response body (available in the AggregateResponseEvent or SseResponseEvent) and store it. The reconnect() method then uses this stored value as a fallback when the Reactor context is not yet populated, which covers the initial GET reconnect. Subsequent reconnects continue to read from the context as before. The existing usesServerSupportedVersion integration test, which had a FIXME acknowledging the bug, now also verifies the GET request uses the negotiated version. Fixes modelcontextprotocol#883
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.
Summary
Fixes #883
After a successful
initializehandshake where the server negotiates down to an older protocol version (e.g.2025-06-18), the subsequent GET request to open the SSE stream was still sending the client's latest supported version inMCP-Protocol-Version. Servers that strictly validate this header (e.g.rmcp) reject the request with400 Bad Request.Motivation and Context
The GET reconnect is triggered inside
sendMessage()at the point wheremarkInitialized()returnstrue:At that moment,
deliveredSink.contextView()does not yet containNEGOTIATED_PROTOCOL_VERSION— it is written byLifecycleInitializer.withInitialization()aftersendMessage()completes. Soreconnect()falls back tolatestSupportedProtocolVersion, sending the wrong version on the GET.Fix
Add a
negotiatedProtocolVersionAtomicReferencetoHttpClientStreamableHttpTransport. WhenmarkInitialized()returnstrue, extract theprotocolVersionfrom the initialize response body (available in theAggregateResponseEventorSseResponseEvent) and store it. Thereconnect()method then uses this stored value as a fallback when the Reactor context is not yet populated, covering the initial GET reconnect. Subsequent reconnects continue to read from the context as before.How Has This Been Tested?
The existing
usesServerSupportedVersionintegration test inHttpClientStreamableHttpVersionNegotiationIntegrationTestsreproduced the exact bug — it even had a// FIXME: Set the correct protocol version on GET /mcpcomment acknowledging it. The test has been updated to also assert that the GET request uses the negotiated version (previously it only checked POST requests to work around the bug).All 144 affected tests pass locally. The 12 failures in the full build are Docker/Testcontainers tests that require a Docker daemon — unrelated to this change and pre-existing on this machine.