fix: reject JSON-RPC requests with null id instead of misclassifying as notifications#2251
Open
shivama205 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Open
fix: reject JSON-RPC requests with null id instead of misclassifying as notifications#2251shivama205 wants to merge 2 commits intomodelcontextprotocol:mainfrom
shivama205 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
…as notifications When a JSON-RPC request arrives with "id": null, Pydantic's union validation rejects it from JSONRPCRequest (since RequestId only allows int | str) but then silently falls through to JSONRPCNotification, which absorbs the extra "id" field. This causes the server to return 202 Accepted with no response body — a silent failure that is hard to debug. Add a model_validator on JSONRPCNotification that rejects any input containing an "id" field, since per JSON-RPC 2.0 notifications must not have one. This ensures messages with invalid id values (null, float, bool, etc.) are properly rejected with a validation error instead of being silently reclassified. Closes modelcontextprotocol#2057 Github-Issue: modelcontextprotocol#2057
2 tasks
Bortlesboat
reviewed
Mar 9, 2026
Bortlesboat
left a comment
Bortlesboat
left a comment
There was a problem hiding this comment.
Clean fix, the model_validator approach is the right call here — catches the union fallthrough without touching any valid message paths. Tests look solid too.
Contributor
Author
|
@Bortlesboat thanks for the comment. Could you please help to review and approve the fix. |
|
Happy to vouch for it, but I don't have write access to this repo so my review doesn't count as a formal approval. Hopefully a maintainer picks this up soon — it's a clean fix for a real bug. |
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
2 participants
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.
Closes #2057
Problem
When a JSON-RPC request arrives with
"id": null, the SDK silently reclassifies it as aJSONRPCNotificationand returns202 Acceptedwith no response body. The caller gets no error and no response — a silent failure that is hard to debug.This happens because of Pydantic's union fallthrough:
JSONRPCRequestcorrectly rejects the invalidid, butJSONRPCNotificationabsorbs the extra"id": nullfield since it doesn't define anidfield and Pydantic's default behavior drops unknown fields.Fix
Add a
model_validatoronJSONRPCNotificationthat rejects any input containing anidfield. Per JSON-RPC 2.0, notifications MUST NOT have anidmember, so this is both correct and targeted — it prevents the union fallthrough without affecting any valid messages.This also catches other invalid
idtypes (float, bool, list, dict) that would similarly be misclassified today.Changes
src/mcp/types/jsonrpc.py— addedreject_id_fieldmodel validator toJSONRPCNotificationtests/test_types.py— added regression tests for null/invalid id rejection, plus tests confirming valid requests and notifications still workTest plan
ruff checkpassesruff formatpasses