Add missing autoreset in Packer.pack_ext_type#663
Add missing autoreset in Packer.pack_ext_type#663bysiber wants to merge 1 commit intomsgpack:mainfrom
Conversation
pack_ext_type writes to the internal buffer but never checks self._autoreset, unlike every other public pack method. This means it always returns None and the packed data leaks into the output of the next pack() call, corrupting the serialized stream. Add the same autoreset pattern used by pack(), pack_map_pairs(), pack_array_header(), and pack_map_header().
There was a problem hiding this comment.
Pull request overview
This PR addresses a long-standing behavior mismatch in Packer.pack_ext_type() where the pure-Python fallback implementation writes to the internal buffer but doesn’t honor self._autoreset, causing pack_ext_type() to return None and potentially corrupt subsequent packed output.
Changes:
- Add autoreset handling to
msgpack/fallback.py:Packer.pack_ext_type()so it returns bytes and clears the internal buffer whenself._autoresetis enabled.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self._autoreset: | ||
| ret = self._buffer.getvalue() | ||
| self._buffer = BytesIO() | ||
| return ret |
There was a problem hiding this comment.
Please add a regression test that asserts Packer().pack_ext_type(...) returns the packed bytes when autoreset is enabled, and that the internal buffer is cleared (e.g., a subsequent packer.pack(99) should not include the previously packed ext bytes). This would have caught the original bug and will prevent it from reappearing.
| if self._autoreset: | ||
| ret = self._buffer.getvalue() | ||
| self._buffer = BytesIO() | ||
| return ret |
There was a problem hiding this comment.
This adds autoreset handling for the pure-Python fallback Packer only. The default C-extension implementation (msgpack/_packer.pyx:pack_ext_type) still doesn’t return bytes or reset its internal buffer when autoreset is enabled, so msgpack.Packer().pack_ext_type(...) will remain broken in typical installs unless the Cython path is updated too.
Packer.pack_ext_type()writes to the internal buffer but never checksself._autoreset. Every other public pack method (pack,pack_map_pairs,pack_array_header,pack_map_header) has this pattern:Without it,
pack_ext_type()always returnsNone, and the packed ext data stays in the buffer. The next call topack()then returns both the extension data and the new value concatenated together, corrupting the serialized stream.