From 0bd9da741999ffde61158eb51ecb8b1a4c68d570 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sun, 18 Oct 2026 16:01:20 +0200 Subject: [PATCH] [Server] Accept nested empty objects in tool arguments --- CHANGELOG.md | 1 + src/Capability/Discovery/SchemaValidator.php | 60 +++++++++++++- .../Handler/Request/CallToolHandler.php | 4 +- .../Discovery/SchemaValidatorTest.php | 82 +++++++++++++++++++ .../Handler/Request/CallToolHandlerTest.php | 27 ++++++ 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0baa099..254f80a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix `ClientGateway::progress()` sending another call's progress token when calls of one session run in parallel: the token now comes from the gateway's own request. * [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. +* Fix `tools/call` rejecting an empty object nested in the arguments as an array. `SchemaValidator::validateAgainstJsonSchema()` takes `emptyArrayMayBeObject: true` for data decoded with `assoc: true`. * Fix `SchemaValidator` falling back to opis' wording for camelCase keywords like `minLength`, `multipleOf` or `additionalProperties`, and reporting a pointer relative to the `$ref` target inside `$defs`. Clients now see the SDK's messages and the full pointer. * Add `Builder::setSchemaValidator()` to configure the validator used for `tools/call` input and output, e.g. with a resolver for external `$ref` schemas. * [BC Break] Remove the `providerClass` argument of `#[CompletionProvider]`. Use `provider:`, which takes the same class-string and is now the first positional argument. diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index 5be96bd7..14c2a01d 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -43,12 +43,13 @@ public function __construct( /** * Validates data against a JSON schema. * - * @param mixed $data the data to validate (should generally be decoded JSON) - * @param array|object $schema the JSON Schema definition (as PHP array or object) + * @param mixed $data the data to validate (should generally be decoded JSON) + * @param array|object $schema the JSON Schema definition (as PHP array or object) + * @param bool $emptyArrayMayBeObject whether an empty array may stand for `{}`, as it does after `json_decode(assoc: true)` * * @return list array of validation errors, empty if valid */ - public function validateAgainstJsonSchema(mixed $data, array|object $schema): array + public function validateAgainstJsonSchema(mixed $data, array|object $schema, bool $emptyArrayMayBeObject = false): array { try { // --- Schema Preparation --- @@ -73,6 +74,9 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar try { $result = $validator->validate($dataToValidate, $schemaObject); + while ($emptyArrayMayBeObject && !$result->isValid() && $this->replaceEmptyArraysExpectedAsObject($result->error(), $dataToValidate)) { + $result = $validator->validate($dataToValidate, $schemaObject); + } } catch (\Throwable $e) { $this->logger->error('MCP SDK: JSON Schema validation failed internally.', [ 'exception' => $e, @@ -150,6 +154,56 @@ private function convertDataForValidator(mixed $data): mixed return $data; } + /** + * Turns each empty array that failed only for not being an object into `{}`. + */ + private function replaceEmptyArraysExpectedAsObject(?ValidationError $error, mixed &$data): bool + { + $errors = []; + if (null !== $error) { + $this->collectLeafErrors($error, $errors); + } + + $replaced = false; + foreach ($errors as $leaf) { + $expected = (array) ($leaf->args()['expected'] ?? []); + if ('type' !== $leaf->keyword() || [] !== $leaf->data()->value() || !\in_array('object', $expected, true) || \in_array('array', $expected, true)) { + continue; + } + + $target = &$data; + foreach ($leaf->data()->fullPath() as $key) { + if ($target instanceof \stdClass) { + $target = &$target->{$key}; + } else { + $target = &$target[$key]; + } + } + $target = new \stdClass(); + unset($target); + $replaced = true; + } + + return $replaced; + } + + /** + * @param list $leaves + */ + private function collectLeafErrors(ValidationError $error, array &$leaves): void + { + $subErrors = $error->subErrors(); + if (empty($subErrors)) { + $leaves[] = $error; + + return; + } + + foreach ($subErrors as $subError) { + $this->collectLeafErrors($subError, $leaves); + } + } + /** * Recursively collects leaf validation errors. * diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index d58a8485..b127fb02 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -80,8 +80,8 @@ public function handle(Request $request, SessionInterface $session): Response|Er } $inputSchema = $reference->tool->inputSchema; - // Arguments are always a JSON object, but an empty one decodes to `[]`. - $validationErrors = $this->schemaValidator->validateAgainstJsonSchema([] === $arguments ? new \stdClass() : $arguments, $inputSchema); + // Arguments are decoded with `assoc: true`, so any `{}` in them arrives as `[]`. + $validationErrors = $this->schemaValidator->validateAgainstJsonSchema($arguments, $inputSchema, emptyArrayMayBeObject: true); if (!empty($validationErrors)) { $summaryMessage = "Invalid parameters for tool '{$toolName}': ".self::summarizeValidationErrors($validationErrors); diff --git a/tests/Unit/Capability/Discovery/SchemaValidatorTest.php b/tests/Unit/Capability/Discovery/SchemaValidatorTest.php index 199fce29..ecd37383 100644 --- a/tests/Unit/Capability/Discovery/SchemaValidatorTest.php +++ b/tests/Unit/Capability/Discovery/SchemaValidatorTest.php @@ -324,6 +324,88 @@ public function testEmptyStdClassValidatesAgainstObjectSchema(): void $this->assertSame([], $this->validator->validateAgainstJsonSchema(new \stdClass(), ['type' => 'object'])); } + /** + * @param array $schema + */ + #[DataProvider('provideEmptyArraysDecodedFromObjects')] + public function testEmptyArrayMayBeAnObject(string $json, array $schema): void + { + $data = json_decode($json, true, flags: \JSON_THROW_ON_ERROR); + + $this->assertSame([], $this->validator->validateAgainstJsonSchema($data, $schema, emptyArrayMayBeObject: true)); + } + + /** + * @return iterable}> + */ + public static function provideEmptyArraysDecodedFromObjects(): iterable + { + yield 'root object' => ['{}', ['type' => 'object']]; + yield 'nested object' => ['{"parameters":{}}', [ + 'type' => 'object', + 'properties' => ['parameters' => ['type' => 'object']], + ]]; + yield 'nested array' => ['{"tags":[]}', [ + 'type' => 'object', + 'properties' => ['tags' => ['type' => 'array']], + ]]; + yield 'array item' => ['[{}]', ['type' => 'array', 'items' => ['type' => 'object']]]; + yield 'additionalProperties' => ['{"a":{}}', [ + 'type' => 'object', + 'additionalProperties' => ['type' => 'object'], + ]]; + yield 'local $ref' => ['{"addr":{}}', [ + 'type' => 'object', + 'properties' => ['addr' => ['$ref' => '#/$defs/address']], + '$defs' => ['address' => ['type' => 'object']], + ]]; + yield 'object or array, sent as object' => ['{"p":{}}', [ + 'type' => 'object', + 'properties' => ['p' => ['oneOf' => [['type' => 'object'], ['type' => 'array']]]], + ]]; + yield 'object or array, sent as array' => ['{"p":[]}', [ + 'type' => 'object', + 'properties' => ['p' => ['oneOf' => [['type' => 'object'], ['type' => 'array']]]], + ]]; + yield 'object or string' => ['{"p":{}}', [ + 'type' => 'object', + 'properties' => ['p' => ['anyOf' => [['type' => 'object'], ['type' => 'string']]]], + ]]; + } + + public function testNestedEmptyArrayIsNotAnObjectByDefault(): void + { + $schema = ['type' => 'object', 'properties' => ['parameters' => ['type' => 'object']]]; + + $errors = $this->validator->validateAgainstJsonSchema(['parameters' => []], $schema); + + $this->assertCount(1, $errors); + $this->assertSame('/parameters', $errors[0]['pointer']); + } + + public function testNonEmptyListIsNotAnObjectEvenIfEmptyArraysMayBe(): void + { + $schema = ['type' => 'object', 'properties' => ['parameters' => ['type' => 'object']]]; + + $errors = $this->validator->validateAgainstJsonSchema(['parameters' => ['x']], $schema, emptyArrayMayBeObject: true); + + $this->assertCount(1, $errors); + $this->assertSame('/parameters', $errors[0]['pointer']); + } + + public function testEmptyObjectStillHasToMeetTheObjectSchema(): void + { + $schema = [ + 'type' => 'object', + 'properties' => ['parameters' => ['type' => 'object', 'required' => ['id']]], + ]; + + $errors = $this->validator->validateAgainstJsonSchema(['parameters' => []], $schema, emptyArrayMayBeObject: true); + + $this->assertCount(1, $errors); + $this->assertSame('required', $errors[0]['keyword']); + } + public function testHandlesEmptySchemaAllowsAnything(): void { $schema = []; // Empty schema object/array implies no constraints diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index dd66a6fd..088d84a0 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -710,6 +710,33 @@ public function testValidationError(): void $this->assertEquals(Error::INVALID_PARAMS, $response->code); } + public function testNestedEmptyObjectArgumentPassesValidation(): void + { + $schema = [ + 'type' => 'object', + 'properties' => ['parameters' => ['type' => 'object']], + 'required' => ['parameters'], + ]; + $arguments = json_decode('{"parameters":{}}', true, flags: \JSON_THROW_ON_ERROR); + $request = $this->createCallToolRequest('query_tool', $arguments); + $toolReference = $this->getMockBuilder(ToolReference::class) + ->setConstructorArgs([new Tool('query_tool', null, $schema, null, null), static function () {}]) + ->onlyMethods(['formatResult']) + ->getMock(); + $toolReference->method('formatResult')->willReturn([new TextContent('ok')]); + + $this->registry->method('getTool')->willReturn($toolReference); + $this->referenceHandler + ->expects($this->once()) + ->method('handle') + ->with($toolReference, ['parameters' => [], '_session' => $this->session, '_request' => $request]) + ->willReturn('ok'); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Response::class, $response); + } + public function testStructuredContentMissingARequiredPropertyIsReportedAsAToolError(): void { $request = $this->createCallToolRequest('get_weather', []);