Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Discrepancy in validation with JSON Schema draft 2020-12 #497

Open
uncenter opened this issue Oct 30, 2023 · 7 comments · May be fixed by #498
Open

Discrepancy in validation with JSON Schema draft 2020-12 #497

uncenter opened this issue Oct 30, 2023 · 7 comments · May be fixed by #498
Labels
schema JSON-schema related issues

Comments

@uncenter
Copy link
Contributor

uncenter commented Oct 30, 2023 •
edited

With this schema, which defines an object with a property test that contains an array of objects that can have certain properties, I am having issues where Taplo should be invalidating a test case but it isn't. This is probably due to something with the new property introduced in this draft, unevaluatedProperties (see this discussion about it).

{
	"$schema": "https://json-schema.org/draft/2020-12/schema",
	"type": "object",
	"properties": {
		"test": {
			"type": "array",
			"items": {
				"$comment": "This schema is closed, and any property used which aren't defined are invalid/not allowed.",
				"type": "object",
				"properties": {
					"foo": { "type": "string" }
				},
				"allOf": [{ "$ref": "#/$defs/bazExtension" }],
				"unevaluatedProperties": false
			}
		}
	},
	"$defs": {
		"bazExtension": {
			"$comment": "This schema can be used as an extension",
			"type": "object",
			"properties": {
				"baz": { "type": "boolean" }
			}
		}
	}
}

These tests should be valid, with any combination of foo and baz together or separate:

{
	"test": [
		{
			"foo": "foo",
			"baz": true
		}
	]
}
{
	"test": [
		{
			"foo": "foo"
		}
	]
}
{
	"test": [
		{
			"baz": true
		}
	]
}
[[test]]
foo = "foo"
baz = true
[[test]]
foo = "foo"
[[test]]
baz = true

And these should be invalid, with an additional property not in the definition:

{
	"test": [
		{
			"foo": "foo",
			"baz": true,
			"otherProperty": true
		}
	]
}
[[test]]
foo = "foo"
baz = true
otherProperty = true

You can test these on https://json-everything.net/json-schema/ for JSON and with taplo obviously for TOML. My issue is that Taplo isn't invalidating the last example with the additional property otherProperty when it should be.
@ia0
Copy link
Collaborator

ia0 commented Oct 30, 2023

Thanks for the report! However, I don't think this can be fixed by Taplo. Taplo is using jsonschema for validation and I can see 2 possibly relevant open issues: Stranger6667/jsonschema-rs#44 and Stranger6667/jsonschema-rs#195. I assume that once those issues are fixed, we would need to update the jsonschema dependency version to benefit from them and thus fix this issue.

@ia0 ia0 added the schema JSON-schema related issues label Oct 30, 2023
@uncenter
Copy link
Contributor Author

Ah okay sorry. I mistakenly assumed Taplo had its own validator! I'll check those issues out.

@uncenter
Copy link
Contributor Author

uncenter commented Oct 30, 2023 •
edited

It looks like unevaluatedProperties should have been implemented in Stranger6667/jsonschema-rs#419 and Stranger6667/jsonschema-rs#423 and we are on the latest version? Hmmm... I'll open an issue with them and let you know when we can update.

@ia0 ia0 linked a pull request Oct 30, 2023 that will close this issue
Draft
@ia0
Copy link
Collaborator

ia0 commented Oct 30, 2023

The fix is simple (see #498). However, I have no idea of the implications and why this is not automatic. Maybe it's because the draft202012 feature is not complete yet? Maybe once Stranger6667/jsonschema-rs#195 is fixed it would be automatic.

@uncenter
Copy link
Contributor Author

Huh alright. Thanks for taking a look.

@salim-b
Copy link

salim-b commented Nov 23, 2023

However, I have no idea of the implications and why this is not automatic. Maybe it's because the draft202012 feature is not complete yet?

Exactly this. As the main author of jsonschema-rs states here:

I think some changes could be done and merged, but we may not expose them (e.g. new keyword impls) until their respective drafts are ready. So, there should be no problem with accumulating changes step-by-step. Though, we can still test them high-level via some cfg(test) items - e.g. conditional Draft201909 enum variant.

@pamburus
Copy link

How about switching from jsonschema-rs to boon?
It looks like it is 100% compatible with Draft 2020-12, 2019-09, Draft 7, Draft 6 and Draft 4.
https://bowtie.report/#/implementations/rust-boon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
schema JSON-schema related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use JSON Schema draft 2020-12 ia0/taplo
4 participants
@ia0 @pamburus @salim-b @uncenter