ExodusMovement / schemasafe

A reasonably safe JSON Schema validator with draft-04/06/07/2019-09/2020-12 support.
https://npmjs.com/@exodus/schemasafe
MIT License
155 stars 12 forks source link

Add schema validation for unknown keys in required property #150

Closed ariesclark closed 1 year ago

ariesclark commented 2 years ago
{
    "type": "object",
    "required": ["foo", "bar"],
    "properties": {
        "foo": {
            "type": "number"
        }
    }
}

I think the schemasafe strict? validator should throw an error in this case.

ChALkeR commented 1 year ago

Hm. Technically, this is not an error per the JSON Schema spec, and has a reasonable meaning.

Schema like that doesn't make sense if there is an immediate additionalProperties: false there though.

And using strong mode of schemasafe, it will ensure that all properties should be validated.

But they could be validated via more complex rules than the one here, and e.g. doing this is possible even in strong mode:

{
  $schema: 'http://json-schema.org/draft/2020-12/schema#',
  type: 'object',
  required: ['foo', 'bar'],
  properties: {
    foo: {
      type: 'number',
    },
  },
  allOf: [
    {
      required: [],
      properties: {
        bar: {
          type: 'number',
        },
      },
    },
  ],
  unevaluatedProperties: false,
}

Also, a more complex example would be possible with refs.

That said, a validation of required in cases where we can statically prove that those properties are not possible can be done. Just not with this example (as it's missing additionalProperites or unevaluatedProperties).

Sorry for the late reply.

ChALkeR commented 1 year ago

Implemented in 1007440914ba51ba0264c972c2dd455f63642c72.

Not that this check is present only for cases when either additionalProperties: false or a statically-provable unevaluatedProperties: false are used, so we can be sure that required attempts to list a property that is forbidden by either of those.

Upd: old test fixup in 22b16f0dba6fa7cf5400af487c5c1dc9d9b71a22.

ChALkeR commented 1 year ago

This check also works with inherited required, i.e. this schema throws at compile time:

{
  type: 'object',
  required: [],
  properties: {
    foo: {
      type: 'number',
    },
  },
  allOf: [
    {
      required: ['foo', 'bar'],
    },
  ],
  unevaluatedProperties: false,
}

~That said, additionalProperties: false checks only adjacent required for now, so this would pass with additionalProperties instead of unevaluatedProperties.~

~Let's reopen as there is room for improvement.~

Upd: fixed in efd613a.