-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: prevent responses being sent to wrong client when multiple transports connect #820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: prevent responses being sent to wrong client when multiple transports connect #820
Conversation
When multiple clients connect to a Protocol instance, responses can be sent to the wrong transport because the Protocol class overwrites its transport reference on each new connection. This test demonstrates the bug where: - Client A connects and sends a request - While A's request is processing, Client B connects (overwriting transport) - A's response incorrectly goes to B's transport - B receives both responses, A receives nothing
The Protocol class was overwriting its transport reference when new clients connected, causing responses to be sent to the wrong transport. This fix captures the transport reference when a request arrives and uses that captured reference throughout the request lifecycle. This ensures: - Each request's response is sent back through the correct transport - Multiple clients can connect without interfering with each other - Async request handling works correctly even when new connections arrive
e29eed3 to
abacaa6
Compare
pcarleton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for this
…ing fix Upgrades the MCP SDK to version 1.17.2 which includes a critical fix for transport handling in multi-client scenarios. The previous SDK version had a bug where the Protocol class would overwrite its transport reference, causing responses to be sent to the wrong client when multiple clients were connected. This was particularly problematic for StreamableHTTP transport in production environments. The fix ensures that each request maintains its proper transport context through closure capture, guaranteeing responses are sent to the correct client. All existing tests pass with the new version, confirming compatibility. Related: modelcontextprotocol/typescript-sdk#820
…ing fix (#96) Upgrades the MCP SDK to version 1.17.2 which includes a critical fix for transport handling in multi-client scenarios. The previous SDK version had a bug where the Protocol class would overwrite its transport reference, causing responses to be sent to the wrong client when multiple clients were connected. This was particularly problematic for StreamableHTTP transport in production environments. The fix ensures that each request maintains its proper transport context through closure capture, guaranteeing responses are sent to the correct client. All existing tests pass with the new version, confirming compatibility. Related: modelcontextprotocol/typescript-sdk#820
|
Just adding a note clarifying something that I was confused about. It still isn't 100% safe to reuse a single McpServer for multiple client connections, even with the fix in this PR. In the case of two clients that are reusing each connection for multiple requests, the This PR does solve a specific scenario, where there's only a single overlapping request that's already in-flight before the second connection arrives. Maybe that's how most MCP clients work, rather than reusing a single long-running connection. But even in this scenario, there's still a potential race condition, if the second client connects before the first client sends its request. The actual fix is to follow the updated examples from #377 that create a fresh McpServer for each client. I'm not an expert, so I could be mistaken, but I found a comment from @bhosmer-ant that seems to confirm this conclusion: #343 (comment) |
This PR enhances the Protocol class to correctly handle multiple client connections by ensuring responses are always sent back through their originating transport. Currently, when multiple clients connect to a Protocol instance, responses can be misdirected due to the transport reference being overwritten on each new connection.
Motivation and Context
The Current Behavior
The Protocol class maintains a single
_transportreference that gets updated each timeconnect()is called. This creates a scenario where responses can be sent to a different transport than the one that originated the request:This is not a theoretical edge case - it happens whenever clients connect while requests are being processed.
Community Reports
This behavior has been observed and reported by multiple users:
These reports highlight the challenges developers face when trying to scale MCP servers in production environments.
Why This Must Be Fixed
1. Security Risk
Responses containing sensitive data can be sent to the wrong client. In a scenario where Client A queries private data and Client B connects during processing, Client B receives Client A's confidential response. This is not just a privacy concern but a potential compliance violation (GDPR, HIPAA, etc.).
2. Fundamental Correctness
This violates the basic contract of request-response protocols - that responses return to their originator. No amount of application-level workarounds can reliably prevent this at scale.
3. Architectural Limitation
Current workaround requires one server instance per client, which:
4. Specification Alignment
The MCP architecture documentation states "each client having a 1:1 relationship with a particular server" from the host's perspective - describing the logical relationship where each client connects to its designated server. This architectural principle doesn't require servers to be limited to single transport connections at the implementation level.
Indeed, allowing servers to handle multiple transport connections is common in protocol implementations and is supported in other MCP SDKs like Python.
5. Production Impact
Teams deploying MCP in production environments have encountered this limitation, as evidenced by issues #204 and #243. The current workaround of creating separate server instances per client adds complexity and resource overhead that could be avoided with proper multi-transport support.
How Has This Been Tested?
Test Implementation
Added comprehensive tests in
protocol-transport-handling.test.tsthat demonstrate:Test Scenarios Covered
Reproducing the Bug
Breaking Changes
No breaking changes. This fix is backward compatible and requires no changes to existing code.
Types of changes
Checklist
Additional context
Implementation Approach
The fix is minimal and elegant - we capture the transport reference at request time using a closure:
This ensures each request's lifecycle is bound to its originating transport, regardless of subsequent connections.
Current Workaround vs Proper Fix
Current Workaround (inefficient):
With This Fix (proper multi-client support):
Why This Wasn't Caught Earlier
The bug only manifests when:
Many examples and tests use single clients or synchronous handlers, masking the issue.
Alignment with MCP Architecture
The MCP specification states that hosts manage multiple clients, each with a 1:1 relationship with a server. This fix ensures that relationship is properly maintained at the transport level, allowing a single server process to correctly handle multiple client connections as intended by the architecture.