Skip to content

Commit e4d6db4

Browse files
g97iulio1609Copilot
andcommitted
fix: reject JSON-RPC requests with id: null instead of misclassifying as notification
When a JSON-RPC message arrives with "id": null, it should be rejected. Both JSON-RPC 2.0 and the MCP spec restrict request IDs to strings or integers. Previously, JSONRPCRequest correctly rejected null IDs, but Pydantic fell through to JSONRPCNotification which silently absorbed the extra "id" field via implicit extra='allow'. The caller got a 202 with no response and no error. Set extra='forbid' on JSONRPCNotification so that any unrecognised field (including "id": null) causes a validation error. Fixes #2057 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 62575ed commit e4d6db4

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

src/mcp/types/jsonrpc.py

Lines changed: 3 additions & 1 deletion
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, ConfigDict, Field, TypeAdapter
88

99
RequestId = Annotated[int, Field(strict=True)] | str
1010
"""The ID of a JSON-RPC request."""
@@ -22,6 +22,8 @@ class JSONRPCRequest(BaseModel):
2222
class JSONRPCNotification(BaseModel):
2323
"""A JSON-RPC notification which does not expect a response."""
2424

25+
model_config = ConfigDict(extra="forbid")
26+
2527
jsonrpc: Literal["2.0"]
2628
method: str
2729
params: dict[str, Any] | None = None

tests/issues/test_2057_null_id.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
"""Test for issue #2057: Requests with "id": null silently misclassified as notifications."""
2+
3+
import pytest
4+
from pydantic import ValidationError
5+
6+
from mcp.types import JSONRPCMessage, JSONRPCNotification, JSONRPCRequest, jsonrpc_message_adapter
7+
8+
9+
class TestNullIdRejection:
10+
"""Verify that JSON-RPC messages with id: null are rejected."""
11+
12+
def test_request_rejects_null_id(self):
13+
"""JSONRPCRequest should reject id: null."""
14+
with pytest.raises(ValidationError):
15+
JSONRPCRequest.model_validate(
16+
{"jsonrpc": "2.0", "method": "initialize", "id": None}
17+
)
18+
19+
def test_notification_rejects_extra_id_field(self):
20+
"""JSONRPCNotification should not absorb an extra 'id' field."""
21+
with pytest.raises(ValidationError):
22+
JSONRPCNotification.model_validate(
23+
{"jsonrpc": "2.0", "method": "initialize", "id": None}
24+
)
25+
26+
def test_message_adapter_rejects_null_id(self):
27+
"""The union adapter should reject messages with id: null entirely."""
28+
with pytest.raises(ValidationError):
29+
jsonrpc_message_adapter.validate_python(
30+
{"jsonrpc": "2.0", "method": "initialize", "id": None}
31+
)
32+
33+
def test_valid_notification_without_id(self):
34+
"""A proper notification (no id field) should still validate."""
35+
msg = jsonrpc_message_adapter.validate_python(
36+
{"jsonrpc": "2.0", "method": "notifications/initialized"}
37+
)
38+
assert isinstance(msg, JSONRPCNotification)
39+
40+
def test_valid_request_with_int_id(self):
41+
"""A proper request with an integer id should still validate."""
42+
msg = jsonrpc_message_adapter.validate_python(
43+
{"jsonrpc": "2.0", "method": "initialize", "id": 1}
44+
)
45+
assert isinstance(msg, JSONRPCRequest)
46+
47+
def test_valid_request_with_string_id(self):
48+
"""A proper request with a string id should still validate."""
49+
msg = jsonrpc_message_adapter.validate_python(
50+
{"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"}
51+
)
52+
assert isinstance(msg, JSONRPCRequest)

0 commit comments

Comments
 (0)