Skip to content

fix: Merge Path Item level parameters into the operation before request validation - #62

Open
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/61-path-item-parameters
Open

fix: Merge Path Item level parameters into the operation before request validation#62
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/61-path-item-parameters

Conversation

@shadowhand

Copy link
Copy Markdown

Fixes #61.

The bug

PathItem.parameters are parsed into the schema model and then dropped before validation. RequestValidator::validate() receives only the Operation and builds $parameterSchemas from $operation->parameters alone, so parameters declared on the Path Item — which the specification defines as applying to every operation under that path — are never enforced. required: true passes when omitted, and schema constraints (enum, format, …) are never checked, for every location (path, query, header, cookie). Nothing errors, so a spec author gets no signal that a whole class of parameters is unvalidated.

The fix

Validator\Internal\PathItemParameterMerger merges the path item's parameters into the operation's set at the point where the operation is resolved from the path item, so $operation->parameters is already complete by the time it reaches request validation — RequestValidator is unchanged. Per the specification, an operation level parameter overrides the path level one on matching name + in; the merger returns the operation unchanged when the path item declares no parameters, so specs that do not use the feature pay nothing.

Applied at all three sites that resolve an Operation from a PathItem:

  • PathFinder::getOperation() — request validation, including additionalOperations
  • WebhookValidator::extractOperation() — webhooks are Path Item objects and had the identical defect
  • CallbackValidator::extractOperation() — likewise, after $ref resolution

ResponseValidationHandler also resolves an operation from a path item, but response validation does not read parameters, so it is left alone.

Tests

Written test-first; each was watched failing against the unfixed source before the implementation landed.

tests/Functional/Request/PathItemParametersTest.php (8 tests) — the three violations from the issue (required query parameter omitted, query enum, path format: uuid), a valid request still accepted, path item parameters applying to a second operation on the same path, and the override rule in both directions.

tests/Integration/Validator/PathItemParametersMergeTest.php (4 tests) — the same enforcement for webhooks (in: header) and callbacks (in: query).

Two checks on the tests themselves:

  • Stashing PathFinder.php and re-running turns 4 of the 8 functional tests red, so they do target the reported defect rather than existing behaviour.
  • Disabling only the name + in override skip makes operation_level_parameter_overrides_path_item_level_parameter_with_same_name_and_in fail — the path level enum: [alpha, beta] rejects a request the operation level enum: [gamma] allows — so that test genuinely pins the override rule and not just the merge.

The reproduction script from the issue now prints REJECTED for all three cases.

Verification

  • vendor/bin/phpunit — 7143 tests, 14734 assertions, OK. The 2 deprecation notices are pre-existing and unrelated to this change.
  • vendor/bin/psalm — no errors.
  • vendor/bin/php-cs-fixer fix --dry-run — 0 of 880 files to fix.
  • vendor/bin/rector process --dry-run — clean.

CHANGELOG entry added under a new ## [Unreleased] / ### Fixed heading.

…st validation

`PathItem.parameters` were parsed into the schema model but dropped before
validation: `RequestValidator` builds its parameter set from the `Operation`
alone, so `required` and `schema` constraints on parameters hoisted to the
path item were silently unenforced for every location.

`PathItemParameterMerger` merges the path item's parameters into the
operation's set where the operation is resolved from the path item, so
`$operation->parameters` is complete by the time it reaches request
validation. Per the specification, an operation level parameter overrides a
path level one on matching `name` + `in`. Applied at all three resolution
sites: `PathFinder`, `WebhookValidator`, and `CallbackValidator`.

Fixes duyler#61
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.

Path-level parameters are parsed then dropped: PathItem.parameters never reach request validation

1 participant