| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
buf_size was untyped, so Cython converted it to size_t separately for the PyMem_Malloc call and for pk.buf_size. An object whose __int__ answers differently each call made the packer allocate one size and record another, and pack.h then grew the buffer against the recorded capacity, so a large enough payload was memcpy'd past the allocation. Typing the parameter converts it once during argument unpacking, the same way Unpacker takes read_size and max_buffer_size. Fixes msgpack#723
| Back | FazBrowse Home | New Git URL |
Fixes #723.
Packer.__cinit__ takes buf_size as an untyped object, and Cython converts it to size_t separately at each of the two lines that use it:
An object that answers differently each time therefore sizes the allocation from one answer and the recorded capacity from the other. Everything afterwards trusts the recorded capacity:
so a payload that fits the recorded capacity but not the real block is copied straight past the allocation. On the reporter's example the packer holds 600 bytes, records 1 MiB, and pack(b"A" * 100000) writes 100000 bytes into the 600-byte block.
The fix
Type the parameter, so the conversion happens once during argument unpacking:
This is how Unpacker already takes read_size and max_buffer_size (_unpacker.pyx:330), and it leaves no local for a later reader to fold back. It also removes a smaller wart: previously a second conversion that raised would leave __cinit__ failing with pk.buf already allocated.
Same converter (__Pyx_PyInt_As_size_t), so nothing user-visible moves. I checked the constructor's edge cases against a pre-fix build and they are identical, message for message: -1 gives OverflowError: can't convert negative value to size_t, 1 << 62 gives MemoryError, "x" and None give TypeError: an integer is required, and 0, 1.5, True and the default are accepted as before.
Tests
test_buf_size_is_converted_once asserts the object is queried exactly once. The overflow itself cannot be asserted portably (it takes the interpreter down rather than failing a test), so the test pins the invariant that prevents it. It fails on main with assert 2 == 1 and passes with the change.
The skipif follows the existing convention in test_unpack.py, since fallback.py accepts buf_size and ignores it. The helper's __int__ is the load-bearing slot on Cython 3.2.5, with __index__ aliased to it so the test keeps counting if a future Cython routes through PyNumber_Index instead.
Verified on CPython 3.14: full suite green in both the C-extension and MSGPACK_PUREPYTHON=1 configurations, and ruff check / ruff format clean. The reporter's reproducer no longer crashes.
Disclosure: I used an AI assistant while investigating and writing this patch. I reproduced the overflow myself, confirmed the pre-fix crash and the edge-case parity against a locally built extension, and I stand behind the change.