Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/mcp/types/jsonrpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -26,6 +26,20 @@ class JSONRPCNotification(BaseModel):
method: str
params: dict[str, Any] | None = None

@model_validator(mode="before")
@classmethod
def _reject_id_field(cls, data: dict[str, Any]) -> dict[str, Any]:
"""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. "
"A JSON-RPC message with 'id' is a request, not a notification."
)
return data
Comment on lines +29 to +41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to validate a key from another structure in this one.



# TODO(Marcelo): This is actually not correct. A JSONRPCResponse is the union of a successful response and an error.
class JSONRPCResponse(BaseModel):
Expand Down
45 changes: 45 additions & 0 deletions tests/issues/test_2057_null_id_rejected.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"""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,
jsonrpc_message_adapter,
)


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})


@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_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() -> 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)