fix: reject invalid JSON-RPC response versions - #1161
Closed
Shubchynskyi wants to merge 1 commit into
Closed
Shubchynskyi wants to merge 1 commit into
Shubchynskyi wants to merge 1 commit into
Conversation
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.
Superseded by #1162, which preserves the identical source changes with corrected commit attribution. Please review #1162 instead.
Reject success and error responses with an invalid JSON-RPC version. The client currently accepts otherwise valid responses with
"jsonrpc":"1.0"; validatingJSONRPC_VERSION.equals(jsonrpc)in theJSONRPCResponsecompact constructor rejects them while preserving valid"2.0"responses and the existing empty-version diagnostic.Fixes #1156
Motivation and Context
Merge dependency: please merge #1158 before this PR.
On current main, a deserialization exception stops
StdioClientTransport's inbound loop. Once this validation rejects an invalid version, the affected tool call and a subsequent ping time out. That recovery problem is tracked in #1157. #1158 makes the malformed response fail its request immediately and keeps the transport reading, so this PR depends on #1158 for stdio recovery. This change adds only schema validation and regression tests; transport recovery stays in #1158.Add two regression cases through
McpSchema.deserializeJsonRpcMessage: an otherwise valid success response and an otherwise valid error response, each with"jsonrpc":"1.0". The tests check the validation root cause across Jackson exception wrappers. Existing positive assertions are unchanged.How Has This Been Tested?
Locally on Debian 13 amd64, Temurin JDK 21.0.12.1+1, Maven wrapper 3.9.9, commit
c32f385d58ada525d0b234542a7ca36f0100b3b0:JsonRpcDispatchTests, Jackson 3JsonRpcDispatchTests, Jackson 2./mvnw clean test, default Jackson 3 profile./mvnw -pl mcp-test -am -Pjackson2 testgit diff --checkBoth full runs reported zero failures, errors and skipped tests. Counts above sum Maven's per-module summaries. The actual runs also set JVM proxy/CA options and appended
-DsurefireArgLine='-javaagent:/workspace/issue-1156-review/runtime-network-agent.jar -Dissue1156.networkConfig=/workspace/issue-1156-review/runtime-network.properties'for this managed environment. The temporary adapter is outside the patch: it configures the proxy/CA for Testcontainers Node processes and makes Reactor Netty use JVM proxy settings. TLS verification stays enabled; SDK validation, fixtures and assertions are unchanged. Both mapper profiles were verified from the test JVM classpaths.Remaining validation limits:
mcp-testcontrol run on unchanged main (1cf7903935ac6a99ea8920b4ce85a43e4f1d0689), with zero failures/errors and exit 0. Its exact cause remains unresolved.Stdio dependency check: a temporary diagnostic used the locally built SDK. Before this fix, the invalid response was accepted and ping succeeded. With this fix and the current transport, the response was rejected but the tool call and next ping timed out. With this core and the transport from #1158 (
4ca7e6be893fef25f1f0556ca05d531ba5222edc), the tool call failed immediately and the next ping succeeded. This diagnostic is outside the patch and is not a full test run of #1158.Breaking Changes
Responses using a JSON-RPC version other than
"2.0"now fail validation. Compliant responses continue to work; API shape and wire serialization are unchanged.Types of changes
Checklist
No documentation change is needed for this validation fix.
Additional context
Prepared with AI assistance; human review is requested before merging. The patch consists of two files and 24 added lines. Please retain the merge order #1158, then this PR to preserve stdio recovery.