Raise OverflowError when use_single_float cannot represent a value - #728
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.
|
The one red job ( Its step That stall reproduces on A re-run should come back green. |
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.
| File | Description |
|---|---|
msgpack/_packer.pyx |
Passes doubles to single-float packing. |
msgpack/pack_template.h |
Checks and propagates PyFloat_Pack4 errors. |
test/test_limits.py |
Covers FLT_MAX, overflow, and infinities. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.