Skip to content

fix: Resolve $ref in requestBody instead of dropping it - #57

Open
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/56-requestbody-ref
Open

fix: Resolve $ref in requestBody instead of dropping it#57
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/56-requestbody-ref

Conversation

@shadowhand

Copy link
Copy Markdown

Fixes #56.

The bug

ComponentTreeBuilder::buildRequestBody() read only description, content and required, so a requestBody that was a Reference Object parsed to an empty RequestBody with the pointer discarded. RequestBodyValidatorWithContext::validate() then returned at its null === $requestBody->content early exit — and the required: true that would otherwise have thrown MissingRequestBodyException had been dropped along with the $ref.

The result was fail-open: every request body behind a $ref went completely unvalidated. Malformed payloads passed, required was not enforced, and unsupported media types were accepted, with no exception and no warning. Inlining the same body validated correctly, so nothing about the spec hinted at the breakage.

The fix

Reference support is wired through the five layers that Parameter and Response already used. The shape mirrors Response at every step rather than inventing anything new:

Layer Change
Schema\Model\RequestBody Adds ref / refSummary / refDescription; jsonSerialize() emits a Reference Object when ref is set
Schema\Parser\Internal\ComponentTreeBuilder buildRequestBody() branches on $ref
Validator\Schema\Internal\DocumentNavigator Accepts #/components/requestBodies/* targets; RefCache widens to match
Validator\Schema\RefResolver{,Interface} Adds resolveRequestBody() and resolveRequestBodyWithOverride()
Validator\Request\RequestBodyValidatorWithContext Dereferences before use

Behaviour matches the response side: chained references follow through, and an unresolvable or wrongly-typed pointer throws UnresolvableRefException instead of failing open. Per OAS 3.1+, a description sibling of $ref overrides the referenced description. Webhooks and callbacks are fixed by the same change, since both validate through RequestValidator — there's a test for the webhook path.

Inline request bodies take an unchanged path: resolveRequestBodyWithOverride() returns the same instance when ref is null.

Tests

21 new tests. I confirmed they fail without the src/ changes (31 failures/errors with src/ reverted, all green with it) so none of them pass vacuously — worth checking, since the pre-fix behaviour was "everything is accepted", which quietly satisfies any happy-path assertion.

  • tests/Functional/Request/RequestBodyRefTest.php — end-to-end: pointer survives parsing, valid body passes, schema violation / missing required property / missing body / unsupported media type are all rejected, chained refs resolve, dangling and mistyped refs throw, sibling description overrides, webhook path, serialization round-trip.
  • tests/Unit/Validator/Schema/RefResolverRequestBodyTest.php — resolver level: caching, chaining, circular refs, external refs, cross-type rejection in both directions (resolveRequestBody on a schema/response, and resolve/resolveParameter/resolveResponse on a request body), and the override semantics.
  • tests/Unit/Schema/Parser/ReferenceOverrideTest.php — adds RequestBody to the existing Reference Object parity suite.

Verified on the full suite: 7163 tests pass (7142 before), psalm reports no errors, php-cs-fixer and rector are clean. The 2 reported deprecations are pre-existing and unrelated (tests that deliberately exercise deprecated factories).

Two things worth a maintainer's call

Both are consequences of mirroring Response, and I flag them rather than assume:

  1. RefResolverInterface gains two methods. Breaking for any external implementor. RefResolver is the only implementor in the repo. Given the interface already carries resolveParameter/resolveResponse pairs, the RequestBody pair is the consistent shape, and the bug can't be fixed without a way to resolve the reference.
  2. RequestBody's constructor gains three leading parameters, so ref is now first — exactly as in Response and Parameter. Breaking for positional construction; every call site in src/ and tests/ uses named arguments, so nothing in-repo is affected. Appending the three instead would avoid the break at the cost of diverging from the other two models. Happy to switch if you'd rather not take the break before 1.0.

`ComponentTreeBuilder::buildRequestBody()` read only `description`,
`content` and `required`, so a `requestBody` that was a Reference Object
parsed to an empty `RequestBody` with the pointer discarded.
`RequestBodyValidatorWithContext::validate()` then returned at its
`null === $requestBody->content` early exit, because the `required: true`
that would have forced a `MissingRequestBodyException` had been dropped
along with the `$ref`.

The result was fail-open: every request body behind a `$ref` went
completely unvalidated. Malformed payloads passed, `required` was not
enforced, and unsupported media types were accepted — with no exception
and no warning. Inlining the same body validated correctly, so the
breakage was invisible from the spec alone.

Reference support is now wired through the five layers that `Parameter`
and `Response` already used:

- `RequestBody` gains `ref`/`refSummary`/`refDescription` and serializes
  back to a Reference Object, matching `Response`.
- `ComponentTreeBuilder::buildRequestBody()` branches on `$ref`.
- `DocumentNavigator` accepts `#/components/requestBodies/*` targets;
  `RefCache` widens to match.
- `RefResolverInterface` gains `resolveRequestBody()` and
  `resolveRequestBodyWithOverride()`.
- `RequestBodyValidatorWithContext::validate()` dereferences before use.

Chained and dangling references behave as they do for responses: chains
follow through, and an unresolvable or wrongly-typed pointer throws
`UnresolvableRefException` rather than failing open. Per OAS 3.1+, a
`description` sibling of `$ref` overrides the referenced description.
Webhooks and callbacks are fixed by the same change, since both validate
through `RequestValidator`.

Inline request bodies take an unchanged path — `resolveRequestBodyWithOverride()`
returns the same instance when `ref` is null.

Closes duyler#56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A $ref in requestBody is silently dropped, so the request body is never validated

1 participant