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

Constraints on numbers do not validate against proper type|format #46

Open
1 task
fredbi opened this issue Dec 18, 2017 · 6 comments
Open
1 task

Constraints on numbers do not validate against proper type|format #46

fredbi opened this issue Dec 18, 2017 · 6 comments
Assignees

Comments

@fredbi
Copy link
Member

fredbi commented Dec 18, 2017

Applies to: numbers or integers with constraints

Constraints such as Maximum, Minimum, MultipleOf are not validated against the proper type and format.
Instead, the constraint operand (boundary or factor) is systematically (and silently) converted as float64, with a risk of information loss.

We should provide spec consistency check between object/param types and constraints,
and avoid possible loss of information when casting int64 to float64.

Don't be mistaken, this one is not about providing bigint arithmetics.

Incompatible operands (e.g. constraining a uint32 with a negative value in Maxium) should be detected.

This kind of error could trigger a warning (please advise), since it is not illegal to specify a constraint with a different type than the constrained value.

NOTE: PR #42 make it work on inline params (validator.go) in presence of default values [a default value will trigger the check, but schemas without default remain skipped].

This should be generalized to all situations (objects, arrays).

Also refered by:

@casualjim
Copy link
Member

I think it's important to note that we have to work within the constraints of multiple languages.
In addition to that this is about json-schema where there is no integer support type only floats exist in json.

Compliance to the constraints with integer bounds are checked by another validator. The spec is a json document, and json-schema is described in json, not go.

@fredbi
Copy link
Member Author

fredbi commented Dec 18, 2017

Thanks for your comment.
My point is to provide support for consistent value checking when validating the spec,
precisely by taking into account language constraints.

This, I believe, is important when using Swagger format extensions for numbers|integers such as: uint32 etc..

@fredbi
Copy link
Member Author

fredbi commented Dec 18, 2017

As we discuss this point, I have 2 questions:

  • there are constants (MAX|MIN)_SAFE_INTEGER defined in swag. I imagined that they were used to validate the spec. They are not and there are no warnings or errors on safe conversion. Should we have some?
  • there is a piece of code in the schema.go which is related to JSON.Number (a string representation of number). We have no test case trying out this section. How could we test this?

@fredbi
Copy link
Member Author

fredbi commented Dec 23, 2017

Fixing #44 did help to support this feature on default values.
The actual checks and type conversions are delivered. All testing could be done with default values.

We just miss the actual call to these validations for schemas bereft of default value

That would come naturally with support of example values, but would require some additional effort to validate constraints boundaries/factor in cases whitout default value nor example (that is, the simplest case... 😀 ).

Also think I will downgrade these checks to the "warning" level.

I guess it is also a good opportunity to check numbers against the JSON SAFE INTEGER and spew a warning.

@fredbi fredbi self-assigned this Dec 23, 2017
@fredbi
Copy link
Member Author

fredbi commented Jan 2, 2018

Here is the full story of what I've currently on the workbench:

  • regardless of the actual go type they are serialized to, json numbers and integers should validate constraints with the native type indicated by their format. This would avoid the infamous issue Error "Must be a multiple of 0.01" for Value=19.99 xeipuuv/gojsonschema#149 other validators struggle with (because they don't use custom formats such as the ones provided by swagger)
  • constraints operands (maximum, minimum, multipleOf) SHOULD validate the format of the value they are supposed to check. This should be a warning in spec validation, and ignored in value validation
  • integers serialized as float json.Number (i.e. string) should be validated against swag.JSON_MAX_SAFE_INTEGER
  • json.Number specific check in schema.go should be moved to typeValidator

Nothing to do with bignum here

@cyberphone
Copy link

Nothing to do with bignum here

@fredbi @casualjim I would claim it is related. OpenAPI seems to be incompatible with https://tools.ietf.org/html/rfc7493 written by the current JSON editor:

I have adopted this RFC and here is my take on how to deal with int64, BigNum, etc.:
https://cyberphone.github.io/doc/security/draft-rundgren-json-canonicalization-scheme.html#rfc.appendix.D

@fredbi fredbi removed the in progress label Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants