-
-
Notifications
You must be signed in to change notification settings - Fork 265
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
Put boundaries on lack of "$schema" behavior #1353
Conversation
jsonschema-core.xml
Outdated
but MUST fall within the following options: | ||
<ul> | ||
<li>Refuse to process the schema, as with unsupported required vocabularies | ||
<li>Assume a specific, documented meta-schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just point out that under this language, it is still valid behavior to proceed with validation and consider all instances valid, or all instances invalid (because I can document my specific documented meta-schema is {}
or an equivalent of {"not": {}}
).
I don't see that as an issue personally, I think there are reasonable reasons that may make sense, but will mention it explicitly since I know there was argument about those cases and whether they're desired behavior or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the main issue is that we can't, in the core spec, restrict the options to just the standard meta-schemas. There are perfectly good reasons to default to something non-standard (e.g. OpenAPI's dialect). There are endless ways to write always-passing and always-failing schemas, so trying to forbid those feels like a bit of a lost cause.
Requiring that one document the meta-schema should at least highlight the possibility, and I suspect people would feel a bit silly documenting {}
as their default meta-schema. But at least it would be clear.
This may well change prior to the next release, but documents the intended range of options so as to avoid crashes or completely arbitrary behavior.
13f1458
to
69dd9a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect this to change a bit as we prepare for the next release, but this is good for now.
Validators should take care that the parsing and validating against schemas does not consume excessive | ||
system resources. | ||
Validators MUST NOT fall into an infinite loop. | ||
Implementations should take care that the parsing and evaluating against schemas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe lose "against"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phrases like "evaluating against"/"evaluated against"/etc. occur throughout the spec, and I was avoiding changing anything but validate->evaluate. I agree that the terminology is not great, but the need to choose better terminology and use it consistently is already being tracked as issue #922, so I'd rather not mess with it in this PR.
Fixes #1315
This may well change prior to the next release, but documents the intended range of options so as to avoid crashes or completely arbitrary behavior. I believe it covers the observed behavior of implementations in the wild, so even though there is a new MUST I don't think it would cause a problem.