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

enforcing correct minimum/maximum constraint formats for specific types #766

Closed
PatMyron opened this issue Dec 22, 2020 · 9 comments
Closed

Comments

@PatMyron
Copy link

PatMyron commented Dec 22, 2020

Described in aws-cloudformation/cloudformation-cli#414:
string uses minLength/maxLength
number/integer uses minimum/maximum/etc.
frequently mixed up and doesn't seem to be caught during jsonschema validation yet

@Julian
Copy link
Member

Julian commented Dec 22, 2020

Hi there. Was this meant to be filed here?

The behavior described from a quick glance at that is the behavior specified by the JSON Schema specification.

Validators which only apply to certain types pass silently on other types.

If you want to protect against that you make sure you're validating type as well essentially.

Let me know if I've misunderstood the intention here.

@Julian Julian closed this as completed Dec 22, 2020
@PatMyron
Copy link
Author

PatMyron commented Dec 22, 2020

Validators which only apply to certain types pass silently on other types.

Asking about this. Could this library catch keywords which only apply to other types during schema validation?

Catching that could be opt-in if you're worried about backwards compatibility / default behavior

@katzj-amzn
Copy link

+1 this would be a huge improvement to the validation.

@Julian Julian reopened this Dec 23, 2020
@Julian
Copy link
Member

Julian commented Dec 23, 2020

I think a decent way to think about this is to write down a metaschema that does what you want.

Not sure it'd even be a very long schema, it's something like {"required": ["type"], "allOf": [{"properties": {"type": {"const" : "string"}}, {"not" : {"properties" : {"minimum": {}, "maximum" : {}}}], ... etc with the other two cases for array and object.

Don't know that I see something particularly special needed at the library level to do so once such a schema is written down.

@PatMyron
Copy link
Author

PatMyron commented Dec 23, 2020

Could be handled just on our side, but could be generally useful since many JSON schemas are mixing up these similar but slightly different JSON schema keywords

@Julian
Copy link
Member

Julian commented Dec 23, 2020

Right that metaschema would be usable by anyone presumably -- it'd be a way to validate that a schema doesn't use validators except within particular types.

Once we had it though anyone who wanted this behavior could validate against it.

@PatMyron
Copy link
Author

PatMyron commented Feb 18, 2021

Actually 10 different min/max JSON schema keywords across 5 different types, which felt like a lot to schematize, so checking in Python now:

aws-cloudformation/cloudformation-cli#675, aws-cloudformation/cloudformation-cli#729

@Julian
Copy link
Member

Julian commented Feb 20, 2021

Fair enough, glad you got a solution that works.

I'm still a bit reluctant to add anything here given that this indeed can be specified within JSON Schema without any special support -- if someone writes down that metaschema that likely would help folks who want to do this (perhaps worth discussing upstream), but for now will close this given that I think a solution like that should address this generically without any additional work for end-users.

@Julian Julian closed this as completed Feb 20, 2021
@PatMyron
Copy link
Author

PatMyron commented Apr 13, 2021

Similar problems with additionalProperties vs additionalItems / pattern vs. patternProperties if anyone investigates

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