From 27d8adbe83bdf40139f42df3149c04dc2e57da89 Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Tue, 17 Feb 2026 09:46:53 -0500 Subject: [PATCH 1/5] 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 --- src/mcp/types/jsonrpc.py | 27 +++++- tests/issues/test_2057_null_id_rejected.py | 106 +++++++++++++++++++++ 2 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 tests/issues/test_2057_null_id_rejected.py diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index 84304a37c..0afbc6e6f 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -4,7 +4,7 @@ from typing import Annotated, Any, Literal -from pydantic import BaseModel, Field, TypeAdapter +from pydantic import BaseModel, Field, TypeAdapter, model_validator RequestId = Annotated[int, Field(strict=True)] | str """The ID of a JSON-RPC request.""" @@ -20,12 +20,35 @@ class JSONRPCRequest(BaseModel): class JSONRPCNotification(BaseModel): - """A JSON-RPC notification which does not expect a response.""" + """A JSON-RPC notification which does not expect a response. + + Per JSON-RPC 2.0, a notification is a request without an ``id`` member. + Messages that include ``id`` (even with a ``null`` value) are requests, + not notifications. The validator below prevents Pydantic's union + resolution from silently absorbing an invalid ``"id": null`` request + as a notification (see :issue:`2057`). + """ jsonrpc: Literal["2.0"] method: str params: dict[str, Any] | None = None + @model_validator(mode="before") + @classmethod + def _reject_id_field(cls, data: Any) -> Any: + """Reject messages that contain an ``id`` field. + + JSON-RPC notifications MUST NOT include an ``id`` member. If the + raw payload contains ``"id"`` (regardless of its value), it is a + malformed request — not a valid notification. + """ + if isinstance(data, dict) and "id" in data: + raise ValueError( + "Notifications must not include an 'id' field. " + "A JSON-RPC message with 'id' is a request, not a notification." + ) + return data + # TODO(Marcelo): This is actually not correct. A JSONRPCResponse is the union of a successful response and an error. class JSONRPCResponse(BaseModel): diff --git a/tests/issues/test_2057_null_id_rejected.py b/tests/issues/test_2057_null_id_rejected.py new file mode 100644 index 000000000..0b4d78cd5 --- /dev/null +++ b/tests/issues/test_2057_null_id_rejected.py @@ -0,0 +1,106 @@ +"""Tests for issue #2057: Requests with "id": null silently misclassified as notifications. + +When a JSON-RPC request arrives with ``"id": null``, the SDK should reject it +rather than silently reclassifying it as a ``JSONRPCNotification``. Both +JSON-RPC 2.0 and the MCP spec restrict request IDs to strings or integers. + +See: https://github.com/modelcontextprotocol/python-sdk/issues/2057 +""" + +import json + +import pytest +from pydantic import ValidationError + +from mcp.types import ( + JSONRPCNotification, + JSONRPCRequest, + jsonrpc_message_adapter, +) + + +class TestNullIdRejection: + """Verify that ``"id": null`` is never silently absorbed.""" + + def test_request_rejects_null_id(self) -> None: + """JSONRPCRequest correctly rejects null id.""" + with pytest.raises(ValidationError): + JSONRPCRequest.model_validate( + {"jsonrpc": "2.0", "method": "initialize", "id": None} + ) + + def test_notification_rejects_id_field(self) -> None: + """JSONRPCNotification must not accept messages with an 'id' field.""" + with pytest.raises(ValidationError, match="must not include an 'id' field"): + JSONRPCNotification.model_validate( + {"jsonrpc": "2.0", "method": "initialize", "id": None} + ) + + def test_notification_rejects_any_id_value(self) -> None: + """Notification rejects 'id' regardless of value — null, int, or str.""" + for id_value in [None, 0, 1, "", "abc"]: + with pytest.raises(ValidationError): + JSONRPCNotification.model_validate( + {"jsonrpc": "2.0", "method": "test", "id": id_value} + ) + + def test_message_adapter_rejects_null_id(self) -> None: + """JSONRPCMessage union must not accept ``"id": null``.""" + raw = {"jsonrpc": "2.0", "method": "initialize", "id": None} + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_python(raw) + + def test_message_adapter_rejects_null_id_json(self) -> None: + """Same test but via validate_json (the path used by transports).""" + raw_json = json.dumps({"jsonrpc": "2.0", "method": "initialize", "id": None}) + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_json(raw_json) + + def test_valid_notification_still_works(self) -> None: + """A valid notification (no 'id' field at all) must still parse fine.""" + msg = JSONRPCNotification.model_validate( + {"jsonrpc": "2.0", "method": "notifications/initialized"} + ) + assert msg.method == "notifications/initialized" + + def test_valid_notification_with_params(self) -> None: + """Notification with params but no 'id' should work.""" + msg = JSONRPCNotification.model_validate( + {"jsonrpc": "2.0", "method": "notifications/progress", "params": {"progress": 50}} + ) + assert msg.method == "notifications/progress" + assert msg.params == {"progress": 50} + + def test_valid_request_with_string_id(self) -> None: + """A valid request with a string id still works.""" + msg = JSONRPCRequest.model_validate( + {"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"} + ) + assert msg.id == "abc-123" + + def test_valid_request_with_int_id(self) -> None: + """A valid request with an integer id still works.""" + msg = JSONRPCRequest.model_validate( + {"jsonrpc": "2.0", "method": "initialize", "id": 42} + ) + assert msg.id == 42 + + def test_message_adapter_parses_valid_request(self) -> None: + """The union adapter correctly identifies a valid request.""" + raw = {"jsonrpc": "2.0", "method": "initialize", "id": 1} + parsed = jsonrpc_message_adapter.validate_python(raw) + assert isinstance(parsed, JSONRPCRequest) + assert parsed.id == 1 + + def test_message_adapter_parses_valid_notification(self) -> None: + """The union adapter correctly identifies a valid notification.""" + raw = {"jsonrpc": "2.0", "method": "notifications/initialized"} + parsed = jsonrpc_message_adapter.validate_python(raw) + assert isinstance(parsed, JSONRPCNotification) + assert parsed.method == "notifications/initialized" + + def test_message_adapter_parses_notification_json(self) -> None: + """The union adapter correctly identifies a valid notification via JSON.""" + raw_json = json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"}) + parsed = jsonrpc_message_adapter.validate_json(raw_json) + assert isinstance(parsed, JSONRPCNotification) From 4f9bc5a624d80525c4cf113061e85d85bff81018 Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Tue, 17 Feb 2026 09:48:53 -0500 Subject: [PATCH 2/5] Fix pre-commit formatting: inline short dict arguments Co-Authored-By: Claude Opus 4.6 --- tests/issues/test_2057_null_id_rejected.py | 24 ++++++---------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/tests/issues/test_2057_null_id_rejected.py b/tests/issues/test_2057_null_id_rejected.py index 0b4d78cd5..a56ac15bd 100644 --- a/tests/issues/test_2057_null_id_rejected.py +++ b/tests/issues/test_2057_null_id_rejected.py @@ -25,24 +25,18 @@ class TestNullIdRejection: def test_request_rejects_null_id(self) -> None: """JSONRPCRequest correctly rejects null id.""" with pytest.raises(ValidationError): - JSONRPCRequest.model_validate( - {"jsonrpc": "2.0", "method": "initialize", "id": None} - ) + JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) def test_notification_rejects_id_field(self) -> None: """JSONRPCNotification must not accept messages with an 'id' field.""" with pytest.raises(ValidationError, match="must not include an 'id' field"): - JSONRPCNotification.model_validate( - {"jsonrpc": "2.0", "method": "initialize", "id": None} - ) + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) def test_notification_rejects_any_id_value(self) -> None: """Notification rejects 'id' regardless of value — null, int, or str.""" for id_value in [None, 0, 1, "", "abc"]: with pytest.raises(ValidationError): - JSONRPCNotification.model_validate( - {"jsonrpc": "2.0", "method": "test", "id": id_value} - ) + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "test", "id": id_value}) def test_message_adapter_rejects_null_id(self) -> None: """JSONRPCMessage union must not accept ``"id": null``.""" @@ -58,9 +52,7 @@ def test_message_adapter_rejects_null_id_json(self) -> None: def test_valid_notification_still_works(self) -> None: """A valid notification (no 'id' field at all) must still parse fine.""" - msg = JSONRPCNotification.model_validate( - {"jsonrpc": "2.0", "method": "notifications/initialized"} - ) + msg = JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "notifications/initialized"}) assert msg.method == "notifications/initialized" def test_valid_notification_with_params(self) -> None: @@ -73,16 +65,12 @@ def test_valid_notification_with_params(self) -> None: def test_valid_request_with_string_id(self) -> None: """A valid request with a string id still works.""" - msg = JSONRPCRequest.model_validate( - {"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"} - ) + msg = JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"}) assert msg.id == "abc-123" def test_valid_request_with_int_id(self) -> None: """A valid request with an integer id still works.""" - msg = JSONRPCRequest.model_validate( - {"jsonrpc": "2.0", "method": "initialize", "id": 42} - ) + msg = JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": 42}) assert msg.id == 42 def test_message_adapter_parses_valid_request(self) -> None: From 2a0d65d86b42bd74ca5938cc2e5901e9469ff335 Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Tue, 17 Feb 2026 09:51:28 -0500 Subject: [PATCH 3/5] Fix pyright: use concrete dict type for model_validator parameter Co-Authored-By: Claude Opus 4.6 --- src/mcp/types/jsonrpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index 0afbc6e6f..2ca7c468e 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -35,7 +35,7 @@ class JSONRPCNotification(BaseModel): @model_validator(mode="before") @classmethod - def _reject_id_field(cls, data: Any) -> Any: + def _reject_id_field(cls, data: dict[str, Any]) -> dict[str, Any]: """Reject messages that contain an ``id`` field. JSON-RPC notifications MUST NOT include an ``id`` member. If the From a841b3cdc31cdcc40920ec1cc743d9ea9f5ae58f Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Tue, 17 Feb 2026 09:54:05 -0500 Subject: [PATCH 4/5] Fix pyright: remove unnecessary isinstance check With dict[str, Any] type annotation, isinstance(data, dict) is redundant. Co-Authored-By: Claude Opus 4.6 --- src/mcp/types/jsonrpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index 2ca7c468e..c26142d75 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -42,7 +42,7 @@ def _reject_id_field(cls, data: dict[str, Any]) -> dict[str, Any]: raw payload contains ``"id"`` (regardless of its value), it is a malformed request — not a valid notification. """ - if isinstance(data, dict) and "id" in data: + if "id" in data: raise ValueError( "Notifications must not include an 'id' field. " "A JSON-RPC message with 'id' is a request, not a notification." From 994a2d4857ae185817172a07ce82cff45456616d Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Tue, 17 Feb 2026 19:15:09 -0500 Subject: [PATCH 5/5] refactor: address review feedback on docstrings and test structure - Trim JSONRPCNotification docstring to single-line style matching file convention - Trim validator docstring to single-line; move spec rationale to inline comment - Fix issue reference from :issue:`2057` to #2057 - Switch tests from class to module-level functions (matching tests/issues/ convention) - Reduce to 4 focused tests that verify the fix (drop redundant pre-existing behavior tests) Co-Authored-By: Claude Opus 4.6 --- src/mcp/types/jsonrpc.py | 19 ++--- tests/issues/test_2057_null_id_rejected.py | 87 +++++----------------- 2 files changed, 24 insertions(+), 82 deletions(-) diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index c26142d75..9f9e4318f 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -20,14 +20,7 @@ class JSONRPCRequest(BaseModel): class JSONRPCNotification(BaseModel): - """A JSON-RPC notification which does not expect a response. - - Per JSON-RPC 2.0, a notification is a request without an ``id`` member. - Messages that include ``id`` (even with a ``null`` value) are requests, - not notifications. The validator below prevents Pydantic's union - resolution from silently absorbing an invalid ``"id": null`` request - as a notification (see :issue:`2057`). - """ + """A JSON-RPC notification which does not expect a response.""" jsonrpc: Literal["2.0"] method: str @@ -36,12 +29,10 @@ class JSONRPCNotification(BaseModel): @model_validator(mode="before") @classmethod def _reject_id_field(cls, data: dict[str, Any]) -> dict[str, Any]: - """Reject messages that contain an ``id`` field. - - JSON-RPC notifications MUST NOT include an ``id`` member. If the - raw payload contains ``"id"`` (regardless of its value), it is a - malformed request — not a valid notification. - """ + """Reject payloads containing an 'id' field (notifications must not have one).""" + # Per JSON-RPC 2.0, notifications must not include an "id" member. + # Without this check, Pydantic's union resolution silently absorbs + # invalid "id": null requests as notifications (#2057). if "id" in data: raise ValueError( "Notifications must not include an 'id' field. " diff --git a/tests/issues/test_2057_null_id_rejected.py b/tests/issues/test_2057_null_id_rejected.py index a56ac15bd..7f27fde54 100644 --- a/tests/issues/test_2057_null_id_rejected.py +++ b/tests/issues/test_2057_null_id_rejected.py @@ -14,81 +14,32 @@ from mcp.types import ( JSONRPCNotification, - JSONRPCRequest, jsonrpc_message_adapter, ) -class TestNullIdRejection: - """Verify that ``"id": null`` is never silently absorbed.""" +def test_notification_rejects_id_field() -> None: + """JSONRPCNotification must not accept messages with an 'id' field.""" + with pytest.raises(ValidationError, match="must not include an 'id' field"): + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) - def test_request_rejects_null_id(self) -> None: - """JSONRPCRequest correctly rejects null id.""" - with pytest.raises(ValidationError): - JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) - def test_notification_rejects_id_field(self) -> None: - """JSONRPCNotification must not accept messages with an 'id' field.""" - with pytest.raises(ValidationError, match="must not include an 'id' field"): - JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) +@pytest.mark.parametrize("id_value", [None, 0, 1, "", "abc"]) +def test_notification_rejects_any_id_value(id_value: object) -> None: + """Notification rejects 'id' regardless of value — null, int, or str.""" + with pytest.raises(ValidationError): + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "test", "id": id_value}) - def test_notification_rejects_any_id_value(self) -> None: - """Notification rejects 'id' regardless of value — null, int, or str.""" - for id_value in [None, 0, 1, "", "abc"]: - with pytest.raises(ValidationError): - JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "test", "id": id_value}) - def test_message_adapter_rejects_null_id(self) -> None: - """JSONRPCMessage union must not accept ``"id": null``.""" - raw = {"jsonrpc": "2.0", "method": "initialize", "id": None} - with pytest.raises(ValidationError): - jsonrpc_message_adapter.validate_python(raw) +def test_message_adapter_rejects_null_id() -> None: + """JSONRPCMessage union must not accept ``"id": null``.""" + raw = {"jsonrpc": "2.0", "method": "initialize", "id": None} + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_python(raw) - def test_message_adapter_rejects_null_id_json(self) -> None: - """Same test but via validate_json (the path used by transports).""" - raw_json = json.dumps({"jsonrpc": "2.0", "method": "initialize", "id": None}) - with pytest.raises(ValidationError): - jsonrpc_message_adapter.validate_json(raw_json) - def test_valid_notification_still_works(self) -> None: - """A valid notification (no 'id' field at all) must still parse fine.""" - msg = JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "notifications/initialized"}) - assert msg.method == "notifications/initialized" - - def test_valid_notification_with_params(self) -> None: - """Notification with params but no 'id' should work.""" - msg = JSONRPCNotification.model_validate( - {"jsonrpc": "2.0", "method": "notifications/progress", "params": {"progress": 50}} - ) - assert msg.method == "notifications/progress" - assert msg.params == {"progress": 50} - - def test_valid_request_with_string_id(self) -> None: - """A valid request with a string id still works.""" - msg = JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": "abc-123"}) - assert msg.id == "abc-123" - - def test_valid_request_with_int_id(self) -> None: - """A valid request with an integer id still works.""" - msg = JSONRPCRequest.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": 42}) - assert msg.id == 42 - - def test_message_adapter_parses_valid_request(self) -> None: - """The union adapter correctly identifies a valid request.""" - raw = {"jsonrpc": "2.0", "method": "initialize", "id": 1} - parsed = jsonrpc_message_adapter.validate_python(raw) - assert isinstance(parsed, JSONRPCRequest) - assert parsed.id == 1 - - def test_message_adapter_parses_valid_notification(self) -> None: - """The union adapter correctly identifies a valid notification.""" - raw = {"jsonrpc": "2.0", "method": "notifications/initialized"} - parsed = jsonrpc_message_adapter.validate_python(raw) - assert isinstance(parsed, JSONRPCNotification) - assert parsed.method == "notifications/initialized" - - def test_message_adapter_parses_notification_json(self) -> None: - """The union adapter correctly identifies a valid notification via JSON.""" - raw_json = json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"}) - parsed = jsonrpc_message_adapter.validate_json(raw_json) - assert isinstance(parsed, JSONRPCNotification) +def test_message_adapter_rejects_null_id_json() -> None: + """Same test but via validate_json (the path used by transports).""" + raw_json = json.dumps({"jsonrpc": "2.0", "method": "initialize", "id": None}) + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_json(raw_json)