{{ message }}
Raise OverflowError when use_single_float cannot represent a value - #728
Merged
Conversation
The C extension narrowed the double to a C float before calling
PyFloat_Pack4, so values above FLT_MAX were silently packed as
infinity. PyFloat_Pack4's return value was also discarded, so the
overflow it detects could not surface. The pure Python packer uses
struct.pack('>f') and has always raised OverflowError here.
Pass the double through and check the return value, matching the
fallback and the existing out-of-range integer behaviour.
Author
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns C-extension single-float packing with the Python fallback by raising OverflowError for unrepresentable finite values.
Changes:
- Preserves double precision until
PyFloat_Pack4. - Propagates packing overflow errors.
- Tests float boundaries and infinities.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fuzzing the Cython packer against the pure Python fallback over 4000 random objects,
packbdisagreed on 38 of them. Every disagreement was the same case:use_single_float=Truewith a value above FLT_MAX.Two things kept the C path quiet.
_packer.pyxcast the value with<float>obefore the call, somsgpack_pack_floatonly ever saw a narrowed float. Andmsgpack_pack_floatdiscarded the return value ofPyFloat_Pack4, which is what reports the overflow.Passing the double through and checking that return value covers both.
PyFloat_Pack4backsstruct.pack('>f', ...), so the exception type and message now match the fallback exactly.Out-of-range integers are handled this way today:
packb(2**64)raisesPackOverflowErrorfrom either packer, pinned bytest_limits.py::test_integer. The new test sits next to it.Reverting the source makes the new test fail on the C extension. It passes either way under
MSGPACK_PUREPYTHON=1, where it pins existing behaviour. After the change the same fuzz run reports 0 of 4000 disagreements onpackb.Infinity is representable in single precision and packs unchanged, which the test also covers.