Skip to content

Commit 3162a4c

Browse files
ShivamShivam
authored andcommitted
fix: reject JSON-RPC requests with null id instead of misclassifying 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 #2057 Github-Issue: #2057
1 parent 7ba41dc commit 3162a4c

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

src/mcp/types/jsonrpc.py

Lines changed: 15 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, Field, TypeAdapter, model_validator
88

99
RequestId = Annotated[int, Field(strict=True)] | str
1010
"""The ID of a JSON-RPC request."""
@@ -26,6 +26,20 @@ class JSONRPCNotification(BaseModel):
2626
method: str
2727
params: dict[str, Any] | None = None
2828

29+
@model_validator(mode="before")
30+
@classmethod
31+
def reject_id_field(cls, data: Any) -> Any:
32+
"""Reject messages that contain an 'id' field.
33+
34+
Per JSON-RPC 2.0, notifications MUST NOT have an 'id' member.
35+
Without this check, a request with an invalid id (e.g. null)
36+
would silently fall through union validation and be misclassified
37+
as a notification.
38+
"""
39+
if isinstance(data, dict) and "id" in data:
40+
raise ValueError("Notifications must not contain an 'id' field")
41+
return data
42+
2943

3044
# TODO(Marcelo): This is actually not correct. A JSONRPCResponse is the union of a successful response and an error.
3145
class JSONRPCResponse(BaseModel):

tests/test_types.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from typing import Any
22

33
import pytest
4+
from pydantic import ValidationError
45

56
from mcp.types import (
67
LATEST_PROTOCOL_VERSION,
@@ -11,6 +12,7 @@
1112
Implementation,
1213
InitializeRequest,
1314
InitializeRequestParams,
15+
JSONRPCNotification,
1416
JSONRPCRequest,
1517
ListToolsResult,
1618
SamplingCapability,
@@ -360,3 +362,42 @@ def test_list_tools_result_preserves_json_schema_2020_12_fields():
360362
assert tool.input_schema["$schema"] == "https://json-schema.org/draft/2020-12/schema"
361363
assert "$defs" in tool.input_schema
362364
assert tool.input_schema["additionalProperties"] is False
365+
366+
367+
def test_jsonrpc_message_rejects_null_id():
368+
"""Requests with 'id': null must not silently become notifications.
369+
370+
Per JSON-RPC 2.0, request IDs must be strings or integers. A null id
371+
should be rejected, not reclassified as a notification (issue #2057).
372+
"""
373+
msg = {"jsonrpc": "2.0", "method": "initialize", "id": None}
374+
with pytest.raises(ValidationError):
375+
jsonrpc_message_adapter.validate_python(msg)
376+
377+
378+
@pytest.mark.parametrize(
379+
"invalid_id",
380+
[None, 1.5, True, False, [], {}],
381+
ids=["null", "float", "true", "false", "list", "dict"],
382+
)
383+
def test_jsonrpc_message_rejects_invalid_id_types(invalid_id: Any):
384+
"""Requests with non-string/non-integer id values must be rejected."""
385+
msg = {"jsonrpc": "2.0", "method": "test", "id": invalid_id}
386+
with pytest.raises(ValidationError):
387+
jsonrpc_message_adapter.validate_python(msg)
388+
389+
390+
def test_jsonrpc_notification_without_id_still_works():
391+
"""Normal notifications (no id field) must still be accepted."""
392+
msg = {"jsonrpc": "2.0", "method": "notifications/initialized"}
393+
parsed = jsonrpc_message_adapter.validate_python(msg)
394+
assert isinstance(parsed, JSONRPCNotification)
395+
396+
397+
def test_jsonrpc_request_with_valid_id_still_works():
398+
"""Requests with valid string or integer ids must still be accepted."""
399+
for valid_id in [1, 0, 42, "abc", "request-1"]:
400+
msg = {"jsonrpc": "2.0", "method": "test", "id": valid_id}
401+
parsed = jsonrpc_message_adapter.validate_python(msg)
402+
assert isinstance(parsed, JSONRPCRequest)
403+
assert parsed.id == valid_id

0 commit comments

Comments
 (0)