Skip to content

unevaluatedProperties should not affect sub-schemas #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aznan2 opened this issue Jun 19, 2023 · 6 comments · Fixed by #827
Closed

unevaluatedProperties should not affect sub-schemas #826

aznan2 opened this issue Jun 19, 2023 · 6 comments · Fixed by #827

Comments

@aznan2
Copy link
Contributor

aznan2 commented Jun 19, 2023

unevaluatedProperties gets its set of evaluated properties from adjacent properties, patternProperties, additionalProperties and unevaluatedProperties . This means that unevaluated properties from sub-schemas should not be considered. That is, given this schema:

{
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "properties": {
    "foo": {}
  },
  "unevaluatedProperties": false
}

the following instance should be valid:

{
  "foo": {
    "bar": "baz"
  }
}

This is currently not the case - if "unevaluatedProperties": false is set at the root of a schema, then any deeply nested additional property will result in a validation error.

@manuelwaltschek
Copy link

manuelwaltschek commented Jun 20, 2023

I found this to be actually useful. I don't know if this violates the spec, but is it possible to make this configurable instead of a fixed rule?

I always found it cumbersome to add "additionalProperties" or "unevaluatedProperties" to each nested level when all I want to have is a very strict schema

@aznan2
Copy link
Contributor Author

aznan2 commented Jun 20, 2023

It should be easy to implement it so it reverts to the old (current) behavior if a configuration flag is set. I don't know if that is desirable though. Redefining JSON schema's validation rules feels a bit icky.

@fdutton
Copy link
Contributor

fdutton commented Jun 20, 2023

@aznan2 This is the behavior defined in the specification. The JSON Schema Team provides a Test Suite for implementors to validate that their implementation conforms to the specification and this behavior is explicitly checked.

You can add "unevaluatedProperties": true to a nested schema if you wish to override the default behavior.

@aznan2
Copy link
Contributor Author

aznan2 commented Jun 20, 2023

@fdutton Huh. Can you point to the test case? I looked for one, but didn't manage to find it.

I was a bit apprehensive at first, but the wording in the spec that unevaluatedProperties depends on adjacent keywords swayed me. Along with the circumstantial evidence that every other validation library i tried said that the instance in the OP was valid.

Also, in the original discussion at json-schema-org/json-schema-spec#556, the refactored OAS 3.0 schema contains "unevaluatedProperties": false all over the place as well as in the root. This shouldn't be necessary if unevaluatedProperties can look inside sub-schemas.

Again, all of that is circumstantial, so I would be happy to be proven wrong.

@fdutton
Copy link
Contributor

fdutton commented Jun 21, 2023

Sorry for the delay. I meant to post this yesterday but forgot to click the button.

@aznan2 I'm reading the spec again and reviewing some conversations from the JSON Schema slack channel so I need more time to complete the review of your pull-request.

Currently, I am agreeing with your position and think that my understanding of the spec is flawed. If your interpretation is correct, the pull-request will need some additional work to stop propagating evaluated properties and items up the stack.

@aznan2
Copy link
Contributor Author

aznan2 commented Jun 22, 2023

@fdutton Sure, and thanks for looking into this. Do you see more cases that we need to test for where nested properties are still considered after the patch?

stevehu pushed a commit that referenced this issue Jun 28, 2023
…ively (#827)

* Add support for subschema references in getSchema(URI) (#619)

* Fixes unevaluatedProperties and unevaluatedItems being applied recursively (#826)

---------

Co-authored-by: Matti Hansson <[email protected]>
Co-authored-by: mathan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants