Skip to content

Raise JMESPathTypeError from ceil()/floor() on infinity and NaN - #394

Open
ApparentlyTejas wants to merge 1 commit into
jmespath:developfrom
ApparentlyTejas:fix/ceil-floor-nonfinite
Open

ApparentlyTejas wants to merge 1 commit into
jmespath:developfrom
ApparentlyTejas:fix/ceil-floor-nonfinite

Conversation

@ApparentlyTejas

Copy link
Copy Markdown

Fixes #381.

ceil() and floor() pass their argument straight to math.ceil/math.floor, which raise on infinity and NaN:

>>> import json, jmespath
>>> jmespath.search('floor(@)', json.loads('Infinity'))
OverflowError: cannot convert float infinity to integer
>>> jmespath.search('ceil(@)', json.loads('NaN'))
ValueError: cannot convert float NaN to integer

json.loads accepts Infinity/NaN by default, so data loaded the usual way can contain them. The @signature({"types": ['number']}) decorator doesn't catch this — inf/nan are still Python float, which maps to the JMESPath "number" type; the check only validates the type category, not finiteness.

Every other invalid-argument case in functions.py raises exceptions.JMESPathTypeError, so OverflowError/ValueError leaking out here means code that catches JMESPathError around search() doesn't catch these. Went with raising JMESPathTypeError (matching the existing convention) rather than returning the value unchanged, since that keeps the "callers catch JMESPathError" contract intact.

Both functions passed their argument straight to math.ceil/math.floor,
which raise the builtin OverflowError/ValueError on non-finite floats
instead of a JMESPathError subclass. json.loads accepts Infinity and
NaN by default, so data loaded the usual way can trigger this, and
code that catches JMESPathError around search() doesn't catch the
builtin exceptions. Every other invalid-argument case in this module
raises JMESPathTypeError; this makes ceil()/floor() consistent.
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

Successfully merging this pull request may close these issues.

floor() and ceil() raise OverflowError / ValueError on Infinity and NaN

1 participant