Repository navigation
fix: raise ValidationError for out-of-range int→float in convert - #1128
takerukondo wants to merge 2 commits into
Conversation
PyLong_AsDouble sets OverflowError for ints that cannot fit in a finite C double. convert was returning a float while that exception stayed set, which leaked SystemError. Match json.decode and report ValidationError. Fixes msgspec#1122 Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
| /* PyLong_AsDouble sets OverflowError and may return ±inf for ints that | ||
| * cannot be represented as a finite C double. Surface that as a |
There was a problem hiding this comment.
Where are you getting the
return ±inf for ints that cannot be represented as a finite C double
part from? It's at least not stated in the documentation: https://docs.python.org/3/c-api/long.html#c.PyLong_AsDouble
|
Thanks for this, and sorry for the long silence. We went with #1162, so I am closing this one. You were here first, so you are owed the actual reasons rather than a bare "duplicate". Two of them are on us, not on you:
Answering that question now, since it is the substantive one: he was right, and the code comment was wrong. The On the changelog entry, to be clear: writing one was not wrong, several merged PRs here carry their own. Only the placement broke, and recently. Since you opened this, Your diagnosis was right and your test case was a good one. Thank you for the time, and please do send more. |
Summary
Fixes #1122.
msgspec.convert(10**400, float)was leakingSystemErrorbecausePyLong_AsDoublesetsOverflowErrorwhileconvertstill returned a float object.json.decodealready reports this asValidationError: Number out of range.Clear the overflow error and raise
ValidationError(same message as the JSON path). Also reject non-finite results as a belt-and-suspenders check.Test plan
pytest tests/unit/test_convert.py::TestFloat::test_float_from_out_of_range_int tests/unit/test_convert.py::TestFloat::test_floatMade with Cursor