From 979a5f9ed90f11f20265a4c490528a88f42d6325 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 02:50:00 +0200 Subject: [PATCH 1/2] [Schema] Omit the id of an error response that never had one An error response whose id could not be read sent `"id": ""`, which claims the peer issued a request with an empty-string id - a different statement from "the id could not be read". JSON-RPC spells the latter as a null id, and a receiver that cannot read the id is exactly the case the code covers. [BC Break] Error::$id accepts null and getId() may return it. All the for*() factories default to null, fromArray() accepts a missing or explicitly-null id, and the member is omitted from the serialized form when absent. --- CHANGELOG.md | 1 + src/Schema/JsonRpc/Error.php | 61 +++++++++++++---------- tests/Unit/JsonRpc/MessageFactoryTest.php | 20 ++++++-- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c02faf30..8c2b0584 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to `mcp/sdk` will be documented in this file. 0.8.0 ----- +* [BC Break] `Mcp\Schema\JsonRpc\Error` accepts `null` as its `$id`, and `getId()` may return it. An error response whose id could not be read now omits the member instead of sending `"id": ""` — which claimed the peer had issued a request with an empty-string id. All the `for*()` factories default to `null`, `fromArray()` accepts a missing or explicitly-null id, and `MessageFactory` decodes both as an id-less error rather than rejecting them. * [BC Break] Drop the SDK-only name pattern on `ResourceDefinition`/`ResourceTemplate` `$name` — the spec allows any string (its own examples use `main.rs` and `Project Files`). URI/URI-template validation is unchanged. * Add `ClientGateway::supportsExtension()`, `Client\Builder::enableExtension()`, and `ClientCapabilities::withExtensions()` so clients can negotiate and check protocol extensions (e.g. MCP Apps) the same way servers already do. [BC Break] `ServerExtensionInterface` is replaced by the side-agnostic `Mcp\Schema\Extension\ExtensionInterface`. * Deprecate Roots, Sampling and Logging per SEP-2577 (protocol revision `2026-07-28`, earliest removal `2027-07-28`). They keep working but using them now triggers a deprecation notice — migrate to tool arguments/resource URIs, a direct LLM provider API, and stderr/OpenTelemetry respectively. diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index 683ed0dc..ca4a8be3 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -57,12 +57,15 @@ class Error implements MessageInterface public const UNSUPPORTED_PROTOCOL_VERSION = -32022; /** - * @param int $code the error type that occurred - * @param string $message a short description of the error - * @param mixed|null $data additional information about the error + * @param string|int|null $id The id of the request this answers. `null` only when it could not be + * read — a malformed body, or a notification that was refused — in which + * case the member is omitted rather than sent as an id nobody issued. + * @param int $code the error type that occurred + * @param string $message a short description of the error + * @param mixed|null $data additional information about the error */ public function __construct( - public readonly string|int $id, + public readonly string|int|null $id, public readonly int $code, public readonly string $message, public readonly mixed $data = null, @@ -77,10 +80,9 @@ final public static function fromArray(array $data): self if (!isset($data['jsonrpc']) || MessageInterface::JSONRPC_VERSION !== $data['jsonrpc']) { throw new InvalidArgumentException('Invalid or missing "jsonrpc" in Error data.'); } - if (!isset($data['id'])) { - throw new InvalidArgumentException('Invalid or missing "id" in Error data.'); - } - if (!\is_string($data['id']) && !\is_int($data['id'])) { + // An error response carrying no id is well-formed: it is what a + // receiver sends when the id could not be read off the request. + if (isset($data['id']) && !\is_string($data['id']) && !\is_int($data['id'])) { throw new InvalidArgumentException('Invalid "id" type in Error data.'); } if (!isset($data['error']) || !\is_array($data['error'])) { @@ -93,45 +95,45 @@ final public static function fromArray(array $data): self throw new InvalidArgumentException('Invalid or missing "message" in Error data.'); } - return new self($data['id'], $data['error']['code'], $data['error']['message'], $data['error']['data'] ?? null); + return new self($data['id'] ?? null, $data['error']['code'], $data['error']['message'], $data['error']['data'] ?? null); } - final public static function forParseError(string $message, string|int $id = ''): self + final public static function forParseError(string $message, string|int|null $id = null): self { return new self($id, self::PARSE_ERROR, $message); } - final public static function forInvalidRequest(string $message, string|int $id = ''): self + final public static function forInvalidRequest(string $message, string|int|null $id = null): self { return new self($id, self::INVALID_REQUEST, $message); } - final public static function forMethodNotFound(string $message, string|int $id = ''): self + final public static function forMethodNotFound(string $message, string|int|null $id = null): self { return new self($id, self::METHOD_NOT_FOUND, $message); } - final public static function forInvalidParams(string $message, string|int $id = '', mixed $data = null): self + final public static function forInvalidParams(string $message, string|int|null $id = null, mixed $data = null): self { return new self($id, self::INVALID_PARAMS, $message, $data); } - final public static function forInternalError(string $message, string|int $id = ''): self + final public static function forInternalError(string $message, string|int|null $id = null): self { return new self($id, self::INTERNAL_ERROR, $message); } - final public static function forServerError(string $message, string|int $id = ''): self + final public static function forServerError(string $message, string|int|null $id = null): self { return new self($id, self::SERVER_ERROR, $message); } - final public static function forResourceNotFound(string $message, string|int $id = ''): self + final public static function forResourceNotFound(string $message, string|int|null $id = null): self { return new self($id, self::RESOURCE_NOT_FOUND, $message); } - final public static function forHeaderMismatch(string $message, string|int $id = ''): self + final public static function forHeaderMismatch(string $message, string|int|null $id = null): self { return new self($id, self::HEADER_MISMATCH, $message); } @@ -142,7 +144,7 @@ final public static function forHeaderMismatch(string $message, string|int $id = final public static function forMissingRequiredClientCapability( string $message, ClientCapabilities $requiredCapabilities, - string|int $id = '', + string|int|null $id = null, ): self { return new self($id, self::MISSING_REQUIRED_CLIENT_CAPABILITY, $message, [ 'requiredCapabilities' => $requiredCapabilities, @@ -159,7 +161,7 @@ final public static function forMissingRequiredClientCapability( final public static function forUnsupportedProtocolVersion( string $requested, array $supported, - string|int $id = '', + string|int|null $id = null, ): self { return new self($id, self::UNSUPPORTED_PROTOCOL_VERSION, 'Unsupported protocol version', [ 'requested' => $requested, @@ -167,7 +169,7 @@ final public static function forUnsupportedProtocolVersion( ]); } - public function getId(): string|int + public function getId(): string|int|null { return $this->id; } @@ -175,7 +177,7 @@ public function getId(): string|int /** * @return array{ * jsonrpc: string, - * id: string|int, + * id?: string|int, * error: array{ * code: int, * message: string, @@ -194,10 +196,17 @@ public function jsonSerialize(): array $error['data'] = $this->data; } - return [ - 'jsonrpc' => MessageInterface::JSONRPC_VERSION, - 'id' => $this->id, - 'error' => $error, - ]; + $data = ['jsonrpc' => MessageInterface::JSONRPC_VERSION]; + + // Omitted, not empty: `"id": ""` claims the sender issued a request + // with an empty-string id, which is a different statement from "the + // id could not be read". + if (null !== $this->id) { + $data['id'] = $this->id; + } + + $data['error'] = $error; + + return $data; } } diff --git a/tests/Unit/JsonRpc/MessageFactoryTest.php b/tests/Unit/JsonRpc/MessageFactoryTest.php index 441a500a..fe2c0d2a 100644 --- a/tests/Unit/JsonRpc/MessageFactoryTest.php +++ b/tests/Unit/JsonRpc/MessageFactoryTest.php @@ -278,13 +278,15 @@ public function testNotificationMethodUsedAsRequest(): void public function testErrorMissingId(): void { + // Well-formed: an error response leaves the member out when the id + // could not be read off the request it answers. $json = '{"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid"}}'; $results = $this->factory->create($json); $this->assertCount(1, $results); - $this->assertInstanceOf(InvalidInputMessageException::class, $results[0]); - $this->assertStringContainsString('id', $results[0]->getMessage()); + $this->assertInstanceOf(Error::class, $results[0]); + $this->assertNull($results[0]->getId()); } public function testErrorMissingCode(): void @@ -365,12 +367,24 @@ public function testResponseWithInvalidIdType(): void $this->assertStringContainsString('id', $results[0]->getMessage()); } - public function testErrorWithInvalidIdType(): void + public function testErrorWithNullId(): void { + // JSON-RPC 2.0 spells the same thing as an explicit null. $json = '{"jsonrpc": "2.0", "id": null, "error": {"code": -32600, "message": "Invalid"}}'; $results = $this->factory->create($json); + $this->assertCount(1, $results); + $this->assertInstanceOf(Error::class, $results[0]); + $this->assertNull($results[0]->getId()); + } + + public function testErrorWithInvalidIdType(): void + { + $json = '{"jsonrpc": "2.0", "id": {"not": "an id"}, "error": {"code": -32600, "message": "Invalid"}}'; + + $results = $this->factory->create($json); + $this->assertCount(1, $results); $this->assertInstanceOf(InvalidInputMessageException::class, $results[0]); $this->assertStringContainsString('id', $results[0]->getMessage()); From e3ff4823951ef26dc873a40b5ece541998f65937 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 09:26:49 +0200 Subject: [PATCH 2/2] Guard null response id in Client/Server Protocol handlers Also fix Error::jsonSerialize()'s phpdoc: data belongs inside error, not beside it. --- src/Client/Protocol.php | 6 ++++++ src/Schema/JsonRpc/Error.php | 2 +- src/Server/Protocol.php | 6 ++++++ tests/Unit/Client/ProtocolTest.php | 13 +++++++++++++ tests/Unit/Server/ProtocolTest.php | 24 ++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/Client/Protocol.php b/src/Client/Protocol.php index e9eabded..47acde7d 100644 --- a/src/Client/Protocol.php +++ b/src/Client/Protocol.php @@ -281,6 +281,12 @@ private function handleResponse(Response|Error $response): void { $requestId = $response->getId(); + if (null === $requestId) { + $this->logger->warning('Received an id-less error response; cannot correlate it to a request.', ['response' => $response->jsonSerialize()]); + + return; + } + $this->logger->debug('Handling response', ['id' => $requestId]); $this->state->storeResponse($requestId, $response->jsonSerialize()); diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index ca4a8be3..4f95f72b 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -181,8 +181,8 @@ public function getId(): string|int|null * error: array{ * code: int, * message: string, + * data?: mixed, * }, - * data?: mixed, * } */ public function jsonSerialize(): array diff --git a/src/Server/Protocol.php b/src/Server/Protocol.php index 5a4e358f..f9f79c35 100644 --- a/src/Server/Protocol.php +++ b/src/Server/Protocol.php @@ -341,6 +341,12 @@ private function handleResponse(Response|Error $response, SessionInterface $sess $messageId = $response->getId(); + if (null === $messageId) { + $this->logger->warning('Received an id-less error response from client; cannot correlate it to a pending request.', ['response' => $response->jsonSerialize()]); + + return; + } + $session->set(self::SESSION_RESPONSES.".{$messageId}", $response->jsonSerialize()); $session->forget(self::SESSION_ACTIVE_REQUEST_META); diff --git a/tests/Unit/Client/ProtocolTest.php b/tests/Unit/Client/ProtocolTest.php index 7545f739..722df9f9 100644 --- a/tests/Unit/Client/ProtocolTest.php +++ b/tests/Unit/Client/ProtocolTest.php @@ -99,6 +99,19 @@ public static function provideUnusableCounterOffers(): iterable yield 'modern revision' => [ProtocolVersion::V2026_07_28->value]; } + #[TestDox('logs and ignores an id-less error response instead of crashing on it')] + public function testIgnoresIdLessErrorResponse(): void + { + $protocol = new Protocol(logger: $logger = new CollectingLogger()); + + $protocol->processMessage(json_encode([ + 'jsonrpc' => MessageInterface::JSONRPC_VERSION, + 'error' => ['code' => -32700, 'message' => 'Parse error'], + ], \JSON_THROW_ON_ERROR)); + + $this->assertCount(1, $logger->warnings); + } + private function createConfiguration(ProtocolVersion $protocolVersion): Configuration { return new Configuration( diff --git a/tests/Unit/Server/ProtocolTest.php b/tests/Unit/Server/ProtocolTest.php index b5f836ea..4b64b4a5 100644 --- a/tests/Unit/Server/ProtocolTest.php +++ b/tests/Unit/Server/ProtocolTest.php @@ -82,6 +82,30 @@ public function testNotificationHandledByMultipleHandlers(): void ); } + #[TestDox('An id-less error response from the client is logged and ignored, not stored under a collapsed session key')] + public function testIdLessErrorResponseIsIgnored(): void + { + $session = $this->createMock(SessionInterface::class); + $session->expects($this->never())->method('set'); + + $this->sessionManager->method('exists')->willReturn(true); + $this->sessionManager->method('createWithId')->willReturn($session); + + $protocol = new Protocol( + requestHandlers: [], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: $this->sessionManager, + ); + + $sessionId = Uuid::v4(); + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "error": {"code": -32700, "message": "Parse error"}}', + $sessionId + ); + } + #[TestDox('A single request is handled only by the first matching handler')] public function testRequestHandledByFirstMatchingHandler(): void {