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

Support minimum/maximum on quoted numerals #171

Closed
kosty opened this issue Jul 16, 2019 · 7 comments
Closed

Support minimum/maximum on quoted numerals #171

kosty opened this issue Jul 16, 2019 · 7 comments
Assignees

Comments

@kosty
Copy link
Contributor

kosty commented Jul 16, 2019

Context

As mentioned in #118 there is a "loose" mode support for numeric types. Although not part of the JSON spec, the likely reason for such mode is JavaScript's specification of number type which basically uses only 64bit double type, aka 64-bit format IEEE 754-2008. Motivated by issues resulting from this well known behavior some languages converged on using serialized/quoted form when serializing long, double and bigger values to JSON, for example protocol buffers

Since quoted numeric values are accepted with loose typing other JSON Schema validators should handle them as well.

This issue focuses on maximum and minimum validators at the moment.

Sample test

A json payload like this

"9223372036854775807"

should check out agains schema like these

case 1

{
    "$schema":"http://json-schema.org/draft-04/schema#",
    "type": "integer",
    "maximum": 9223372036854775807
}

case 2

{
    "$schema":"http://json-schema.org/draft-04/schema#",
    "type": "integer",
    "maximum": 1E+20
}

case 3

{
    "$schema":"http://json-schema.org/draft-04/schema#",
    "type": "number",
    "maximum": "9223372036854775807.0"
}

case 4

{
    "$schema":"http://json-schema.org/draft-04/schema#",
    "type": "number",
    "maximum": 922337203685477580700
}

Essentially either of integer/number types and any valid JSON number as maximum/minimum value (including quoted numeric type).

Edge case

Not clear if the following combination should trigger a maximum/minimum validators
json

"42"

schema

{
    "$schema":"http://json-schema.org/draft-04/schema#",
    "type": "string",
    "maximum": 100
}
@kosty
Copy link
Contributor Author

kosty commented Jul 16, 2019

Related issue include #106 #132 Also touches upon code submitted as part of #160 #164 #157 #116

@kosty
Copy link
Contributor Author

kosty commented Jul 16, 2019

Side note, current MinimumValidator code has a line with

schemaNode.isLong() || schemaNode.isInt() && JsonType.INTEGER.toString().equals(getNodeFieldType())

assuming it is a typo bug 🐜 and MaximumValidator code has it right

( schemaNode.isLong() || schemaNode.isInt() ) && (JsonType.INTEGER.toString().equals(getNodeFieldType()))

note the parentheses around || operands

@stevehu
Copy link
Contributor

stevehu commented Jul 16, 2019

@kosty You are right. The reason that we support the string format of integer type is due to the OpenAPI specification headers and query/path parameters. It is not part of the schema validation but since the library is tightly integrated into the light-4j framework, we need to give users an option to support the OpenAPI 3.0 specification for the data type validation. When we implementing that, we didn't even think about the maximum and minimum validators at all. Based on your suggestion, I think we need to implement both maximum and minimum and potentially other validators related to integers. Regarding the bug, I think one of them is not correct. @BalloonWen @jiachen1120, could you please take a look at the min/max validators? Thanks a lot for pointing out the issue.

@kosty
Copy link
Contributor Author

kosty commented Jul 17, 2019

Bigger concern, actually, is that jackson's TextNode happens to return 0 as value of node.bigIntegerValue() in place where number type is assumed. Thus causing an unintended behavior.

Oftentimes minimum/maximum are below/above 0 correspondingly which makes any quoted numeral pass the validation successfully.

@jiachen1120
Copy link
Contributor

jiachen1120 commented Jul 17, 2019

@kosty Thanks for point it out. I just debugged into it and found that textNode will be convert to 0 when using bigIntergerValue() or longValue(). This is a bug should be fixed. This would be the reason why serialized numbers cannot be validated with maximum/minimum properly.

@jiachen1120 jiachen1120 self-assigned this Jul 17, 2019
@kosty
Copy link
Contributor Author

kosty commented Jul 17, 2019

@jiachen1120 Thanks for a quick turnaround! I have done some work on this as well, and should have a PR for you to look at.

@jiachen1120
Copy link
Contributor

@kosty Thanks!!

stevehu added a commit that referenced this issue Jul 20, 2019
fixes #171 validation of quoted numerics added
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