From f091ecc71dac837914c14eaa28c10f4dae4a0073 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 14 Oct 2026 00:36:06 +0200 Subject: [PATCH] [Client] Fix native JsonException escaping for unencodable messages --- CHANGELOG.md | 1 + docs/client/errors.md | 2 +- src/Client.php | 8 ++++- src/Client/Protocol.php | 21 ++++++++++-- tests/Unit/Client/ProtocolTest.php | 51 ++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64fb49a6..fbf45925 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to `mcp/sdk` will be documented in this file. ----- * [BC Break] `DnsRebindingProtectionMiddleware` always validates `Host` (a missing one is rejected) and checks `Origin` separately against the new `allowedOrigins`, including scheme and port and empty by default. Same-origin requests are always allowed and both lists accept `['*']`. `allowedOrigins` is the new second constructor argument; pass the factories by name. +* Fix the client throwing a native `\JsonException` for request params it cannot encode (e.g. invalid UTF-8): it now throws `InvalidArgumentException`, and a request handler result it cannot encode is answered with an internal error instead of no answer. * Fix concurrent requests of one session overwriting each other's session changes: `Session::save()` now writes only the keys it changed, onto what the store holds at that point. * [BC Break] `SchemaValidator` takes an optional `Opis\JsonSchema\Validator` as its first constructor argument, moving `$logger` to second. Pass `logger:` by name. * [BC Break] `SchemaValidator::validateAgainstJsonSchema()` no longer validates an empty array as an object. Pass `new \stdClass()` for an empty object. diff --git a/docs/client/errors.md b/docs/client/errors.md index 4995a4d1..80b63eb1 100644 --- a/docs/client/errors.md +++ b/docs/client/errors.md @@ -9,7 +9,7 @@ all client errors in one place. All classes below live in the `Mcp\Exception` na | `RequestException` | the server answers with an error, the request reaches the default request timeout (code `-32603`, message `Request timed out`), the server answers with an HTTP error status, or the STDIO server process exits while you wait for an answer | | `TimeoutException` | the `timeoutSeconds` deadline of a `callTool()` call expires | | `RequestCancelledException` | the cancellation token of a `callTool()` call is canceled, see [Cancellation and deadlines](transports.md#cancellation-and-deadlines) | -| `InvalidArgumentException` | `callTool()` gets a `timeoutSeconds` that is not finite and positive, `readResource()` gets an empty URI, the builder gets an invalid timeout, retry count or fallback revision, or the server returns a malformed result (also from `connect()`) | +| `InvalidArgumentException` | `callTool()` gets a `timeoutSeconds` that is not finite and positive, `readResource()` gets an empty URI, the builder gets an invalid timeout, retry count or fallback revision, the request can't be encoded as JSON (for example invalid UTF-8 in tool arguments), or the server returns a malformed result (also from `connect()`) | | `RuntimeException` | `callTool()` calls a tool with malformed `x-mcp-header` annotations, you call `sendRootsListChanged()` without the `rootsListChanged` capability, or, on `2026-07-28`, the server asks for input (elicitation, sampling, roots) that no request handler answers, or whose callback throws | An HTTP error status or an exited server process fails the request at once. The client doesn't diff --git a/src/Client.php b/src/Client.php index 0f1d5a92..f70fdeee 100644 --- a/src/Client.php +++ b/src/Client.php @@ -285,6 +285,8 @@ public function listResourceTemplates(?string $cursor = null): ListResourceTempl * @param string $uri The resource URI * @param (callable(float $progress, ?float $total, ?string $message): void)|null $onProgress * Optional callback for progress updates + * + * @throws RequestException|ConnectionException|InvalidArgumentException */ public function readResource(string $uri, ?callable $onProgress = null): ReadResourceResult { @@ -318,6 +320,8 @@ public function listPrompts(?string $cursor = null): ListPromptsResult * @param array $arguments Prompt arguments * @param (callable(float $progress, ?float $total, ?string $message): void)|null $onProgress * Optional callback for progress updates + * + * @throws RequestException|ConnectionException|InvalidArgumentException */ public function getPrompt(string $name, array $arguments = [], ?callable $onProgress = null): GetPromptResult { @@ -333,6 +337,8 @@ public function getPrompt(string $name, array $arguments = [], ?callable $onProg * * @param PromptReference|ResourceReference $ref The prompt or resource reference * @param array{name: string, value: string} $argument The argument to complete + * + * @throws RequestException|ConnectionException|InvalidArgumentException */ public function complete(PromptReference|ResourceReference $ref, array $argument): CompletionCompleteResult { @@ -397,7 +403,7 @@ public function sendRootsListChanged(): void * * @return Response * - * @throws RequestException|ConnectionException + * @throws RequestException|ConnectionException|InvalidArgumentException */ private function sendRequest(Request $request, ?callable $onProgress = null, ?CancellationTokenInterface $cancellation = null, ?float $timeoutSeconds = null): Response { diff --git a/src/Client/Protocol.php b/src/Client/Protocol.php index 9c187c80..a2637f25 100644 --- a/src/Client/Protocol.php +++ b/src/Client/Protocol.php @@ -23,6 +23,7 @@ use Mcp\Client\Transport\HeaderAwareTransportInterface; use Mcp\Client\Transport\HttpTransport; use Mcp\Client\Transport\TransportInterface; +use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\RequestCancelledException; use Mcp\Exception\TimeoutException; use Mcp\JsonRpc\MessageFactory; @@ -776,6 +777,8 @@ public function sendNotification(Notification $notification): void * envelope on the way out when the revision calls for one. * * @param array $payload + * + * @throws InvalidArgumentException if the payload cannot be encoded as JSON, e.g. invalid UTF-8 */ private function send(array $payload, string $kind): void { @@ -788,7 +791,13 @@ private function send(array $payload, string $kind): void 'method' => $payload['method'] ?? null, ]); - $this->transport?->send(json_encode($payload, \JSON_THROW_ON_ERROR)); + try { + $encoded = json_encode($payload, \JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + throw new InvalidArgumentException(\sprintf('Could not encode the "%s" %s as JSON: %s', $payload['method'] ?? 'unknown', $kind, $e->getMessage()), previous: $e); + } + + $this->transport?->send($encoded); } /** @@ -817,7 +826,15 @@ private function sendResponse(Response|Error $response): void { $this->logger->debug('Sending response', ['id' => $response->getId()]); - $encoded = json_encode($response, \JSON_THROW_ON_ERROR); + try { + $encoded = json_encode($response, \JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + // The server still awaits an answer, so it gets an error instead. + $this->logger->warning('Could not encode the response', ['id' => $response->getId(), 'exception' => $e]); + + $encoded = json_encode(Error::forInternalError('Could not encode the response', $response->getId()), \JSON_THROW_ON_ERROR); + } + $this->transport?->send($encoded); } diff --git a/tests/Unit/Client/ProtocolTest.php b/tests/Unit/Client/ProtocolTest.php index ac8a69f8..9180e856 100644 --- a/tests/Unit/Client/ProtocolTest.php +++ b/tests/Unit/Client/ProtocolTest.php @@ -13,10 +13,12 @@ use Mcp\Client\CancellationTokenInterface; use Mcp\Client\Configuration; +use Mcp\Client\Handler\Request\RequestHandlerInterface; use Mcp\Client\Protocol; use Mcp\Client\State\ClientStateInterface; use Mcp\Client\Transport\TransportInterface; use Mcp\Exception\ConnectionException; +use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\LogicException; use Mcp\Exception\RequestCancelledException; use Mcp\Exception\TimeoutException; @@ -26,6 +28,7 @@ use Mcp\Schema\Implementation; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\MessageInterface; +use Mcp\Schema\JsonRpc\Request; use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Request\CallToolRequest; use Mcp\Schema\Request\PingRequest; @@ -625,6 +628,54 @@ public function testProgressAfterCompletionIsDropped(): void $this->assertSame(['prog-1'], $seen); } + #[TestDox('a request that cannot be encoded as JSON fails with an SDK exception and leaves the client usable')] + public function testUnencodableRequestThrowsInvalidArgument(): void + { + $transport = new InterruptingTransport(); + $protocol = new Protocol(); + $protocol->connect($transport, $this->createConfiguration(ProtocolVersion::V2025_11_25)); + + try { + $protocol->request(new CallToolRequest('echo', ['text' => "\xB1"]), 5); + $this->fail('A request with invalid UTF-8 must not reach the transport.'); + } catch (InvalidArgumentException $e) { + $this->assertStringContainsString('"tools/call"', $e->getMessage()); + $this->assertInstanceOf(\JsonException::class, $e->getPrevious()); + } + + $this->assertSame([], $transport->messages); + $this->assertSame([], $protocol->getState()->getPendingRequests()); + $this->assertSame('fast', $this->toolText($protocol->request(new CallToolRequest('fast', []), 5))); + } + + #[TestDox('a request handler result that cannot be encoded as JSON is answered with an internal error')] + public function testUnencodableResponseIsAnsweredWithInternalError(): void + { + $handler = new class implements RequestHandlerInterface { + public function supports(Request $request): bool + { + return true; + } + + public function handle(Request $request): Response + { + return new Response($request->getId(), ['text' => "\xB1"]); + } + }; + + $transport = new InterruptingTransport(); + $protocol = new Protocol([$handler], logger: $logger = new CollectingLogger()); + $protocol->connect($transport, $this->createConfiguration(ProtocolVersion::V2025_11_25)); + + $protocol->processMessage('{"jsonrpc":"2.0","id":7,"method":"ping"}'); + + $this->assertCount(1, $transport->messages); + $this->assertSame(7, $transport->messages[0]['id']); + $this->assertSame(Error::INTERNAL_ERROR, $transport->messages[0]['error']['code'] ?? null); + $this->assertCount(1, $logger->warnings); + $this->assertInstanceOf(\JsonException::class, $logger->warnings[0]['exception'] ?? null); + } + private function createConfiguration(ProtocolVersion $protocolVersion, ?ProtocolVersion $fallback = ProtocolVersion::V2025_11_25): Configuration { return new Configuration(