diff --git a/src/OpenApiResponseValidator.php b/src/OpenApiResponseValidator.php index e2a583a..4174c19 100644 --- a/src/OpenApiResponseValidator.php +++ b/src/OpenApiResponseValidator.php @@ -189,6 +189,21 @@ 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)) { + return OpenApiValidationResult::failure([ + "Malformed 'responses[{$matchedResponseKey}]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], $matchedPath, $statusCodeStr); + } + + /** @var array $responseSpec */ $bodyResult = $this->validateBody( $specName, $method, @@ -412,7 +427,19 @@ private function validateBody( return new ResponseBodyValidationResult([], null); } - /** @var array> $content */ + // 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'])) { + return new ResponseBodyValidationResult([ + "Malformed 'responses[{$statusCode}].content' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], null); + } + + /** @var array $content */ $content = $responseSpec['content']; // Inlined try/catch mirrors ValidatorErrorBoundary::safely() for the diff --git a/src/Validation/Response/ResponseBodyValidator.php b/src/Validation/Response/ResponseBodyValidator.php index 1d1d030..a2e9d6c 100644 --- a/src/Validation/Response/ResponseBodyValidator.php +++ b/src/Validation/Response/ResponseBodyValidator.php @@ -15,6 +15,7 @@ use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; +use function array_key_exists; use function array_keys; use function implode; use function in_array; @@ -49,7 +50,10 @@ public function __construct( * {@see OpenApiResponseValidator::validate()} — when the response spec * has no `content` key, this validator is never invoked. * - * @param array> $content the `responses[$status].content` map + * @param array $content the `responses[$status].content` map. + * Values are media-type objects, but a malformed spec can + * carry a scalar — the guard loop below rejects those loudly + * before any value is dereferenced as an array. */ public function validate( string $specName, @@ -61,6 +65,31 @@ public function validate( ?string $responseContentType, OpenApiVersion $version, ): 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. + foreach ($content as $mediaType => $mediaTypeSpec) { + if (!is_array($mediaTypeSpec)) { + return new ResponseBodyValidationResult([ + "Malformed 'responses[{$statusCode}].content[\"{$mediaType}\"]' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], 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'])) { + return new ResponseBodyValidationResult([ + "Malformed 'responses[{$statusCode}].content[\"{$mediaType}\"].schema' for {$method} {$matchedPath} in '{$specName}' spec: expected object, got scalar.", + ], null); + } + } + // When the actual response Content-Type is provided, handle content negotiation: // non-JSON types are checked for spec presence only, while JSON-compatible types // fall through to schema validation. For JSON-flavoured response Content-Types @@ -84,11 +113,11 @@ public function validate( // with no `schema` has nothing to validate — stay // silently successful, as before. // - // `isset` treats a degenerate `schema: null` as "no - // schema" (silent success). Unlike the request validator, - // this validator has no upstream malformed-schema guard, - // so a non-array `schema` is not surfaced as a loud spec - // error here — tracked separately (see #256). + // `isset` (not `array_key_exists`) is deliberate: an + // explicit `schema: null` is a degenerate entry, and the + // per-media-type malformed-schema guard above already + // rejected it loudly before this point — so it never + // reaches here as a silent "no schema" case. if (isset($content[$matchedKey]['schema'])) { return new ResponseBodyValidationResult( [], diff --git a/tests/Unit/OpenApiResponseValidatorTest.php b/tests/Unit/OpenApiResponseValidatorTest.php index ee516db..fff2438 100644 --- a/tests/Unit/OpenApiResponseValidatorTest.php +++ b/tests/Unit/OpenApiResponseValidatorTest.php @@ -2059,4 +2059,149 @@ public function validate_treats_absent_body_envelope_like_a_bare_null(): void $this->assertFalse($result->isValid()); $this->assertStringContainsString('Response body is empty', $result->errorMessage()); } + + #[Test] + public function malformed_response_content_block_returns_failure(): void + { + // `responses.200.content` is a scalar. Without the guard the scalar + // reaches ResponseBodyValidator::validate()'s `array $content` + // parameter and raises an uncaught TypeError (TypeError extends Error, + // not RuntimeException, so validateBody()'s catch does not see it). + // The guard surfaces a loud spec error instead, mirroring the + // request-side `Malformed 'requestBody.content'` guard (issue #256). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/response-scalar-content', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'responses[200].content'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_response_content_media_type_entry_returns_failure(): void + { + // `responses.200.content["application/json"]` is a scalar. The + // per-media-type guard in ResponseBodyValidator surfaces it as a spec + // error, which the orchestrator turns into a Failure (issue #256). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/response-scalar-content-media-type', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["application/json"]\'', + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_response_content_schema_returns_failure(): void + { + // `responses.200.content["application/json"].schema` is a scalar. + // Without the guard the scalar would reach OpenApiSchemaConverter and + // raise a TypeError; the orchestrator now reports a clean spec error. + $result = $this->validator->validate( + 'malformed', + 'GET', + '/response-scalar-content-schema', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["application/json"].schema\'', + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function null_response_content_schema_returns_failure(): void + { + // Locks `array_key_exists` over `isset` at the orchestrator level: an + // explicit `schema: null` must surface a Failure, not slip through the + // downstream presence check as a silent pass. + $result = $this->validator->validate( + 'malformed', + 'GET', + '/response-null-content-schema', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["application/json"].schema\'', + $result->errors()[0], + ); + } + + #[Test] + public function malformed_response_status_entry_returns_failure(): void + { + // `responses["200"]` is a scalar instead of a response object. Without + // the guard the scalar reaches validateBody()/validateHeaders()' `array + // $responseSpec` parameter and raises an uncaught TypeError (TypeError + // extends Error, not RuntimeException). The guard added for issue #258 + // surfaces a loud spec error, mirroring the content-level guards and + // RequestBodyValidator's `requestBody` guard. + $result = $this->validator->validate( + 'malformed-response', + 'GET', + '/things', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'responses[200]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } + + #[Test] + public function malformed_response_status_entry_keys_message_off_matched_spec_key(): void + { + // The spec declares only `default`; a wire status of 200 resolves to + // the `default` key (SpecResponseKeyResolver runs before the guard). + // The guard's error message must name the matched spec key + // (`responses[default]`), not the wire status — `responses[200]` would + // point at a map entry the spec author never wrote (issue #258). + $result = $this->validator->validate( + 'malformed', + 'GET', + '/response-default-status-scalar', + 200, + ['id' => 1], + 'application/json', + ); + + $this->assertFalse($result->isValid()); + $this->assertStringContainsString( + "Malformed 'responses[default]'", + $result->errors()[0], + ); + $this->assertStringContainsString('expected object, got scalar', $result->errors()[0]); + } } diff --git a/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php b/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php index 766a954..f7c1bdf 100644 --- a/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php +++ b/tests/Unit/Validation/Response/ResponseBodyValidatorTest.php @@ -510,4 +510,180 @@ public function validate_flags_schema_mismatch(): void $this->assertStringContainsString('/id', $result->errors[0]); $this->assertSame('application/json', $result->matchedContentType); } + + // ======================================== + // Malformed-spec guards (issue #256) — symmetric with RequestBodyValidator + // ======================================== + + #[Test] + public function validate_flags_malformed_media_type_entry(): void + { + // `content: {"application/json": "oops"}` — a scalar where a media + // type object was expected. Without the guard the scalar slips past + // the downstream `isset(...['schema'])` presence check and the body + // is silently recorded as a clean pass. Surface a loud spec error, + // mirroring RequestBodyValidator's sibling guard. + $content = ['application/json' => 'oops']; + + $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 scalar', $result->errors[0]); + $this->assertNull($result->matchedContentType); + } + + #[Test] + public function validate_flags_malformed_media_type_schema_for_json_content_type(): void + { + // A non-array `schema` on a JSON media type would reach + // OpenApiSchemaConverter::convert() as a scalar and raise a confusing + // TypeError. The guard turns it into a spec-level error instead. + $content = ['application/json' => ['schema' => 'oops']]; + + $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 scalar', $result->errors[0]); + $this->assertNull($result->matchedContentType); + } + + #[Test] + public function validate_flags_null_media_type_schema_for_json_content_type(): void + { + // Locks in `array_key_exists` over `isset`: an explicit `schema: null` + // must be flagged. With `isset` it would fall through the downstream + // presence check and accept any body — a silent pass. + $content = ['application/json' => ['schema' => null]]; + + $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], + ); + } + + #[Test] + public function validate_flags_null_media_type_schema_for_non_json_content_type(): void + { + // Before the guard, a non-JSON entry with `schema: null` slipped + // through the `isset(...['schema'])` skip check (issue #254) as a + // silent success — asymmetric with the request validator, which + // rejects the same `schema: null` loudly. The guard runs before + // content negotiation, so request and response now agree. + $content = ['text/plain' => ['schema' => null]]; + + $result = $this->validator->validate( + 'spec', + 'GET', + '/pets', + 200, + $content, + DecodedBody::present('blob'), + 'text/plain', + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["text/plain"].schema\'', + $result->errors[0], + ); + $this->assertNull($result->skipReason); + } + + #[Test] + public function validate_flags_malformed_media_type_schema_for_non_json_content_type(): void + { + // A non-null scalar `schema` (the natural shape of an unresolved $ref + // that decoded to a string) on a non-JSON media type. Like the + // `schema: null` case, the guard runs before content negotiation, so + // it is flagged regardless of the actual response Content-Type. + $content = ['text/plain' => ['schema' => 'oops']]; + + $result = $this->validator->validate( + 'spec', + 'GET', + '/pets', + 200, + $content, + DecodedBody::present('blob'), + 'text/plain', + OpenApiVersion::V3_0, + ); + + $this->assertCount(1, $result->errors); + $this->assertStringContainsString( + 'Malformed \'responses[200].content["text/plain"].schema\'', + $result->errors[0], + ); + $this->assertNull($result->skipReason); + } + + #[Test] + public function validate_flags_malformed_entry_even_when_not_the_negotiated_content_type(): void + { + // The guard loop pre-scans every media-type entry before content + // negotiation runs. A malformed `text/plain` entry must be flagged + // even though the JSON response Content-Type would negotiate the + // well-formed `application/json` entry — a malformed spec is surfaced + // regardless of which entry the request would have matched. + $content = [ + 'application/json' => ['schema' => ['type' => 'object']], + 'text/plain' => 'oops', + ]; + + $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["text/plain"]\'', + $result->errors[0], + ); + } } diff --git a/tests/fixtures/specs/malformed.json b/tests/fixtures/specs/malformed.json index b844f3d..2c6fd1d 100644 --- a/tests/fixtures/specs/malformed.json +++ b/tests/fixtures/specs/malformed.json @@ -119,6 +119,73 @@ } } }, + "/response-scalar-content": { + "get": { + "summary": "responses[200].content is a scalar (not an object)", + "operationId": "responseScalarContent", + "responses": { + "200": { + "description": "OK", + "content": "this should have been an object" + } + } + } + }, + "/response-scalar-content-media-type": { + "get": { + "summary": "responses[200].content[mediaType] is a scalar (not an object)", + "operationId": "responseScalarContentMediaType", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": "this should have been an object" + } + } + } + } + }, + "/response-scalar-content-schema": { + "get": { + "summary": "responses[200].content[mediaType].schema is a scalar (not an object)", + "operationId": "responseScalarContentSchema", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": "this should have been an object" + } + } + } + } + } + }, + "/response-null-content-schema": { + "get": { + "summary": "responses[200].content[mediaType].schema is null (explicitly absent value)", + "operationId": "responseNullContentSchema", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": null + } + } + } + } + } + }, + "/response-default-status-scalar": { + "get": { + "summary": "responses[default] entry is a scalar — declares only `default`, so a wire status resolves to the `default` spec key", + "operationId": "responseDefaultStatusScalar", + "responses": { + "default": "this should have been an object" + } + } + }, "/path-scalar-parameter/{id}": { "parameters": [ "this should have been an object"