Skip to content

fix: Honor nullable when it sits beside a composition keyword - #51

Open
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/nullable-alongside-composition
Open

fix: Honor nullable when it sits beside a composition keyword#51
shadowhand wants to merge 1 commit into
duyler:mainfrom
shadowhand:fix/nullable-alongside-composition

Conversation

@shadowhand

Copy link
Copy Markdown

Closes #50.

The bug

In OAS 3.0 a $ref cannot have siblings, so the documented way to make a referenced schema nullable is to wrap it:

data:
  allOf:
    - $ref: '#/components/schemas/FormIdentifier'
  nullable: true

null was rejected. TypeValidator::validate() honors nullable on the schema being validated, but AbstractCompositionalValidator::normalizeForBranch() consulted $subSchema->nullable — the nullability of each branch — and never saw the parent that carries the keyword. AllOfValidator, AnyOfValidator and OneOfValidator therefore dispatched null into every branch, where it failed. OneOfValidatorWithContext::hasNullableSchema() made the same branch-only assumption on the context-carrying path, so both validator families were affected. Adding type: object beside the composition keyword did not help — TypeValidator's early return only skips its own keyword.

Before / after

The issue's repro script, unchanged:

BEFORE  FAIL: All of the schemas must match, but 1 failed
              getErrors() returned 0 error(s)

AFTER   PASS: null relationship accepted

Full matrix via validateSchema(), same branch schema B = {type: object, required: [type, id], properties: {type: {enum: [forms]}, id: {type: string}}} in every row:

parent schema null (before) null (after) valid object
{allOf: [B], nullable: true} FAIL pass pass
{type: object, allOf: [B], nullable: true} FAIL pass pass
{anyOf: [B], nullable: true} FAIL pass pass
{oneOf: [B], nullable: true} FAIL pass pass
{allOf: [B]} (not nullable) FAIL FAIL pass
{allOf: [B + nullable: true]} (branch nullable) pass pass pass
{type: object, nullable: true, …} (no composition) pass pass pass

The fix

AllOfValidator, AnyOfValidator, OneOfValidator and OneOfValidatorWithContext short-circuit when the schema carrying the keyword is nullable, mirroring TypeValidator. For anyOf/oneOf this means the keyword is satisfied — not that a branch matched — so oneOf still enforces exactly-one for non-null data. The rule lives in one place, SchemaValueNormalizer::isNullableSchema(), which TypeValidator now routes through too.

Deliberate boundaries:

  • Gated on nullableAsType. With disableNullableAsType(), null against a nullable composition still fails, matching today's behaviour for plain nullable schemas.
  • Branch-level nullable and OAS 3.1 type: ['object', 'null'] are unchanged, inside and outside compositions. A null member of a type union is ordinary JSON Schema and keeps composing — {type: ['object','null'], allOf: [B]} still rejects null, because allOf is an in-place applicator. Only nullable: true, which OAS 3.0.3 defines as "allows sending a null value for the defined schema", waives branches. Both directions are pinned by tests.
  • Discriminator handling is unaffected; a discriminated oneOf with a nullable parent accepts null and still discriminates non-null data.

Audit of the other SchemaValueNormalizer::normalize() callers

The defect is specific to in-place applicators — a sub-schema applied to the same data instance the parent's nullable covers.

  • No defectPropertiesValidator, ItemsValidator, PrefixItemsValidator and the *WithContext variants normalize a child value against that child's own schema. The sub-schema consulted is the schema being applied to that value, so branch-level nullability is the correct question there; the parent's nullable describes the parent value, not its children.
  • No defectDependentSchemasValidator returns early for non-array data, so null never reaches its normalize() call.
  • No defectNotValidator already accepts null under a nullable parent: a not branch that fails to normalize counts as not-matched, which satisfies not.
  • FixedIfThenElseValidator did share the defect. {if: …, else: …, nullable: true} leaked an InvalidDataTypeException (not even a ValidationException) out of the else branch for null.

The compiler path needs no change: UnsupportedKeywordDetector rejects allOf/anyOf/oneOf, so compiled validators never reach this code.

Tests

Written before the fix and confirmed failing for the documented reason:

  • tests/Unit/Validator/SchemaValidator/NullableCompositionTest.php — the full acceptance matrix via validateSchema(), the nullableAsType: false rows, OAS 3.1 type unions in both directions, oneOf overlap rejection, discriminated oneOf, and the other in-place applicators.
  • tests/Functional/Response/NullableCompositionTest.php — the issue's exact JSON:API-shaped document through the PSR-7 validateResponse() path and its nested properties chain, which is where real documents hit this.
  • Validator-level cases added to AllOfValidatorTest, AnyOfValidatorTest, OneOfValidatorTest and IfThenElseValidatorTest to cover the no-context path.

make tests (7177 tests), make psalm (no errors), make cs-fix and make rector are clean. Scoped make infection over the touched files: covered MSI 90% (threshold 78) — the one escaped mutant in new code is an equivalent ?->-> rewrite inside a ??.

Out of scope

ValidationException::getErrors() returning an empty list for composition failures (the issue's secondary observation) is untouched and still reproduces for genuinely-failing allOf branches — AbstractCompositionalValidator::validateBranch() drops branch errors that are not AbstractValidationError instances. Worth its own issue.

…er#50)

In OAS 3.0 a `$ref` cannot have siblings, so the documented way to make a
referenced schema nullable is to wrap it:

    data:
      allOf:
        - $ref: '#/components/schemas/FormIdentifier'
      nullable: true

`TypeValidator` honors `nullable` on the schema being validated, but the
composition validators consulted `nullable` on each *branch* instead —
`AbstractCompositionalValidator::normalizeForBranch()` and
`OneOfValidatorWithContext` never saw the parent that carries the keyword. So
`null` was dispatched into every branch and failed there. Adding `type: object`
next to the composition keyword did not help: `TypeValidator`'s early return
only skips its own keyword.

`AllOfValidator`, `AnyOfValidator`, `OneOfValidator` and
`OneOfValidatorWithContext` now short-circuit when the schema carrying the
keyword is nullable. `anyOf`/`oneOf` treat this as the keyword being satisfied
rather than as a matching branch, so `oneOf` still enforces exactly-one for
non-null data. Branch-level `nullable` keeps working unchanged, and the
short-circuit is gated on `nullableAsType`.

Audit of the other `SchemaValueNormalizer::normalize()` callers: `properties`,
`items` and `prefixItems` (and their `*WithContext` variants) normalize a
*child* value against that child's own schema, so consulting the sub-schema is
correct there — no defect. `DependentSchemasValidator` returns early for
non-array data, so `null` never reaches its normalize call. `NotValidator`
already accepts `null` under a nullable parent, because a `not` branch that
fails to normalize counts as not-matched. `IfThenElseValidator` did share the
defect and is fixed: `{if: …, else: …, nullable: true}` leaked an
`InvalidDataTypeException` out of the `else` branch for `null`.

The nullable rule now lives in one place,
`SchemaValueNormalizer::isNullableSchema()`, which `TypeValidator` also routes
through.
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.

Combining "nullable" and "allOf" does not work correctly

1 participant