Skip to content

Commit 27d8adb

Browse files
BabyChrist666claude
andcommitted
Reject JSON-RPC requests with null id instead of misclassifying as notifications
When a JSON-RPC message contains "id": null, Pydantic's union resolution would silently reclassify it as a JSONRPCNotification (since the Request type correctly rejects null ids, but the Notification type absorbed it as an extra field). This violates the MCP spec which requires request ids to be strings or integers only. Add a model_validator on JSONRPCNotification that rejects any input containing an "id" field, ensuring that malformed requests with null ids produce a proper validation error instead of being silently swallowed. Fixes #2057 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent be5bb7c commit 27d8adb

File tree

2 files changed

+131
-2
lines changed

2 files changed

+131
-2
lines changed

src/mcp/types/jsonrpc.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from typing import Annotated, Any, Literal
66

7-
from pydantic import BaseModel, Field, TypeAdapter
7+
from pydantic import BaseModel, Field, TypeAdapter, model_validator
88

99
RequestId = Annotated[int, Field(strict=True)] | str
1010
"""The ID of a JSON-RPC request."""
@@ -20,12 +20,35 @@ class JSONRPCRequest(BaseModel):
2020

2121

2222
class JSONRPCNotification(BaseModel):
23-
"""A JSON-RPC notification which does not expect a response."""
23+
"""A JSON-RPC notification which does not expect a response.
24+
25+
Per JSON-RPC 2.0, a notification is a request without an ``id`` member.
26+
Messages that include ``id`` (even with a ``null`` value) are requests,
27+
not notifications. The validator below prevents Pydantic's union
28+
resolution from silently absorbing an invalid ``"id": null`` request
29+
as a notification (see :issue:`2057`).
30+
"""
2431

2532
jsonrpc: Literal["2.0"]
2633
method: str
2734
params: dict[str, Any] | None = None
2835

36+
@model_validator(mode="before")
37+
@classmethod
38+
def _reject_id_field(cls, data: Any) -> Any:
39+
"""Reject messages that contain an ``id`` field.
40+
41+
JSON-RPC notifications MUST NOT include an ``id`` member. If the
42+
raw payload contains ``"id"`` (regardless of its value), it is a
43+
malformed request — not a valid notification.
44+
"""
45+
if isinstance(data, dict) and "id" in data:
46+
raise ValueError(
47+
"Notifications must not include an 'id' field. "
48+
"A JSON-RPC message with 'id' is a request, not a notification."
49+
)
50+
return data
51+
2952

3053
# TODO(Marcelo): This is actually not correct. A JSONRPCResponse is the union of a successful response and an error.
3154
class JSONRPCResponse(BaseModel):
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
"""Tests for issue #2057: Requests with "id": null silently misclassified as notifications.
2+
3+
When a JSON-RPC request arrives with ``"id": null``, the SDK should reject it
4+
rather than silently reclassifying it as a ``JSONRPCNotification``. Both
5+
JSON-RPC 2.0 and the MCP spec restrict request IDs to strings or integers.
6+
7+
See: https://github.com/modelcontextprotocol/python-sdk/issues/2057
8+
"""
9+
10+
import json
11+
12+
import pytest
13+
from pydantic import ValidationError
14+
15+
from mcp.types import (
16+
JSONRPCNotification,
17+
JSONRPCRequest,
18+
jsonrpc_message_adapter,
19+
)
20+
21+
22+
class TestNullIdRejection:
23+
"""Verify that ``"id": null`` is never silently absorbed."""
24+
25+
def test_request_rejects_null_id(self) -> None:
26+
"""JSONRPCRequest correctly rejects null id."""
27+
with pytest.raises(ValidationError):
28+
JSONRPCRequest.model_validate(
29+
{"jsonrpc": "2.0", "method": "initialize", "id": None}
30+
)
31+
32+
def test_notification_rejects_id_field(self) -> None:
33+
"""JSONRPCNotification must not accept messages with an 'id' field."""
34+
with pytest.raises(ValidationError, match="must not include an 'id' field"):
35+
JSONRPCNotification.model_validate(
36+
{"jsonrpc": "2.0", "method": "initialize", "id": None}
37+
)
38+
39+
def test_notification_rejects_any_id_value(self) -> None:
40+
"""Notification rejects 'id' regardless of value — null, int, or str."""
41+
for id_value in [None, 0, 1, "", "abc"]:
42+
with pytest.raises(ValidationError):
43+
JSONRPCNotification.model_validate(
44+
{"jsonrpc": "2.0", "method": "test", "id": id_value}
45+
)
46+
47+
def test_message_adapter_rejects_null_id(self) -> None:
48+
"""JSONRPCMessage union must not accept ``"id": null``."""
49+
raw = {"jsonrpc": "2.0", "method": "initialize", "id": None}
50+
with pytest.raises(ValidationError):
51+
jsonrpc_message_adapter.validate_python(raw)
52+
53+
def test_message_adapter_rejects_null_id_json(self) -> None:
54+
"""Same test but via validate_json (the path used by transports)."""
55+
raw_json = json.dumps({"jsonrpc": "2.0", "method": "initialize", "id": None})
56+
with pytest.raises(ValidationError):
57+
jsonrpc_message_adapter.validate_json(raw_json)
58+
59+
def test_valid_notification_still_works(self) -> None:
60+
"""A valid notification (no 'id' field at all) must still parse fine."""
61+
msg = JSONRPCNotification.model_validate(
62+
{"jsonrpc": "2.0", "method": "notifications/initialized"}
63+
)
64+
assert msg.method == "notifications/initialized"
65+
66+
def test_valid_notification_with_params(self) -> None:
67+
"""Notification with params but no 'id' should work."""
68+
msg = JSONRPCNotification.model_validate(
69+
{"jsonrpc": "2.0", "method": "notifications/progress", "params": {"progress": 50}}
70+
)
71+
assert msg.method == "notifications/progress"
72+
assert msg.params == {"progress": 50}
73+
74+
def test_valid_request_with_string_id(self) -> None:
75+
"""A valid request with a string id still works."""
76+
msg = JSONRPCRequest.model_validate(
77+
{"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"}
78+
)
79+
assert msg.id == "abc-123"
80+
81+
def test_valid_request_with_int_id(self) -> None:
82+
"""A valid request with an integer id still works."""
83+
msg = JSONRPCRequest.model_validate(
84+
{"jsonrpc": "2.0", "method": "initialize", "id": 42}
85+
)
86+
assert msg.id == 42
87+
88+
def test_message_adapter_parses_valid_request(self) -> None:
89+
"""The union adapter correctly identifies a valid request."""
90+
raw = {"jsonrpc": "2.0", "method": "initialize", "id": 1}
91+
parsed = jsonrpc_message_adapter.validate_python(raw)
92+
assert isinstance(parsed, JSONRPCRequest)
93+
assert parsed.id == 1
94+
95+
def test_message_adapter_parses_valid_notification(self) -> None:
96+
"""The union adapter correctly identifies a valid notification."""
97+
raw = {"jsonrpc": "2.0", "method": "notifications/initialized"}
98+
parsed = jsonrpc_message_adapter.validate_python(raw)
99+
assert isinstance(parsed, JSONRPCNotification)
100+
assert parsed.method == "notifications/initialized"
101+
102+
def test_message_adapter_parses_notification_json(self) -> None:
103+
"""The union adapter correctly identifies a valid notification via JSON."""
104+
raw_json = json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"})
105+
parsed = jsonrpc_message_adapter.validate_json(raw_json)
106+
assert isinstance(parsed, JSONRPCNotification)

0 commit comments

Comments
 (0)