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

Unhandled OverflowError when checking multipleOf with large numbers #743

Closed
Zac-HD opened this issue Sep 30, 2020 · 1 comment · Fixed by #746
Closed

Unhandled OverflowError when checking multipleOf with large numbers #743

Zac-HD opened this issue Sep 30, 2020 · 1 comment · Fixed by #746
Labels
Bug Something doesn't work the way it should.

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Sep 30, 2020

>>> jsonschema.validate(1e308, {"type": "integer", "multipleOf": 0.5})

Traceback (most recent call last):
    ...
    File "jsonschema/_validators.py", line 170, in multipleOf
        failed = int(quotient) != quotient
OverflowError: cannot convert float infinity to integer

So the specific problem is that while 1e308 is finite, 1e308 / 0.5 overflows to infinity and then this raises an error when int(inf) is called. (it's a floating-point issue, but at least it's not "division doesn't work like real numbers"!)

I do consider this a bug because attempting to validate a well-formed instance with a well-formed schema should return either True or False, not raise an error. You could note that the spec allows implementations to limit the range of numbers they accept, but if so I'd still want a better error message and more specific exception type.

Of course, the question of which this should return is a somewhat ambiguous...

  • 0.5 is exactly representable as a float, 1e308 ==int(1e308), and obviously integers are all multiples of 0.5. My personal preference is therefore to call this instance valid and that's what master...Zac-HD:multipleOf-overflow would do.

  • The spec for multipleOf just says "A numeric instance is valid only if division by this keyword's value results in an integer"; overflowing to infinity is not an integer, and therefore the instance is invalid. That could easily be implemented with except OverflowError: failed = True.

In either case, we should probably also add an optional test to the upstream test suite...

@Julian
Copy link
Member

Julian commented Oct 2, 2020

Hi! Yeah that seems reasonable (the fallback logic) -- the spec yeah probably is going to allow either behavior given it allows IEEE floats to be used as numbers but maybe we should ensure everyone agrees with that.

But yeah we should start with a test upstream (there's already some in the bignum optional tests as you probably have seen).

@Julian Julian added the Bug Something doesn't work the way it should. label Oct 3, 2020
Julian added a commit that referenced this issue Oct 4, 2020
What can I say, we're naive...
Julian added a commit to willson-chen/jsonschema that referenced this issue Oct 15, 2020
* origin/master:
  Handle multipleOf overflow
  Don't fail when codecov.io flakes.
  pep517.build is dead or dying.
  Temporarily skip the failing tests for python-jsonschema#743.
  Sigh, ugly, but assrtRegexpMatches is deprecated.
  Squashed 'json/' changes from 21555a85..96742ba3
  Squashed 'json/' changes from fce9e9b3..21555a85
  Whoops, send coverage to codecov again.
  Be able to run coverage on all toxenvs.
  Remove outdated release notes.
  Use the longer form of the CLI option for clarity.
  Bump doc requirements (via pip-compile)
  Remove the Py2 coding declaration.
  Run pre-commit in CI.
  Remove docformatter, it's too unwieldy.
  Minor doc formatting.
  Sigh, this appears to be a regex, not exact match, so it was ignoring everything.
  Update pre-commit hooks.
@Zac-HD Zac-HD mentioned this issue Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something doesn't work the way it should.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants