fix(validation): guard response validation against malformed content and response specs#257
Merged
Merged
Conversation
…t schemas (#256) ResponseBodyValidator lacked the malformed-schema guards that RequestBodyValidator already had, so the two sides of the contract behaved asymmetrically on a broken spec. Previously, a non-array content[mediaType] entry or a non-array `schema` could either trigger a confusing TypeError deep in the converter, or — when `schema` was null — slip through as a silent pass that validated nothing. This adds symmetric guards so both cases are surfaced as a loud spec-level error. OpenApiResponseValidator::validateBody() also now guards a non-array `content` block before delegating. TDD tests and a fixture cover both the TypeError and silent-pass regressions.
…#256) Add review-feedback tests for the malformed-schema guards introduced in the previous commit. No production code change. - Add orchestrator-level OpenApiResponseValidator tests for the malformed media-type entry and malformed/null schema guards, for parity with the request-side OpenApiRequestValidatorTest. - Add a non-JSON non-null scalar schema case. - Pin that the guard pre-scans every media-type entry regardless of content negotiation. Adds 3 fixture paths to malformed.json and 5 test methods total.
When a responses[$status] spec entry is a non-array scalar (e.g. an unresolved $ref), the scalar reached the `array $responseSpec` parameter of validateBody()/validateHeaders() and raised an uncaught TypeError. Since TypeError extends Error rather than RuntimeException, it bypassed the validator's error handling and surfaced as a hard crash instead of a validation result. validate() now detects this case and returns a loud `Malformed 'responses[...]'` spec error via OpenApiValidationResult::failure(), mirroring the content-level guards already added in this PR and RequestBodyValidator's `requestBody` guard. A failing test that reproduced the TypeError was added first, then the guard. Closes #258
…us guard (#258) Add malformed_response_status_entry_keys_message_off_matched_spec_key, which exercises the wire-status-vs-spec-key dimension the prior test missed: a spec that declares only a responses[default] entry, hit by a wire status that resolves to default. This confirms the responses[$status] guard's error message names the matched spec key rather than the raw wire status code. A new /response-default-status-scalar fixture path in malformed.json backs the test. The OpenApiResponseValidator comment rewording and the test docblock tweak are non-functional, clarifying that the (issue #258) tag scopes the new guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ResponseBodyValidatorandOpenApiResponseValidatorlacked themalformed-schema guards that
RequestBodyValidatoralready had, so the twosides of the contract behaved asymmetrically on a broken spec. This adds
symmetric guards at every level of the response spec so a non-array
responses[$status]entry, a non-arraycontentblock, a non-arraycontent[mediaType]entry, or a non-arrayschemais surfaced as a loudspec-level error.
Why
Fixes #256. Fixes #258.
With no guard, a malformed
responsesmap produced one of two bad outcomes onthe response side:
TypeError— a non-arrayresponses[$status]entry, a non-arraycontentblock, or a non-arrayschemaon a JSON media type reached anarray-typed parameter (validateBody()/validateHeaders()/ResponseBodyValidator::validate()) orOpenApiSchemaConverter::convert().TypeErrorextendsError, notRuntimeException, sovalidateBody()'scatch never saw it — the run crashed with a stack trace instead of a readable
spec error.
schema: nullwasread as "no schema" by the
isset(...['schema'])skip check (issue tech-debt(validation) — spec が宣言した非 JSON Content-Type のボディが診断なしで素通りする #254) andslipped through as a clean success. The request side rejects the same
schema: nullloudly, so request and response disagreed.Both contradict the contract-testing principle of surfacing malformed specs
rather than masking them, and broke parity with
RequestBodyValidator's guards.#258 (the non-array
responses[$status]entry) was originally split out as afollow-up; it is the same logical change one level up, so it is folded into this
PR.
Verification
Failing tests were added first (each reproducing a
TypeErroror a silentpass), then the guards were implemented.
array_key_exists(notisset) is usedso an explicit
schema: nullis also flagged.A multi-agent review (
/pr-review-toolkit:review-pr) prompted additional tests:orchestrator-level tests for the malformed media-type-entry and malformed/null
schemaguards (for parity with the request-sideOpenApiRequestValidatorTest),a non-JSON non-null scalar
schemacase, and a test pinning that the guardpre-scans every media-type entry regardless of content negotiation.
composer testpasses (1815 tests)composer stanpassescomposer cs-checkpassesNotes for reviewers
responses[$status]andcontent-block guards inOpenApiResponseValidator::validate()/validateBody()(theirarray-typedparameters are the crash site), and the per-entry / per-
schemaguards inResponseBodyValidator::validate(), mirroringRequestBodyValidator.responses[...]status in the error message uses the matched spec key forthe
responses[$status]guard (a literal spec pointer) and the wire statuscode for the
content-level guards (consistent with the validator's existing(status N)messages). When the matched key is a range (5XX) ordefault,the wire-status form is not a literal pointer but is enough to locate the
malformed
content.above
responses[$status]— a non-arraypaths, path item, operation, orresponsesmap — still raises an uncaughtTypeErrorin the spec-traversalpath of
validate(). Hardening that traversal is a separate concern from theper-response malformed guards this PR adds.