From a37f4b0b6c71662275deedd4b80d05152b5a5753 Mon Sep 17 00:00:00 2001 From: wadakatu Date: Mon, 18 May 2026 20:09:33 +0900 Subject: [PATCH 1/3] fix(validation): guard spec traversal against non-array structure nodes (#259) A malformed spec whose `paths`, path item, operation, or `responses` node decoded to a scalar slipped past the `?? []` absence guards and raised an uncaught TypeError deep in the traversal path. Add `is_array()` guards along the full traversal: four stages in OpenApiResponseValidator (`paths`, path item, operation, `responses` map) and three in OpenApiRequestValidator (`paths`, path item, operation). Each surfaces a loud spec error ("expected object, got scalar") instead of a misleading diagnostic or a crash. This extends the per-response guards from #256/#258 across the entire traversal. Add a malformed-paths fixture and extend the malformed fixture; cover the new guards with TDD tests. --- src/OpenApiRequestValidator.php | 38 ++++++- src/OpenApiResponseValidator.php | 51 ++++++++- tests/Unit/OpenApiRequestValidatorTest.php | 70 ++++++++++++ tests/Unit/OpenApiResponseValidatorTest.php | 119 ++++++++++++++++++++ tests/fixtures/specs/malformed-paths.json | 8 ++ tests/fixtures/specs/malformed.json | 11 ++ 6 files changed, 294 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/specs/malformed-paths.json diff --git a/src/OpenApiRequestValidator.php b/src/OpenApiRequestValidator.php index feb74cb..c5bb3e8 100644 --- a/src/OpenApiRequestValidator.php +++ b/src/OpenApiRequestValidator.php @@ -129,6 +129,18 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); + // A present-but-non-array `paths` is a malformed spec (stray scalar, + // e.g. a YAML/JSON node that decoded to the wrong type). `?? []` + // guards key *absence* only; a scalar value slips through to the + // `array_keys()` call below and raises an uncaught TypeError. Surface + // it as a loud spec error instead, mirroring the response-side + // traversal guards (issue #259). + if (isset($spec['paths']) && !is_array($spec['paths'])) { + return OpenApiValidationResult::failure([ + "Malformed 'paths' for {$method} {$requestPath} in '{$specName}' spec: expected object, got scalar.", + ]); + } + /** @var string[] $specPaths */ $specPaths = array_keys($spec['paths'] ?? []); $matcher = $this->getPathMatcher($specName, $specPaths); @@ -144,18 +156,40 @@ public function validate( $pathVariables = $matched['variables']; $lowerMethod = strtolower($method); - /** @var array $pathSpec */ $pathSpec = $spec['paths'][$matchedPath] ?? []; + // A present-but-non-array path item is a malformed spec; without this + // guard a scalar `$pathSpec` produces a misleading "method not + // defined" diagnostic (the `isset()` below is false for a scalar) and + // would otherwise reach `ParameterCollector::collect()`'s `array + // $pathSpec` parameter. Surface it loudly instead (issue #259). + if (!is_array($pathSpec)) { + return OpenApiValidationResult::failure([ + "Malformed 'paths[\"{$matchedPath}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath); + } + + /** @var array $pathSpec */ if (!isset($pathSpec[$lowerMethod])) { return OpenApiValidationResult::failure([ PathDiagnosticsFormatter::methodNotDefined($specName, $method, $matchedPath, $spec), ], $matchedPath); } - /** @var array $operation */ $operation = $pathSpec[$lowerMethod]; + // A present-but-non-array operation is a malformed spec; a scalar here + // would reach the `array $operation` parameters of + // `ParameterCollector::collect()` / `RequestBodyValidator::validate()` + // and raise an uncaught TypeError (issue #259). + if (!is_array($operation)) { + return OpenApiValidationResult::failure([ + "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath); + } + + /** @var array $operation */ + // Collect merged path/operation parameters once so path + query + header // validation share a single view of the spec and malformed-entry errors // are surfaced only once. diff --git a/src/OpenApiResponseValidator.php b/src/OpenApiResponseValidator.php index 4174c19..4e6c95a 100644 --- a/src/OpenApiResponseValidator.php +++ b/src/OpenApiResponseValidator.php @@ -108,6 +108,18 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); + // A present-but-non-array `paths` is a malformed spec (stray scalar, + // e.g. a YAML/JSON node that decoded to the wrong type). `?? []` + // guards key *absence* only; a scalar value slips through to the + // `array_keys()` call below and raises an uncaught TypeError. Surface + // it as a loud spec error instead — the traversal-level sibling of + // the per-response content/schema guards (issue #259). + if (isset($spec['paths']) && !is_array($spec['paths'])) { + return OpenApiValidationResult::failure([ + "Malformed 'paths' for {$method} {$requestPath} in '{$specName}' spec: expected object, got scalar.", + ]); + } + /** @var string[] $specPaths */ $specPaths = array_keys($spec['paths'] ?? []); $matcher = $this->getPathMatcher($specName, $specPaths); @@ -122,14 +134,51 @@ public function validate( $lowerMethod = strtolower($method); $pathSpec = $spec['paths'][$matchedPath] ?? []; + // A present-but-non-array path item is a malformed spec; without this + // guard a scalar `$pathSpec` produces a misleading "method not + // defined" diagnostic (the `isset()` below is false for a scalar) and + // hides the real fault. Surface it loudly instead (issue #259). + if (!is_array($pathSpec)) { + return OpenApiValidationResult::failure([ + "Malformed 'paths[\"{$matchedPath}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath); + } + if (!isset($pathSpec[$lowerMethod])) { return OpenApiValidationResult::failure([ PathDiagnosticsFormatter::methodNotDefined($specName, $method, $matchedPath, $spec), ], $matchedPath); } + $operation = $pathSpec[$lowerMethod]; + + // A present-but-non-array operation is a malformed spec; without this + // guard a scalar operation produces a misleading "status code not + // defined" diagnostic (the `responses` lookup `?? []`-falls-back to an + // empty map for a scalar) and hides the real fault (issue #259). + if (!is_array($operation)) { + return OpenApiValidationResult::failure([ + "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath); + } + + /** @var array $operation */ $statusCodeStr = (string) $statusCode; - $responses = $pathSpec[$lowerMethod]['responses'] ?? []; + $responses = $operation['responses'] ?? []; + + // A present-but-non-array `responses` map is a malformed spec; a + // scalar here would reach `SpecResponseKeyResolver::resolve()`'s + // `array $responses` parameter and raise an uncaught TypeError. The + // guard runs BEFORE the skip-by-status-code check below: a malformed + // `responses` map is a structural spec error, not a status-code-level + // failure mode, so a configured skip pattern must not hide it. This + // is the traversal-level sibling of the #258 `responses[$status]` + // per-entry guard (issue #259). + if (!is_array($responses)) { + return OpenApiValidationResult::failure([ + "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}.responses' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath); + } // Skip-by-status-code: applied before the "Status code not defined" // branch so a configured skip suppresses both status-code-level failure diff --git a/tests/Unit/OpenApiRequestValidatorTest.php b/tests/Unit/OpenApiRequestValidatorTest.php index b429758..5e04930 100644 --- a/tests/Unit/OpenApiRequestValidatorTest.php +++ b/tests/Unit/OpenApiRequestValidatorTest.php @@ -586,6 +586,76 @@ public function null_content_media_type_schema_returns_failure(): void ); } + #[Test] + public function malformed_paths_node_returns_failure(): void + { + // The spec's root `paths` is a scalar. `?? []` guards key absence + // only — a scalar slips through to `array_keys()` and raises an + // uncaught TypeError. The traversal guard surfaces a loud spec error + // instead, mirroring the response-side traversal guards (issue #259). + $result = $this->validator->validate( + 'malformed-paths', + 'POST', + '/things', + [], + [], + ['foo' => 'bar'], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_path_item_returns_failure(): void + { + // `paths["/scalar-path-item"]` is a scalar. Without the guard the + // scalar reaches `ParameterCollector::collect()`'s `array $pathSpec` + // parameter (after a misleading "method not defined" diagnostic). + // The guard surfaces a loud spec error instead (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-path-item', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_operation_returns_failure(): void + { + // `paths["/scalar-operation"].get` is a scalar. Without the guard the + // scalar reaches the `array $operation` parameters of + // `ParameterCollector::collect()` / `RequestBodyValidator::validate()` + // and raises an uncaught TypeError (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-operation', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + // ======================================== // Constructor validation (mirrors response validator) // ======================================== diff --git a/tests/Unit/OpenApiResponseValidatorTest.php b/tests/Unit/OpenApiResponseValidatorTest.php index fff2438..edf31de 100644 --- a/tests/Unit/OpenApiResponseValidatorTest.php +++ b/tests/Unit/OpenApiResponseValidatorTest.php @@ -2204,4 +2204,123 @@ public function malformed_response_status_entry_keys_message_off_matched_spec_ke ); $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); } + + #[Test] + public function malformed_paths_node_returns_failure(): void + { + // The spec's root `paths` is a scalar. `?? []` guards key absence + // only — a scalar slips through to `array_keys()` and raises an + // uncaught TypeError. The traversal guard surfaces a loud spec error + // instead, extending the #256/#258 per-response guards to the spec + // walk itself (issue #259). + $result = $this->validator->validate( + 'malformed-paths', + 'GET', + '/things', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_path_item_returns_failure(): void + { + // `paths["/scalar-path-item"]` is a scalar. Without the guard the + // scalar yields a misleading "method not defined" diagnostic (the + // `isset()` lookup is false for a scalar) or a TypeError further + // down. The guard surfaces a loud spec error instead (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-path-item', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_operation_returns_failure(): void + { + // `paths["/scalar-operation"].get` is a scalar. Without the guard the + // scalar reaches the `['responses']` offset access and raises an + // uncaught TypeError (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-operation', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_responses_map_returns_failure(): void + { + // `paths["/scalar-responses-map"].get.responses` is a scalar. Without + // the guard the scalar reaches `SpecResponseKeyResolver::resolve()`'s + // `array $responses` parameter and raises an uncaught TypeError. The + // guard surfaces a loud spec error — the traversal-level sibling of + // the #258 `responses[$status]` per-entry guard (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-responses-map', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-responses-map\"].get.responses'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_responses_map_surfaces_even_for_skip_status(): void + { + // A malformed `responses` map is a structural spec error, not a + // status-code-level failure mode — so the guard fires BEFORE the + // skip-by-status-code check. A 503 (which matches the default + // `5\d\d` skip pattern) must still surface the malformed map loudly + // rather than be silently skipped (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-responses-map', + 503, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertFalse($result->isSkipped()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-responses-map\"].get.responses'", + $result->errors()[0], + ); + } } diff --git a/tests/fixtures/specs/malformed-paths.json b/tests/fixtures/specs/malformed-paths.json new file mode 100644 index 0000000..537bcc4 --- /dev/null +++ b/tests/fixtures/specs/malformed-paths.json @@ -0,0 +1,8 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Malformed Paths Fixture", + "version": "0.1.0" + }, + "paths": "this should have been an object, not a scalar" +} diff --git a/tests/fixtures/specs/malformed.json b/tests/fixtures/specs/malformed.json index 2c6fd1d..bbe6e64 100644 --- a/tests/fixtures/specs/malformed.json +++ b/tests/fixtures/specs/malformed.json @@ -186,6 +186,17 @@ } } }, + "/scalar-path-item": "this should have been an object", + "/scalar-operation": { + "get": "this should have been an object" + }, + "/scalar-responses-map": { + "get": { + "summary": "responses map is a scalar (not an object)", + "operationId": "scalarResponsesMap", + "responses": "this should have been an object" + } + }, "/path-scalar-parameter/{id}": { "parameters": [ "this should have been an object" From 941fce8527005e66bebd25aadd3a9af2a30c749c Mon Sep 17 00:00:00 2001 From: wadakatu Date: Mon, 18 May 2026 20:26:32 +0900 Subject: [PATCH 2/3] fix(validation): extend spec traversal guards to null structure nodes (#259) Address multi-agent review feedback on PR #260. Extend all four traversal-depth guards to also catch null structure nodes. Switch presence checks for `paths` and operations from `isset` to `array_key_exists`, since `isset` returns false for present-but-null values and silently dropped them past the guard. Route path item and responses lookups through `array_key_exists` / `?? null` so an explicit null reaches the guard instead of being masked by `?? []`. Replace the hardcoded "got scalar" error text with `get_debug_type()` so null is no longer mislabeled as a scalar and the concrete type (string, null, etc.) is reported. Correct the guard comments to describe the actual behavior: each guard prevents an uncaught TypeError on a non-array node. Add fixtures covering each level: `/null-path-item`, `/null-operation`, `/null-responses-map` and `/scalar-operation-template/{id}` in malformed.json, plus a new malformed-paths-null.json whose root `paths` is null. Add tests for the null guards at every depth and verify that, for templated paths, the message is keyed by the matched spec path. --- src/OpenApiRequestValidator.php | 76 ++++++--- src/OpenApiResponseValidator.php | 86 +++++++--- tests/Unit/OpenApiRequestValidatorTest.php | 98 ++++++++++-- tests/Unit/OpenApiResponseValidatorTest.php | 147 ++++++++++++++++-- .../fixtures/specs/malformed-paths-null.json | 8 + tests/fixtures/specs/malformed.json | 14 ++ 6 files changed, 359 insertions(+), 70 deletions(-) create mode 100644 tests/fixtures/specs/malformed-paths-null.json diff --git a/src/OpenApiRequestValidator.php b/src/OpenApiRequestValidator.php index c5bb3e8..472dd11 100644 --- a/src/OpenApiRequestValidator.php +++ b/src/OpenApiRequestValidator.php @@ -20,7 +20,9 @@ use Studio\OpenApiContractTesting\Validation\Support\StatusCodePatternSet; use Studio\OpenApiContractTesting\Validation\Support\ValidatorErrorBoundary; +use function array_key_exists; use function array_keys; +use function get_debug_type; use function is_array; use function sprintf; use function strtolower; @@ -129,15 +131,24 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); - // A present-but-non-array `paths` is a malformed spec (stray scalar, - // e.g. a YAML/JSON node that decoded to the wrong type). `?? []` - // guards key *absence* only; a scalar value slips through to the - // `array_keys()` call below and raises an uncaught TypeError. Surface - // it as a loud spec error instead, mirroring the response-side - // traversal guards (issue #259). - if (isset($spec['paths']) && !is_array($spec['paths'])) { + // A present-but-non-array `paths` is a malformed spec — a stray + // scalar or an explicit `null` (a YAML `paths:` left empty, or a node + // that decoded to the wrong type). `array_key_exists` (not `isset`) + // is deliberate: `isset` is false for `null`, which would let a + // present-but-`null` `paths` slip through to `?? []` below and be + // silently coalesced to an empty map. A non-array value otherwise + // reaches the `array_keys()` call and raises an uncaught TypeError. + // Surface it as a loud spec error instead, mirroring the + // response-side traversal guards (issue #259). + if (array_key_exists('paths', $spec) && !is_array($spec['paths'])) { return OpenApiValidationResult::failure([ - "Malformed 'paths' for {$method} {$requestPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths' for %s %s in '%s' spec: expected object, got %s.", + $method, + $requestPath, + $specName, + get_debug_type($spec['paths']), + ), ]); } @@ -156,21 +167,35 @@ public function validate( $pathVariables = $matched['variables']; $lowerMethod = strtolower($method); - $pathSpec = $spec['paths'][$matchedPath] ?? []; + // `$matchedPath` is always a key of `$spec['paths']` (the matcher was + // built from its `array_keys()`), so `?? null` here only fires for an + // explicit `null` *value* — which the guard below then treats as + // malformed, exactly like a scalar path item. + $pathSpec = $spec['paths'][$matchedPath] ?? null; // A present-but-non-array path item is a malformed spec; without this - // guard a scalar `$pathSpec` produces a misleading "method not - // defined" diagnostic (the `isset()` below is false for a scalar) and - // would otherwise reach `ParameterCollector::collect()`'s `array - // $pathSpec` parameter. Surface it loudly instead (issue #259). + // guard a scalar/`null` `$pathSpec` reaches the `array_key_exists()` + // method lookup below (and `ParameterCollector::collect()`'s `array + // $pathSpec` parameter) and raises an uncaught TypeError. Surface it + // loudly instead (issue #259). if (!is_array($pathSpec)) { return OpenApiValidationResult::failure([ - "Malformed 'paths[\"{$matchedPath}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", + $matchedPath, + $method, + $matchedPath, + $specName, + get_debug_type($pathSpec), + ), ], $matchedPath); } /** @var array $pathSpec */ - if (!isset($pathSpec[$lowerMethod])) { + // `array_key_exists` (not `isset`) so an explicit `{method}: null` + // reaches the operation guard below as malformed rather than being + // misreported as an undefined method. + if (!array_key_exists($lowerMethod, $pathSpec)) { return OpenApiValidationResult::failure([ PathDiagnosticsFormatter::methodNotDefined($specName, $method, $matchedPath, $spec), ], $matchedPath); @@ -178,21 +203,28 @@ public function validate( $operation = $pathSpec[$lowerMethod]; - // A present-but-non-array operation is a malformed spec; a scalar here - // would reach the `array $operation` parameters of - // `ParameterCollector::collect()` / `RequestBodyValidator::validate()` - // and raise an uncaught TypeError (issue #259). + // A present-but-non-array operation is a malformed spec; a scalar or + // `null` here would reach `ParameterCollector::collect()`'s `array + // $operation` parameter (the first scalar-typed sink) and raise an + // uncaught TypeError (issue #259). if (!is_array($operation)) { return OpenApiValidationResult::failure([ - "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths[\"%s\"].%s' for %s %s in '%s' spec: expected object, got %s.", + $matchedPath, + $lowerMethod, + $method, + $matchedPath, + $specName, + get_debug_type($operation), + ), ], $matchedPath); } - /** @var array $operation */ - // Collect merged path/operation parameters once so path + query + header // validation share a single view of the spec and malformed-entry errors // are surfaced only once. + /** @var array $operation */ $collected = ParameterCollector::collect($method, $matchedPath, $pathSpec, $operation); // Each sub-validator is wrapped in ValidatorErrorBoundary::safely() so a diff --git a/src/OpenApiResponseValidator.php b/src/OpenApiResponseValidator.php index 4e6c95a..50e6e0a 100644 --- a/src/OpenApiResponseValidator.php +++ b/src/OpenApiResponseValidator.php @@ -21,6 +21,7 @@ use Studio\OpenApiContractTesting\Validation\Support\StatusCodePatternSet; use Studio\OpenApiContractTesting\Validation\Support\ValidatorErrorBoundary; +use function array_key_exists; use function array_keys; use function array_merge; use function get_debug_type; @@ -108,15 +109,24 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); - // A present-but-non-array `paths` is a malformed spec (stray scalar, - // e.g. a YAML/JSON node that decoded to the wrong type). `?? []` - // guards key *absence* only; a scalar value slips through to the - // `array_keys()` call below and raises an uncaught TypeError. Surface - // it as a loud spec error instead — the traversal-level sibling of - // the per-response content/schema guards (issue #259). - if (isset($spec['paths']) && !is_array($spec['paths'])) { + // A present-but-non-array `paths` is a malformed spec — a stray + // scalar or an explicit `null` (a YAML `paths:` left empty, or a node + // that decoded to the wrong type). `array_key_exists` (not `isset`) + // is deliberate: `isset` is false for `null`, which would let a + // present-but-`null` `paths` slip through to `?? []` below and be + // silently coalesced to an empty map. A non-array value otherwise + // reaches the `array_keys()` call and raises an uncaught TypeError. + // Surface it as a loud spec error instead — the traversal-level + // sibling of the per-response content/schema guards (issue #259). + if (array_key_exists('paths', $spec) && !is_array($spec['paths'])) { return OpenApiValidationResult::failure([ - "Malformed 'paths' for {$method} {$requestPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths' for %s %s in '%s' spec: expected object, got %s.", + $method, + $requestPath, + $specName, + get_debug_type($spec['paths']), + ), ]); } @@ -132,19 +142,33 @@ public function validate( } $lowerMethod = strtolower($method); - $pathSpec = $spec['paths'][$matchedPath] ?? []; + // `$matchedPath` is always a key of `$spec['paths']` (the matcher was + // built from its `array_keys()`), so `?? null` here only fires for an + // explicit `null` *value* — which the guard below then treats as + // malformed, exactly like a scalar path item. + $pathSpec = $spec['paths'][$matchedPath] ?? null; // A present-but-non-array path item is a malformed spec; without this - // guard a scalar `$pathSpec` produces a misleading "method not - // defined" diagnostic (the `isset()` below is false for a scalar) and - // hides the real fault. Surface it loudly instead (issue #259). + // guard a scalar/`null` `$pathSpec` reaches the `array_key_exists()` + // method lookup below and raises an uncaught TypeError. Surface it + // loudly instead (issue #259). if (!is_array($pathSpec)) { return OpenApiValidationResult::failure([ - "Malformed 'paths[\"{$matchedPath}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", + $matchedPath, + $method, + $matchedPath, + $specName, + get_debug_type($pathSpec), + ), ], $matchedPath); } - if (!isset($pathSpec[$lowerMethod])) { + // `array_key_exists` (not `isset`) so an explicit `{method}: null` + // reaches the operation guard below as malformed rather than being + // misreported as an undefined method. + if (!array_key_exists($lowerMethod, $pathSpec)) { return OpenApiValidationResult::failure([ PathDiagnosticsFormatter::methodNotDefined($specName, $method, $matchedPath, $spec), ], $matchedPath); @@ -153,21 +177,33 @@ public function validate( $operation = $pathSpec[$lowerMethod]; // A present-but-non-array operation is a malformed spec; without this - // guard a scalar operation produces a misleading "status code not - // defined" diagnostic (the `responses` lookup `?? []`-falls-back to an - // empty map for a scalar) and hides the real fault (issue #259). + // guard a scalar/`null` operation reaches the `array_key_exists()` + // `responses` lookup below and raises an uncaught TypeError + // (issue #259). if (!is_array($operation)) { return OpenApiValidationResult::failure([ - "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths[\"%s\"].%s' for %s %s in '%s' spec: expected object, got %s.", + $matchedPath, + $lowerMethod, + $method, + $matchedPath, + $specName, + get_debug_type($operation), + ), ], $matchedPath); } /** @var array $operation */ $statusCodeStr = (string) $statusCode; - $responses = $operation['responses'] ?? []; + // `array_key_exists` (not `?? []`) so a present-but-`null` `responses` + // is caught by the guard below as malformed, while a genuinely absent + // `responses` key still falls back to an empty map (resolved later as + // "status code not defined"). + $responses = array_key_exists('responses', $operation) ? $operation['responses'] : []; // A present-but-non-array `responses` map is a malformed spec; a - // scalar here would reach `SpecResponseKeyResolver::resolve()`'s + // scalar/`null` here would reach `SpecResponseKeyResolver::resolve()`'s // `array $responses` parameter and raise an uncaught TypeError. The // guard runs BEFORE the skip-by-status-code check below: a malformed // `responses` map is a structural spec error, not a status-code-level @@ -176,7 +212,15 @@ public function validate( // per-entry guard (issue #259). if (!is_array($responses)) { return OpenApiValidationResult::failure([ - "Malformed 'paths[\"{$matchedPath}\"].{$lowerMethod}.responses' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'paths[\"%s\"].%s.responses' for %s %s in '%s' spec: expected object, got %s.", + $matchedPath, + $lowerMethod, + $method, + $matchedPath, + $specName, + get_debug_type($responses), + ), ], $matchedPath); } diff --git a/tests/Unit/OpenApiRequestValidatorTest.php b/tests/Unit/OpenApiRequestValidatorTest.php index 5e04930..ddb2bf2 100644 --- a/tests/Unit/OpenApiRequestValidatorTest.php +++ b/tests/Unit/OpenApiRequestValidatorTest.php @@ -589,10 +589,10 @@ public function null_content_media_type_schema_returns_failure(): void #[Test] public function malformed_paths_node_returns_failure(): void { - // The spec's root `paths` is a scalar. `?? []` guards key absence - // only — a scalar slips through to `array_keys()` and raises an - // uncaught TypeError. The traversal guard surfaces a loud spec error - // instead, mirroring the response-side traversal guards (issue #259). + // The spec's root `paths` is a scalar. The traversal guard surfaces a + // loud spec error before it reaches `array_keys()` (which would raise + // an uncaught TypeError), mirroring the response-side traversal + // guards (issue #259). $result = $this->validator->validate( 'malformed-paths', 'POST', @@ -605,16 +605,40 @@ public function malformed_paths_node_returns_failure(): void $this->assertFalse($result->isValid()); $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_paths_node_returns_failure(): void + { + // The spec's root `paths` is an explicit `null` (a YAML `paths:` left + // empty). The guard uses `array_key_exists` (not `isset`, which is + // false for `null`), so a present-but-`null` `paths` surfaces as a + // malformed spec rather than being coalesced to an empty paths map + // (issue #259). + $result = $this->validator->validate( + 'malformed-paths-null', + 'POST', + '/things', + [], + [], + ['foo' => 'bar'], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } #[Test] public function malformed_path_item_returns_failure(): void { // `paths["/scalar-path-item"]` is a scalar. Without the guard the - // scalar reaches `ParameterCollector::collect()`'s `array $pathSpec` - // parameter (after a misleading "method not defined" diagnostic). - // The guard surfaces a loud spec error instead (issue #259). + // scalar reaches the `array_key_exists()` method lookup (and + // `ParameterCollector::collect()`'s `array $pathSpec` parameter) and + // raises an uncaught TypeError. The guard surfaces a loud spec error + // instead (issue #259). $result = $this->validator->validate( 'malformed', 'GET', @@ -629,16 +653,40 @@ public function malformed_path_item_returns_failure(): void "Malformed 'paths[\"/scalar-path-item\"]'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_path_item_returns_failure(): void + { + // `paths["/null-path-item"]` is an explicit `null`. The `?? null` + // fallback on the path-item lookup keeps the `null` value flowing + // into the `!is_array()` guard rather than coalescing it to an empty + // path item — so a null path item surfaces as malformed (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/null-path-item', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/null-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } #[Test] public function malformed_operation_returns_failure(): void { // `paths["/scalar-operation"].get` is a scalar. Without the guard the - // scalar reaches the `array $operation` parameters of - // `ParameterCollector::collect()` / `RequestBodyValidator::validate()` - // and raises an uncaught TypeError (issue #259). + // scalar reaches the `array_key_exists()` method lookup and + // `ParameterCollector::collect()`'s `array $operation` parameter, + // raising an uncaught TypeError (issue #259). $result = $this->validator->validate( 'malformed', 'GET', @@ -653,7 +701,31 @@ public function malformed_operation_returns_failure(): void "Malformed 'paths[\"/scalar-operation\"].get'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_operation_returns_failure(): void + { + // `paths["/null-operation"].get` is an explicit `null`. The method + // lookup uses `array_key_exists` (not `isset`), so a `get: null` + // entry reaches the operation guard as malformed rather than being + // misreported as an undefined method (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/null-operation', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/null-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } // ======================================== diff --git a/tests/Unit/OpenApiResponseValidatorTest.php b/tests/Unit/OpenApiResponseValidatorTest.php index edf31de..de39045 100644 --- a/tests/Unit/OpenApiResponseValidatorTest.php +++ b/tests/Unit/OpenApiResponseValidatorTest.php @@ -2208,11 +2208,10 @@ public function malformed_response_status_entry_keys_message_off_matched_spec_ke #[Test] public function malformed_paths_node_returns_failure(): void { - // The spec's root `paths` is a scalar. `?? []` guards key absence - // only — a scalar slips through to `array_keys()` and raises an - // uncaught TypeError. The traversal guard surfaces a loud spec error - // instead, extending the #256/#258 per-response guards to the spec - // walk itself (issue #259). + // The spec's root `paths` is a scalar. The traversal guard surfaces a + // loud spec error before it reaches `array_keys()` (which would raise + // an uncaught TypeError), extending the #256/#258 per-response guards + // to the spec walk itself (issue #259). $result = $this->validator->validate( 'malformed-paths', 'GET', @@ -2224,16 +2223,38 @@ public function malformed_paths_node_returns_failure(): void $this->assertFalse($result->isValid()); $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_paths_node_returns_failure(): void + { + // The spec's root `paths` is an explicit `null` (a YAML `paths:` left + // empty). `isset()` would be false for `null`, so the guard uses + // `array_key_exists`: a present-but-`null` `paths` must surface as a + // malformed spec, not be silently coalesced to an empty paths map + // (issue #259). `get_debug_type` reports the concrete `null` type. + $result = $this->validator->validate( + 'malformed-paths-null', + 'GET', + '/things', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } #[Test] public function malformed_path_item_returns_failure(): void { // `paths["/scalar-path-item"]` is a scalar. Without the guard the - // scalar yields a misleading "method not defined" diagnostic (the - // `isset()` lookup is false for a scalar) or a TypeError further - // down. The guard surfaces a loud spec error instead (issue #259). + // scalar reaches the `array_key_exists()` method lookup and raises an + // uncaught TypeError. The guard surfaces a loud spec error instead + // (issue #259). $result = $this->validator->validate( 'malformed', 'GET', @@ -2248,15 +2269,39 @@ public function malformed_path_item_returns_failure(): void "Malformed 'paths[\"/scalar-path-item\"]'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_path_item_returns_failure(): void + { + // `paths["/null-path-item"]` is an explicit `null`. The `?? null` + // fallback on the path-item lookup keeps the `null` value flowing + // into the `!is_array()` guard rather than coalescing it to an empty + // path item — so a null path item surfaces as malformed (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/null-path-item', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/null-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } #[Test] public function malformed_operation_returns_failure(): void { // `paths["/scalar-operation"].get` is a scalar. Without the guard the - // scalar reaches the `['responses']` offset access and raises an - // uncaught TypeError (issue #259). + // scalar reaches the `array_key_exists()` `responses` lookup and + // raises an uncaught TypeError (issue #259). $result = $this->validator->validate( 'malformed', 'GET', @@ -2271,7 +2316,56 @@ public function malformed_operation_returns_failure(): void "Malformed 'paths[\"/scalar-operation\"].get'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_operation_returns_failure(): void + { + // `paths["/null-operation"].get` is an explicit `null`. The method + // lookup uses `array_key_exists` (not `isset`), so a `get: null` + // entry reaches the operation guard as malformed rather than being + // misreported as an undefined method (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/null-operation', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/null-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); + } + + #[Test] + public function malformed_operation_message_keys_off_matched_spec_path(): void + { + // The malformed operation lives under a templated path + // (`/scalar-operation-template/{id}`); the request hits a concrete + // `/scalar-operation-template/42`. The guard message must name the + // matched spec key (the `{id}` template), not the wire request path + // — mirroring `malformed_response_status_entry_keys_message_off_matched_spec_key`. + $result = $this->validator->validate( + 'malformed', + 'GET', + '/scalar-operation-template/42', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/scalar-operation-template/{id}\"].get'", + $result->errors()[0], + ); + $this->assertStringNotContainsString('/scalar-operation-template/42', $result->errors()[0]); } #[Test] @@ -2296,7 +2390,32 @@ public function malformed_responses_map_returns_failure(): void "Malformed 'paths[\"/scalar-responses-map\"].get.responses'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); + } + + #[Test] + public function null_responses_map_returns_failure(): void + { + // `paths["/null-responses-map"].get.responses` is an explicit `null`. + // The `responses` lookup uses `array_key_exists` (not `?? []`), so a + // present-but-`null` `responses` reaches the guard as malformed while + // a genuinely absent `responses` key still falls back to an empty map + // (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/null-responses-map', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/null-responses-map\"].get.responses'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } #[Test] diff --git a/tests/fixtures/specs/malformed-paths-null.json b/tests/fixtures/specs/malformed-paths-null.json new file mode 100644 index 0000000..93397c8 --- /dev/null +++ b/tests/fixtures/specs/malformed-paths-null.json @@ -0,0 +1,8 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Malformed Paths Fixture (explicit null)", + "version": "0.1.0" + }, + "paths": null +} diff --git a/tests/fixtures/specs/malformed.json b/tests/fixtures/specs/malformed.json index bbe6e64..424ac62 100644 --- a/tests/fixtures/specs/malformed.json +++ b/tests/fixtures/specs/malformed.json @@ -197,6 +197,20 @@ "responses": "this should have been an object" } }, + "/null-path-item": null, + "/null-operation": { + "get": null + }, + "/null-responses-map": { + "get": { + "summary": "responses map is an explicit null (not an object)", + "operationId": "nullResponsesMap", + "responses": null + } + }, + "/scalar-operation-template/{id}": { + "get": "this should have been an object" + }, "/path-scalar-parameter/{id}": { "parameters": [ "this should have been an object" From 6a42f8921667ae3f81ab6de0f35a5c0138c334ff Mon Sep 17 00:00:00 2001 From: wadakatu Date: Mon, 18 May 2026 20:42:41 +0900 Subject: [PATCH 3/3] fix(validation): unify malformed structural node detection across guards (#259) Introduce a MalformedSpecNode helper (isMalformed() / describe()) and route all 15 guard sites through it: #259 traversal (7), #258 responses[$status] (1), and #256 content/schema (7). In addition to scalars and null, JSON lists ([...] written where an object is expected) are now detected as malformed. List values previously passed is_array() and were silently mis-resolved; they are now surfaced as loud spec errors. Empty arrays remain tolerated since {} and [] are indistinguishable. Error messages now report the actual node type via describe() (string / null / list, etc.) instead of the hardcoded "got scalar". Add unit tests for the helper and list-value integration tests for each guard family, and update existing assertions that assumed "got scalar" to the real type. Extend malformed.json with list-value paths and add the malformed-paths-list.json fixture. --- src/OpenApiRequestValidator.php | 49 +++---- src/OpenApiResponseValidator.php | 120 ++++++++++------- .../Request/RequestBodyValidator.php | 48 +++++-- .../Response/ResponseBodyValidator.php | 39 ++++-- src/Validation/Support/MalformedSpecNode.php | 68 ++++++++++ tests/Unit/OpenApiRequestValidatorTest.php | 70 +++++++++- tests/Unit/OpenApiResponseValidatorTest.php | 123 +++++++++++++++++- .../Request/RequestBodyValidatorTest.php | 53 +++++++- .../Response/ResponseBodyValidatorTest.php | 57 +++++++- .../Support/MalformedSpecNodeTest.php | 70 ++++++++++ .../fixtures/specs/malformed-paths-list.json | 8 ++ tests/fixtures/specs/malformed.json | 20 +++ 12 files changed, 621 insertions(+), 104 deletions(-) create mode 100644 src/Validation/Support/MalformedSpecNode.php create mode 100644 tests/Unit/Validation/Support/MalformedSpecNodeTest.php create mode 100644 tests/fixtures/specs/malformed-paths-list.json diff --git a/src/OpenApiRequestValidator.php b/src/OpenApiRequestValidator.php index 472dd11..e4993bf 100644 --- a/src/OpenApiRequestValidator.php +++ b/src/OpenApiRequestValidator.php @@ -14,6 +14,7 @@ use Studio\OpenApiContractTesting\Validation\Request\RequestBodyValidationResult; use Studio\OpenApiContractTesting\Validation\Request\RequestBodyValidator; use Studio\OpenApiContractTesting\Validation\Request\SecurityValidator; +use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\PathDiagnosticsFormatter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; use Studio\OpenApiContractTesting\Validation\Support\SpecResponseKeyResolver; @@ -22,7 +23,6 @@ use function array_key_exists; use function array_keys; -use function get_debug_type; use function is_array; use function sprintf; use function strtolower; @@ -131,23 +131,22 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); - // A present-but-non-array `paths` is a malformed spec — a stray - // scalar or an explicit `null` (a YAML `paths:` left empty, or a node - // that decoded to the wrong type). `array_key_exists` (not `isset`) - // is deliberate: `isset` is false for `null`, which would let a - // present-but-`null` `paths` slip through to `?? []` below and be - // silently coalesced to an empty map. A non-array value otherwise - // reaches the `array_keys()` call and raises an uncaught TypeError. - // Surface it as a loud spec error instead, mirroring the + // The root `paths` must decode to a JSON object; a scalar, `null`, or + // a JSON list is a malformed spec ({@see MalformedSpecNode}). + // Unguarded, a non-array reaches the `array_keys()` call below + // (uncaught TypeError) and a list mis-resolves silently. The presence + // test uses `array_key_exists` (not `isset`) so a present-but-`null` + // `paths` is caught here rather than coalesced to an empty map by + // `?? []`. Surface it as a loud spec error instead, mirroring the // response-side traversal guards (issue #259). - if (array_key_exists('paths', $spec) && !is_array($spec['paths'])) { + if (array_key_exists('paths', $spec) && MalformedSpecNode::isMalformed($spec['paths'])) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths' for %s %s in '%s' spec: expected object, got %s.", $method, $requestPath, $specName, - get_debug_type($spec['paths']), + MalformedSpecNode::describe($spec['paths']), ), ]); } @@ -173,12 +172,13 @@ public function validate( // malformed, exactly like a scalar path item. $pathSpec = $spec['paths'][$matchedPath] ?? null; - // A present-but-non-array path item is a malformed spec; without this - // guard a scalar/`null` `$pathSpec` reaches the `array_key_exists()` - // method lookup below (and `ParameterCollector::collect()`'s `array - // $pathSpec` parameter) and raises an uncaught TypeError. Surface it - // loudly instead (issue #259). - if (!is_array($pathSpec)) { + // A path item must decode to a JSON object; a scalar, `null`, or a + // JSON list is malformed ({@see MalformedSpecNode}). Unguarded, a + // non-array reaches the `array_key_exists()` method lookup below (and + // `ParameterCollector::collect()`'s `array $pathSpec` parameter), + // raising an uncaught TypeError, and a list mis-resolves silently. + // Surface it loudly instead (issue #259). + if (MalformedSpecNode::isMalformed($pathSpec)) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", @@ -186,7 +186,7 @@ public function validate( $method, $matchedPath, $specName, - get_debug_type($pathSpec), + MalformedSpecNode::describe($pathSpec), ), ], $matchedPath); } @@ -203,11 +203,12 @@ public function validate( $operation = $pathSpec[$lowerMethod]; - // A present-but-non-array operation is a malformed spec; a scalar or - // `null` here would reach `ParameterCollector::collect()`'s `array - // $operation` parameter (the first scalar-typed sink) and raise an - // uncaught TypeError (issue #259). - if (!is_array($operation)) { + // An operation must decode to a JSON object; a scalar, `null`, or a + // JSON list is malformed ({@see MalformedSpecNode}). A non-array + // would reach `ParameterCollector::collect()`'s `array $operation` + // parameter (the first scalar-typed sink) and raise an uncaught + // TypeError; a list mis-resolves silently (issue #259). + if (MalformedSpecNode::isMalformed($operation)) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths[\"%s\"].%s' for %s %s in '%s' spec: expected object, got %s.", @@ -216,7 +217,7 @@ public function validate( $method, $matchedPath, $specName, - get_debug_type($operation), + MalformedSpecNode::describe($operation), ), ], $matchedPath); } diff --git a/src/OpenApiResponseValidator.php b/src/OpenApiResponseValidator.php index 50e6e0a..01ac1bb 100644 --- a/src/OpenApiResponseValidator.php +++ b/src/OpenApiResponseValidator.php @@ -15,6 +15,7 @@ use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredBodyWalker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallChecker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredTracker; +use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\PathDiagnosticsFormatter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; use Studio\OpenApiContractTesting\Validation\Support\SpecResponseKeyResolver; @@ -109,23 +110,23 @@ public function validate( $version = OpenApiVersion::fromSpec($spec); - // A present-but-non-array `paths` is a malformed spec — a stray - // scalar or an explicit `null` (a YAML `paths:` left empty, or a node - // that decoded to the wrong type). `array_key_exists` (not `isset`) - // is deliberate: `isset` is false for `null`, which would let a - // present-but-`null` `paths` slip through to `?? []` below and be - // silently coalesced to an empty map. A non-array value otherwise - // reaches the `array_keys()` call and raises an uncaught TypeError. - // Surface it as a loud spec error instead — the traversal-level - // sibling of the per-response content/schema guards (issue #259). - if (array_key_exists('paths', $spec) && !is_array($spec['paths'])) { + // The root `paths` must decode to a JSON object; a scalar, `null`, or + // a JSON list is a malformed spec ({@see MalformedSpecNode}). + // Unguarded, a non-array reaches the `array_keys()` call below + // (uncaught TypeError) and a list mis-resolves silently. The presence + // test uses `array_key_exists` (not `isset`) so a present-but-`null` + // `paths` is caught here rather than coalesced to an empty map by + // `?? []`. Surface it as a loud spec error instead — the + // traversal-level sibling of the per-response content/schema guards + // (issue #259). + if (array_key_exists('paths', $spec) && MalformedSpecNode::isMalformed($spec['paths'])) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths' for %s %s in '%s' spec: expected object, got %s.", $method, $requestPath, $specName, - get_debug_type($spec['paths']), + MalformedSpecNode::describe($spec['paths']), ), ]); } @@ -148,11 +149,12 @@ public function validate( // malformed, exactly like a scalar path item. $pathSpec = $spec['paths'][$matchedPath] ?? null; - // A present-but-non-array path item is a malformed spec; without this - // guard a scalar/`null` `$pathSpec` reaches the `array_key_exists()` - // method lookup below and raises an uncaught TypeError. Surface it + // A path item must decode to a JSON object; a scalar, `null`, or a + // JSON list is malformed ({@see MalformedSpecNode}). Unguarded, a + // non-array reaches the `array_key_exists()` method lookup below + // (uncaught TypeError) and a list mis-resolves silently. Surface it // loudly instead (issue #259). - if (!is_array($pathSpec)) { + if (MalformedSpecNode::isMalformed($pathSpec)) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", @@ -160,7 +162,7 @@ public function validate( $method, $matchedPath, $specName, - get_debug_type($pathSpec), + MalformedSpecNode::describe($pathSpec), ), ], $matchedPath); } @@ -176,11 +178,11 @@ public function validate( $operation = $pathSpec[$lowerMethod]; - // A present-but-non-array operation is a malformed spec; without this - // guard a scalar/`null` operation reaches the `array_key_exists()` - // `responses` lookup below and raises an uncaught TypeError - // (issue #259). - if (!is_array($operation)) { + // An operation must decode to a JSON object; a scalar, `null`, or a + // JSON list is malformed ({@see MalformedSpecNode}). Unguarded, a + // non-array reaches the `array_key_exists()` `responses` lookup below + // (uncaught TypeError) and a list mis-resolves silently (issue #259). + if (MalformedSpecNode::isMalformed($operation)) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths[\"%s\"].%s' for %s %s in '%s' spec: expected object, got %s.", @@ -189,7 +191,7 @@ public function validate( $method, $matchedPath, $specName, - get_debug_type($operation), + MalformedSpecNode::describe($operation), ), ], $matchedPath); } @@ -202,15 +204,16 @@ public function validate( // "status code not defined"). $responses = array_key_exists('responses', $operation) ? $operation['responses'] : []; - // A present-but-non-array `responses` map is a malformed spec; a - // scalar/`null` here would reach `SpecResponseKeyResolver::resolve()`'s - // `array $responses` parameter and raise an uncaught TypeError. The - // guard runs BEFORE the skip-by-status-code check below: a malformed - // `responses` map is a structural spec error, not a status-code-level - // failure mode, so a configured skip pattern must not hide it. This - // is the traversal-level sibling of the #258 `responses[$status]` - // per-entry guard (issue #259). - if (!is_array($responses)) { + // The `responses` map must decode to a JSON object; a scalar, `null`, + // or a JSON list is malformed ({@see MalformedSpecNode}). Unguarded, + // a non-array reaches `SpecResponseKeyResolver::resolve()`'s `array + // $responses` parameter (uncaught TypeError) and a list mis-resolves + // silently. The guard runs BEFORE the skip-by-status-code check + // below: a malformed `responses` map is a structural spec error, not + // a status-code-level failure mode, so a configured skip pattern must + // not hide it. This is the traversal-level sibling of the #258 + // `responses[$status]` per-entry guard (issue #259). + if (MalformedSpecNode::isMalformed($responses)) { return OpenApiValidationResult::failure([ sprintf( "Malformed 'paths[\"%s\"].%s.responses' for %s %s in '%s' spec: expected object, got %s.", @@ -219,7 +222,7 @@ public function validate( $method, $matchedPath, $specName, - get_debug_type($responses), + MalformedSpecNode::describe($responses), ), ], $matchedPath); } @@ -282,17 +285,25 @@ public function validate( $statusCodeStr = $matchedResponseKey; $responseSpec = $responses[$matchedResponseKey]; - // A present-but-non-array response entry is a malformed spec (stray - // scalar, e.g. an unresolved $ref); surface it as a loud spec error - // (issue #258). Without this guard the scalar reaches the - // `array $responseSpec` parameters of validateBody() / validateHeaders() - // and raises an uncaught TypeError (TypeError extends Error, not - // RuntimeException, so validateBody()'s catch would not see it). This - // mirrors the content-level guards in validateBody() and - // RequestBodyValidator's `requestBody` guard. - if (!is_array($responseSpec)) { + // A response entry must decode to a JSON object; a scalar, `null`, or + // a JSON list is a malformed spec ({@see MalformedSpecNode}) — e.g. + // an unresolved $ref. Surface it as a loud spec error (issue #258). + // Without this guard the bad value reaches the `array $responseSpec` + // parameters of validateBody() / validateHeaders() and raises an + // uncaught TypeError (TypeError extends Error, not RuntimeException, + // so validateBody()'s catch would not see it). This mirrors the + // content-level guards in validateBody() and RequestBodyValidator's + // `requestBody` guard. + if (MalformedSpecNode::isMalformed($responseSpec)) { return OpenApiValidationResult::failure([ - "Malformed 'responses[{$matchedResponseKey}]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'responses[%s]' for %s %s in '%s' spec: expected object, got %s.", + $matchedResponseKey, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($responseSpec), + ), ], $matchedPath, $statusCodeStr); } @@ -520,15 +531,24 @@ private function validateBody( return new ResponseBodyValidationResult([], null); } - // A present-but-non-array `content` is a malformed spec (stray scalar, - // e.g. an unresolved $ref). Surface it before it reaches - // ResponseBodyValidator::validate()'s `array $content` parameter, where - // it would raise an uncaught TypeError (TypeError extends Error, not - // RuntimeException, so the catch below would not see it). Mirrors - // RequestBodyValidator's `requestBody.content` guard (issue #256). - if (!is_array($responseSpec['content'])) { + // A `content` block must decode to a JSON object; a scalar or a JSON + // list is a malformed spec ({@see MalformedSpecNode}) — e.g. an + // unresolved $ref. Surface it before it reaches + // ResponseBodyValidator::validate()'s `array $content` parameter, + // where a non-array would raise an uncaught TypeError (TypeError + // extends Error, not RuntimeException, so the catch below would not + // see it). Mirrors RequestBodyValidator's `requestBody.content` guard + // (issue #256). + if (MalformedSpecNode::isMalformed($responseSpec['content'])) { return new ResponseBodyValidationResult([ - "Malformed 'responses[{$statusCode}].content' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'responses[%s].content' for %s %s in '%s' spec: expected object, got %s.", + $statusCode, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($responseSpec['content']), + ), ], null); } diff --git a/src/Validation/Request/RequestBodyValidator.php b/src/Validation/Request/RequestBodyValidator.php index e468b89..f83bac3 100644 --- a/src/Validation/Request/RequestBodyValidator.php +++ b/src/Validation/Request/RequestBodyValidator.php @@ -10,6 +10,7 @@ use Studio\OpenApiContractTesting\SchemaContext; use Studio\OpenApiContractTesting\Spec\OpenApiSchemaConverter; use Studio\OpenApiContractTesting\Validation\Support\ContentTypeMatcher; +use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -58,11 +59,18 @@ public function validate( return new RequestBodyValidationResult([]); } - // A present-but-non-array requestBody signals a malformed spec (stray scalar). + // A `requestBody` must decode to a JSON object; a scalar or a JSON + // list signals a malformed spec ({@see MalformedSpecNode}). // Contract-testing tools should surface this, not mask it as "no body". - if (!is_array($operation['requestBody'])) { + if (MalformedSpecNode::isMalformed($operation['requestBody'])) { return new RequestBodyValidationResult([ - "Malformed 'requestBody' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'requestBody' for %s %s in '%s' spec: expected object, got %s.", + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($operation['requestBody']), + ), ]); } @@ -75,9 +83,15 @@ public function validate( return new RequestBodyValidationResult([]); } - if (!is_array($requestBodySpec['content'])) { + if (MalformedSpecNode::isMalformed($requestBodySpec['content'])) { return new RequestBodyValidationResult([ - "Malformed 'requestBody.content' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'requestBody.content' for %s %s in '%s' spec: expected object, got %s.", + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($requestBodySpec['content']), + ), ]); } @@ -89,9 +103,18 @@ public function validate( // runtime — a malformed spec like `content: {"application/json": "oops"}` // would TypeError on downstream array accesses. Surface it as a loud spec // error instead, matching the sibling guard on `requestBody.content` above. - if (!is_array($mediaTypeSpec)) { + // A JSON list written for the entry is rejected the same way + // ({@see MalformedSpecNode}). + if (MalformedSpecNode::isMalformed($mediaTypeSpec)) { return new RequestBodyValidationResult([ - "Malformed 'requestBody.content[\"{$mediaType}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'requestBody.content[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", + $mediaType, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($mediaTypeSpec), + ), ]); } @@ -100,9 +123,16 @@ public function validate( // OpenApiSchemaConverter::convert() as a scalar, producing a confusing // TypeError instead of a spec-level error. array_key_exists rather than // isset so an explicit `schema: null` is also flagged. - if (array_key_exists('schema', $mediaTypeSpec) && !is_array($mediaTypeSpec['schema'])) { + if (array_key_exists('schema', $mediaTypeSpec) && MalformedSpecNode::isMalformed($mediaTypeSpec['schema'])) { return new RequestBodyValidationResult([ - "Malformed 'requestBody.content[\"{$mediaType}\"].schema' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'requestBody.content[\"%s\"].schema' for %s %s in '%s' spec: expected object, got %s.", + $mediaType, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($mediaTypeSpec['schema']), + ), ]); } } diff --git a/src/Validation/Response/ResponseBodyValidator.php b/src/Validation/Response/ResponseBodyValidator.php index a2e9d6c..1fb64e6 100644 --- a/src/Validation/Response/ResponseBodyValidator.php +++ b/src/Validation/Response/ResponseBodyValidator.php @@ -12,6 +12,7 @@ use Studio\OpenApiContractTesting\SchemaContext; use Studio\OpenApiContractTesting\Spec\OpenApiSchemaConverter; use Studio\OpenApiContractTesting\Validation\Support\ContentTypeMatcher; +use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -67,25 +68,43 @@ public function validate( ): ResponseBodyValidationResult { // Pre-scan the content map for malformed media-type entries before any // content negotiation runs. This mirrors RequestBodyValidator's - // per-media-type guards: a scalar entry would slip past the downstream - // `isset(...['schema'])` checks as a silent pass, and a non-array - // `schema` on a JSON media type would reach OpenApiSchemaConverter as a - // scalar and raise a confusing TypeError. Surface both as loud - // spec-level errors (issue #256). `matchedContentType` is null: no - // content-type lookup succeeded. + // per-media-type guards: a media-type entry and its `schema` must each + // decode to a JSON object — a scalar entry would slip past the + // downstream `isset(...['schema'])` checks as a silent pass, a non-array + // `schema` on a JSON media type would reach OpenApiSchemaConverter and + // raise a confusing TypeError, and a JSON list mis-resolves silently. + // Surface all of them as loud spec-level errors via + // {@see MalformedSpecNode} (issue #256). `matchedContentType` is null: + // no content-type lookup succeeded. foreach ($content as $mediaType => $mediaTypeSpec) { - if (!is_array($mediaTypeSpec)) { + if (MalformedSpecNode::isMalformed($mediaTypeSpec)) { return new ResponseBodyValidationResult([ - "Malformed 'responses[{$statusCode}].content[\"{$mediaType}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'responses[%s].content[\"%s\"]' for %s %s in '%s' spec: expected object, got %s.", + $statusCode, + $mediaType, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($mediaTypeSpec), + ), ], null); } // array_key_exists rather than isset so an explicit `schema: null` // is also flagged — otherwise it falls through the downstream // presence check as a silent "no schema" pass. - if (array_key_exists('schema', $mediaTypeSpec) && !is_array($mediaTypeSpec['schema'])) { + if (array_key_exists('schema', $mediaTypeSpec) && MalformedSpecNode::isMalformed($mediaTypeSpec['schema'])) { return new ResponseBodyValidationResult([ - "Malformed 'responses[{$statusCode}].content[\"{$mediaType}\"].schema' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + sprintf( + "Malformed 'responses[%s].content[\"%s\"].schema' for %s %s in '%s' spec: expected object, got %s.", + $statusCode, + $mediaType, + $method, + $matchedPath, + $specName, + MalformedSpecNode::describe($mediaTypeSpec['schema']), + ), ], null); } } diff --git a/src/Validation/Support/MalformedSpecNode.php b/src/Validation/Support/MalformedSpecNode.php new file mode 100644 index 0000000..f26f9f3 --- /dev/null +++ b/src/Validation/Support/MalformedSpecNode.php @@ -0,0 +1,68 @@ + $node + */ + public static function isMalformed(mixed $node): bool + { + if (!is_array($node)) { + return true; + } + + return $node !== [] && array_is_list($node); + } + + /** + * Human-readable type of a malformed node for the + * "expected object, got X" diagnostic. Only meaningful when + * {@see self::isMalformed()} returned true. + * + * A malformed array is always a non-empty list, reported as `list`; + * anything else is a scalar / `null`, reported via `get_debug_type()` + * (`string`, `int`, `float`, `bool`, `null`). + */ + public static function describe(mixed $node): string + { + return is_array($node) ? 'list' : get_debug_type($node); + } +} diff --git a/tests/Unit/OpenApiRequestValidatorTest.php b/tests/Unit/OpenApiRequestValidatorTest.php index ddb2bf2..7fb5c14 100644 --- a/tests/Unit/OpenApiRequestValidatorTest.php +++ b/tests/Unit/OpenApiRequestValidatorTest.php @@ -501,7 +501,7 @@ public function malformed_request_body_returns_failure(): void $this->assertFalse($result->isValid()); $this->assertStringContainsString("Malformed 'requestBody'", $result->errors()[0]); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -536,7 +536,7 @@ public function scalar_content_media_type_returns_failure(): void $this->assertFalse($result->isValid()); $this->assertStringContainsString("Malformed 'requestBody.content[\"application/json\"]'", $result->errors()[0]); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -557,7 +557,7 @@ public function scalar_content_media_type_schema_returns_failure(): void "Malformed 'requestBody.content[\"application/json\"].schema'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } /** @@ -728,6 +728,70 @@ public function null_operation_returns_failure(): void $this->assertStringContainsString('expected object, got null', $result->errors()[0]); } + #[Test] + public function list_paths_node_returns_failure(): void + { + // The spec's root `paths` is a JSON list (`[...]`). A list passes + // `is_array()` but is not an object — the guard surfaces it as a loud + // malformed-spec error rather than letting integer keys mis-resolve + // (issue #259). + $result = $this->validator->validate( + 'malformed-paths-list', + 'POST', + '/things', + [], + [], + ['foo' => 'bar'], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_path_item_returns_failure(): void + { + // `paths["/list-path-item"]` is a JSON list (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-path-item', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/list-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_operation_returns_failure(): void + { + // `paths["/list-operation"].get` is a JSON list (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-operation', + [], + [], + null, + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/list-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + // ======================================== // Constructor validation (mirrors response validator) // ======================================== diff --git a/tests/Unit/OpenApiResponseValidatorTest.php b/tests/Unit/OpenApiResponseValidatorTest.php index de39045..cd6743b 100644 --- a/tests/Unit/OpenApiResponseValidatorTest.php +++ b/tests/Unit/OpenApiResponseValidatorTest.php @@ -2083,7 +2083,7 @@ public function malformed_response_content_block_returns_failure(): void "Malformed 'responses[200].content'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -2106,7 +2106,7 @@ public function malformed_response_content_media_type_entry_returns_failure(): v 'Malformed \'responses[200].content["application/json"]\'', $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -2129,7 +2129,7 @@ public function malformed_response_content_schema_returns_failure(): void 'Malformed \'responses[200].content["application/json"].schema\'', $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -2177,7 +2177,7 @@ public function malformed_response_status_entry_returns_failure(): void "Malformed 'responses[200]'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -2202,7 +2202,7 @@ public function malformed_response_status_entry_keys_message_off_matched_spec_ke "Malformed 'responses[default]'", $result->errors()[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + $this->assertStringContainsString('expected object, got string', $result->errors()[0]); } #[Test] @@ -2442,4 +2442,117 @@ public function malformed_responses_map_surfaces_even_for_skip_status(): void $result->errors()[0], ); } + + #[Test] + public function list_paths_node_returns_failure(): void + { + // The spec's root `paths` is a JSON list (`[...]`). A list passes + // `is_array()` but is not an object: `array_keys()` would yield + // integer keys that mis-resolve silently against every request path. + // The guard surfaces it as a loud malformed-spec error (issue #259). + $result = $this->validator->validate( + 'malformed-paths-list', + 'GET', + '/things', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString("Malformed 'paths'", $result->errors()[0]); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_path_item_returns_failure(): void + { + // `paths["/list-path-item"]` is a JSON list. A list path item passes + // `is_array()` but has no method keys, so it would mis-resolve to a + // misleading "method not defined" — the guard surfaces it as + // malformed instead (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-path-item', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/list-path-item\"]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_operation_returns_failure(): void + { + // `paths["/list-operation"].get` is a JSON list (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-operation', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/list-operation\"].get'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_responses_map_returns_failure(): void + { + // `paths["/list-responses-map"].get.responses` is a JSON list. A list + // passes `is_array()` and would reach `SpecResponseKeyResolver` with + // integer keys that never match a status — the guard surfaces it as + // malformed instead (issue #259). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-responses-map', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'paths[\"/list-responses-map\"].get.responses'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } + + #[Test] + public function list_response_status_entry_returns_failure(): void + { + // `responses["200"]` is a JSON list. The per-entry guard (issue #258) + // now routes through MalformedSpecNode, so a list response entry is + // surfaced with the same loud diagnostic as a scalar one. + $result = $this->validator->validate( + 'malformed', + 'GET', + '/list-response-status', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'responses[200]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors()[0]); + } } diff --git a/tests/Unit/Validation/Request/RequestBodyValidatorTest.php b/tests/Unit/Validation/Request/RequestBodyValidatorTest.php index 690ba49..46af148 100644 --- a/tests/Unit/Validation/Request/RequestBodyValidatorTest.php +++ b/tests/Unit/Validation/Request/RequestBodyValidatorTest.php @@ -279,7 +279,58 @@ public function validate_flags_malformed_media_type_schema(): void $this->assertCount(1, $result->errors); $this->assertStringContainsString('.schema\'', $result->errors[0]); - $this->assertStringContainsString('expected object, got scalar', $result->errors[0]); + $this->assertStringContainsString('expected object, got string', $result->errors[0]); + } + + #[Test] + public function validate_flags_list_request_body(): void + { + // A `requestBody` written as a JSON list passes `is_array()` but is + // not an object. The shared MalformedSpecNode guard surfaces it with + // the same loud diagnostic as a scalar `requestBody` (issue #256). + $operation = ['requestBody' => ['this should have been an object']]; + + $result = $this->validator->validate( + 'spec', + 'POST', + '/pets', + $operation, + DecodedBody::absent(), + null, + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString("Malformed 'requestBody'", $result->errors[0]); + $this->assertStringContainsString('expected object, got list', $result->errors[0]); + } + + #[Test] + public function validate_flags_list_media_type_schema(): void + { + // A `schema` written as a JSON list is malformed the same way — a + // list is not a JSON Schema object (issue #256). + $operation = [ + 'requestBody' => [ + 'content' => [ + 'application/json' => ['schema' => ['this should have been an object']], + ], + ], + ]; + + $result = $this->validator->validate( + 'spec', + 'POST', + '/pets', + $operation, + DecodedBody::absent(), + null, + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString('.schema\'', $result->errors[0]); + $this->assertStringContainsString('expected object, got list', $result->errors[0]); } #[Test] diff --git a/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php b/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php index f7c1bdf..4ab4dd3 100644 --- a/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php +++ b/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php @@ -541,7 +541,7 @@ public function validate_flags_malformed_media_type_entry(): void 'Malformed \'responses[200].content["application/json"]\'', $result->errors[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors[0]); + $this->assertStringContainsString('expected object, got string', $result->errors[0]); $this->assertNull($result->matchedContentType); } @@ -569,10 +569,63 @@ public function validate_flags_malformed_media_type_schema_for_json_content_type 'Malformed \'responses[200].content["application/json"].schema\'', $result->errors[0], ); - $this->assertStringContainsString('expected object, got scalar', $result->errors[0]); + $this->assertStringContainsString('expected object, got string', $result->errors[0]); $this->assertNull($result->matchedContentType); } + #[Test] + public function validate_flags_list_media_type_entry(): void + { + // A media-type entry written as a JSON list passes `is_array()` but is + // not an object. The shared MalformedSpecNode guard surfaces it with + // the same loud diagnostic as a scalar entry (issue #256). + $content = ['application/json' => ['this should have been an object']]; + + $result = $this->validator->validate( + 'spec', + 'GET', + '/pets', + 200, + $content, + DecodedBody::present(['id' => 1]), + 'application/json', + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["application/json"]\'', + $result->errors[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors[0]); + } + + #[Test] + public function validate_flags_list_media_type_schema(): void + { + // A `schema` written as a JSON list is malformed the same way + // (issue #256). + $content = ['application/json' => ['schema' => ['this should have been an object']]]; + + $result = $this->validator->validate( + 'spec', + 'GET', + '/pets', + 200, + $content, + DecodedBody::present(['id' => 1]), + 'application/json', + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["application/json"].schema\'', + $result->errors[0], + ); + $this->assertStringContainsString('expected object, got list', $result->errors[0]); + } + #[Test] public function validate_flags_null_media_type_schema_for_json_content_type(): void { diff --git a/tests/Unit/Validation/Support/MalformedSpecNodeTest.php b/tests/Unit/Validation/Support/MalformedSpecNodeTest.php new file mode 100644 index 0000000..846a800 --- /dev/null +++ b/tests/Unit/Validation/Support/MalformedSpecNodeTest.php @@ -0,0 +1,70 @@ + + */ + public static function provideIs_malformed_rejects_scalars_null_and_listsCases(): iterable + { + yield 'string scalar' => ['this should have been an object']; + yield 'int scalar' => [42]; + yield 'float scalar' => [3.14]; + yield 'bool scalar' => [true]; + yield 'null' => [null]; + yield 'non-empty list' => [['a', 'b']]; + yield 'single-element list' => [['only']]; + yield 'nested list' => [[['a']]]; + } + + /** + * @return iterable + */ + public static function provideIs_malformed_accepts_objects_and_the_empty_nodeCases(): iterable + { + yield 'map with string keys' => [['get' => [], 'post' => []]]; + yield 'single-key map' => [['200' => ['description' => 'OK']]]; + // `{}` and `[]` both decode to `[]` in PHP: an empty node is + // ambiguous and treated as an (empty) object, never malformed. + yield 'empty array' => [[]]; + } + + #[Test] + #[DataProvider('provideIs_malformed_rejects_scalars_null_and_listsCases')] + public function is_malformed_rejects_scalars_null_and_lists(mixed $node): void + { + $this->assertTrue(MalformedSpecNode::isMalformed($node)); + } + + #[Test] + #[DataProvider('provideIs_malformed_accepts_objects_and_the_empty_nodeCases')] + public function is_malformed_accepts_objects_and_the_empty_node(mixed $node): void + { + $this->assertFalse(MalformedSpecNode::isMalformed($node)); + } + + #[Test] + public function describe_reports_scalar_and_null_types_via_get_debug_type(): void + { + $this->assertSame('string', MalformedSpecNode::describe('x')); + $this->assertSame('int', MalformedSpecNode::describe(42)); + $this->assertSame('float', MalformedSpecNode::describe(3.14)); + $this->assertSame('bool', MalformedSpecNode::describe(true)); + $this->assertSame('null', MalformedSpecNode::describe(null)); + } + + #[Test] + public function describe_reports_a_non_empty_list_as_list(): void + { + $this->assertSame('list', MalformedSpecNode::describe(['a', 'b'])); + } +} diff --git a/tests/fixtures/specs/malformed-paths-list.json b/tests/fixtures/specs/malformed-paths-list.json new file mode 100644 index 0000000..0b869e3 --- /dev/null +++ b/tests/fixtures/specs/malformed-paths-list.json @@ -0,0 +1,8 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Malformed Paths Fixture (JSON list)", + "version": "0.1.0" + }, + "paths": ["this should have been an object, not a list"] +} diff --git a/tests/fixtures/specs/malformed.json b/tests/fixtures/specs/malformed.json index 424ac62..a712c9a 100644 --- a/tests/fixtures/specs/malformed.json +++ b/tests/fixtures/specs/malformed.json @@ -208,6 +208,26 @@ "responses": null } }, + "/list-path-item": ["this should have been an object, not a list"], + "/list-operation": { + "get": ["this should have been an object, not a list"] + }, + "/list-responses-map": { + "get": { + "summary": "responses map is a JSON list (not an object)", + "operationId": "listResponsesMap", + "responses": ["this should have been an object, not a list"] + } + }, + "/list-response-status": { + "get": { + "summary": "responses[200] entry is a JSON list (not an object)", + "operationId": "listResponseStatus", + "responses": { + "200": ["this should have been an object, not a list"] + } + } + }, "/scalar-operation-template/{id}": { "get": "this should have been an object" },