Conversation
…ms instead of 500 The legacy initialize handshake eagerly deserializes its params inside StreamableHttpPostTransport before the message reaches the session's JSON-RPC error handling. A structurally valid JSON-RPC envelope whose initialize params were invalid (e.g. a missing required clientInfo.version) threw a JsonException that bubbled up as an opaque 500 Internal Server Error. Catch that JsonException at the HTTP boundary while the response has not started and emit a conformant JSON-RPC error (400 InvalidParams, -32602) that echoes the request id, mirroring the existing malformed-JSON handling. The Core transport stays HTTP-agnostic. Fixes modelcontextprotocol#788
Author
|
Closing as a duplicate. I missed that #1709 already addresses #788 with the same approach (validate initialize params at the HTTP boundary, return 400 with JSON-RPC InvalidParams echoing the request id, plus a missing clientInfo.version test) — and it's already APPROVED, mergeable, and green. Apologies for the noise; deferring to #1709. Thanks @lntutor / @PranavSenthilnathan. |
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
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
An
initializerequest with a structurally valid JSON-RPC envelope but invalid params (for example a missing requiredclientInfo.version) returned an opaque 500 Internal Server Error instead of a client-facing error. This made it very hard for callers to tell that the problem was in their request body.Root cause
For legacy protocol revisions,
StreamableHttpPostTransport.HandlePostAsynceagerly deserializes theinitializeparams before the message reaches the session's JSON-RPC error handling loop. When the params are invalid,JsonSerializer.Deserializethrows aJsonExceptionthat propagates unhandled throughHandlePostRequestAsyncup to the ASP.NET Core pipeline, producing a raw 500.(Malformed JSON envelopes were already handled at the boundary and returned 400; only the eager
initializeparams path was missing.)Fix
Catch the
JsonExceptionat the HTTP boundary inStreamableHttpHandler.HandlePostRequestAsyncwhile the response has not started, and emit a conformant JSON-RPC error — 400 Bad Request withInvalidParams(-32602) — echoing the request id, mirroring the existing malformed-JSON handling. The Core transport stays HTTP-agnostic (no status-code knowledge leaks into it).Tests
Added
PostInitializeWithMissingRequiredParam_Returns400_InvalidParams_EchoesRequestIdtoStreamableHttpServerConformanceTests, which posts aninitializerequest whoseclientInfoomits the requiredversion. It is red before the fix (500) and green after (400 +InvalidParams, id echoed).Validated the full
ModelContextProtocol.AspNetCore.Testssuite locally onnet8.0,net9.0, andnet10.0(Docker tests included) — all green.Fixes #788
Note
I used an AI assistant (GitHub Copilot) to help investigate the issue, implement the fix, and write the test. I ran the tests myself and verified the results.