Fix multipleOf crashing on Decimal instances and large integers#1515
Open
vineethsaivs wants to merge 2 commits into
Open
Fix multipleOf crashing on Decimal instances and large integers#1515vineethsaivs wants to merge 2 commits into
vineethsaivs wants to merge 2 commits into
Conversation
When multipleOf has a float divisor, the validator divides the instance by it. The division sat outside the try that guarded the subsequent int() cast, and the except only caught OverflowError. As a result: - a Decimal instance (which jsonschema supports, e.g. from json.loads(text, parse_float=Decimal)) raised TypeError, because Decimal / float is unsupported; and - a very large integer instance raised OverflowError at the division itself (int too large to convert to float), which the Fraction fallback was meant to cover but never reached. Move the division inside the try and also catch TypeError, so both fall back to the exact Fraction computation. 10.0 (as Decimal) is correctly reported a multiple of 2.5, and every integer a multiple of 0.5.
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
multipleOfwith a float divisor raises an uncaught exception (instead of validating) for two reasonable inputs:Both should validate successfully:
10.0is a multiple of2.5, and every integer is a multiple of0.5.Decimalis a supported instance type (it is anumbers.Number, and there is an existingtest_it_can_validate_with_decimals), andDecimalinstances arise naturally fromjson.loads(text, parse_float=Decimal).Root cause
In the
floatbranch ofmultipleOf(jsonschema/_keywords.py), the division was placed outside thetry, and theexceptonly caughtOverflowError:So the division could raise uncaught:
TypeErrorforDecimal / float, andOverflowErrorwhen a huge integer is divided by a float (the division itself overflows, before theint()cast the guard was written for).Fix
Move the division inside the
tryand also catchTypeError, so both cases fall back to the exact, overflow-proofFractioncomputation that was already there:The
else(instance % dB) path and the fast float path are unchanged.Tests
Added
TestMultipleOf.test_it_does_not_crash_on_decimals_or_large_integerstojsonschema/tests/test_validators.py. It fails before the change (TypeErrorat the division) and passes after; it also checks theFractionfallback still rejects genuine non-multiples (Decimal("10")vs3.3,10**400vs0.3). The fulltest_validators.py(304) and the official Test SuitemultipleOfcases (55) pass. Happy to add a CHANGELOG entry if you'd like.