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.

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

metaschema permissiveness (why are unsupported keywords allowed?) #577

Closed
russwe opened this issue Mar 26, 2018 · 6 comments
Closed

metaschema permissiveness (why are unsupported keywords allowed?) #577

russwe opened this issue Mar 26, 2018 · 6 comments

Comments

@russwe
Copy link

russwe commented Mar 26, 2018

I've recently written (and validated) a schema that contained "requiredProperties": [] rather than "required": [] and accidentally let a much more permissive schema into the wild than intended. Is there a reason this sort of error should be allowed, or is this simply an oversight in the existing drafts?

Maybe I'm missing a use-case that "free-form" keywords in the schema allow?

@russwe
Copy link
Author

russwe commented Mar 26, 2018

TL;DR: Why isn't "additionalProperties": false for the meta-schema as a whole?

@Julian
Copy link
Member

Julian commented Mar 26, 2018

Not to take away from an independent question that it sounds like you're asking ("should the meta schema disallow additionalProperties by default", which I believe has some previous discussion somewhere that I'm sure @handrews will know about), but I'd say if you didn't have unit tests for your schema you have got some issues that defaults won't fix :). They can make it slightly harder, but I'd still always recommend personally that you have a suite that's testing examples of instances you expect to be valid and invalid.

@russwe
Copy link
Author

russwe commented Mar 26, 2018

That is also an excellent point, not to be discounted. While the schema did undergo testing, this was clearly a miss... in part because the intention was clear in the schema and we all know the spiel about assumptions.

It would be nice, though, to have an additional layer, especially one that would allow simply pulling referenced meta-schema to do validation of the actual schema at runtime ;)

@handrews
Copy link
Contributor

@russwe JSON Schema is specifically designed as an extensible system. Per JSON Schema Core §4.3.1:

A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be ignored.

Setting "additionalProperties": false in the meta-schema would directly contradict that requirement.

You can, of course, write your own meta-schema, add "additionalProperties": false where desired, and test your schemas against that. I have a build step that does just that in the schema repository that I use currently. Currently, the downside of using such a meta-schema in $schema is that if your validator checks $schema and expects the standard meta-schema, it will get confused. Although many validators ignore $schema in favor of whatever you tell them to do on the command line or through configuration.

With #561 (vocabulary support, targeted for draft-08) it will be easier to write your own meta-schema but still have implementations recognize the underlying standard vocabulary. Once draft-08 is published and implemented you could use that plus #556 (unevaluatedProperties, also targeted for draft-08) to very easily write a meta-schema that does this, but is still recognizable as using the standard validation vocabulary.

@handrews
Copy link
Contributor

@russwe hopefully this makes sense- it's done this way because schemas are a constraint system, and once you add a constraint to a schema, no one else who re-uses a schema can remove that constraint.

Once we have unevaluatedProperties, you can easily do {"allOf": [{"$ref": "http://json-schema.org/draft-08/schema"}], "unevaluatedProperties": false} and get the behavior you want. Or you can do something more complex with vocabularies. These changes are already tracked in other issues so I'm going to close this one.

@russwe
Copy link
Author

russwe commented Mar 27, 2018

That absolutely makes sense. I missed 4.3.1, though I was looking for something similar. I'll look over the links you've provided and determine the best path forward for us... certainly it is no real hardship to add the "additionalProperties": false for internal validation.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants