jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 57 forks source link

Many bogus OpenAPI schema errors if coerce "defaults" is turned on. #254

Closed mschout closed 2 years ago

mschout commented 2 years ago

Steps to reproduce the behavior

Enabling $schema->coerce('defaults') causes all kinds of OpenAPI V3 schema validation errors. These errors do not happen if defaults coerce is not enabled.

I have a test case which I will attach in a PR shortly.

Expected behavior

$schema->errors should not return errors if the schema is valid.

Actual behavior

Many errors were returned from Schema->errors. Specifically for this test case:

# Errors: /paths/~1pets/get/parameters/0/required: /oneOf/0/allOf/2/oneOf/0 Missing property.
# /paths/~1pets/get/parameters/0/in: /oneOf/0/allOf/2/oneOf/0 Not in enum list: path.
# /paths/~1pets/get/parameters/0/style: /oneOf/0/allOf/2/oneOf/1 Not in enum list: form, spaceDelimited, pipeDelimited, deepObject.
# /paths/~1pets/get/parameters/0/in: /oneOf/0/allOf/2/oneOf/2 Not in enum list: header.
# /paths/~1pets/get/parameters/0/in: /oneOf/0/allOf/2/oneOf/3 Not in enum list: cookie.
# /paths/~1pets/get/parameters/0/style: /oneOf/0/allOf/2/oneOf/3 Not in enum list: form.
# /paths/~1pets/get/parameters/0/$ref: /oneOf/1 Missing property.
mschout commented 2 years ago

I tried on an older app that is still using v2 spec, and it has the same problem so this is not specfic to OpenAPIv3, for what it's worth.

mschout commented 2 years ago

Bisected this using the test case, and 37abf9862f36bf0aef67441a1152614e5d272f03 is the commit where this broke apparently (but in a different way so maybe thats not the real broken commit)

(Edit: confirmed, the breakage in 37abf9862f36bf0aef67441a1152614e5d272f03 is not related)

mschout commented 2 years ago

Real place that things go wrong is in lib/JSON/Validator/Schema/Draft4.pm in _validate_type_object_properties

This part, where it sticks in default props from the schema:

   if ($self->{coerce}{defaults} and ref $r eq 'HASH' and exists $r->{default} and !exists $data->{$k}) {
      $data->{$k} = $r->{default};
    }

Somehow things go wrong after that. Commenting out the if block, the schema validates fine.

I dumped out $data before this block and after, and if defaults coerce is enabled, it changes in the test case from:

$VAR1 = {
          'type' => 'integer'
        };

To this:

$VAR1 = {
          'exclusiveMaximum' => bless( do{\(my $o = 0)}, 'JSON::PP::Boolean' ),
          'readOnly' => $VAR1->{'exclusiveMaximum'},
          'minItems' => 0,
          'additionalProperties' => bless( do{\(my $o = 1)}, 'JSON::PP::Boolean' ),
          'minProperties' => 0,
          'deprecated' => $VAR1->{'exclusiveMaximum'},
          'nullable' => $VAR1->{'exclusiveMaximum'},
          'type' => 'integer',
          'exclusiveMinimum' => $VAR1->{'exclusiveMaximum'},
          'writeOnly' => $VAR1->{'exclusiveMaximum'},
          'minLength' => 0,
          'uniqueItems' => $VAR1->{'exclusiveMaximum'}
        };
mschout commented 2 years ago

Possibly during schema validation the defaults coercion should be off? Hmmm..