Skip to content

Add missing autoreset in Packer.pack_ext_type#663

Open
bysiber wants to merge 1 commit intomsgpack:mainfrom
bysiber:fix/pack-ext-type-autoreset
Open

Add missing autoreset in Packer.pack_ext_type#663
bysiber wants to merge 1 commit intomsgpack:mainfrom
bysiber:fix/pack-ext-type-autoreset

Conversation

@bysiber
Copy link
Copy Markdown

@bysiber bysiber commented Feb 20, 2026

Packer.pack_ext_type() writes to the internal buffer but never checks self._autoreset. Every other public pack method (pack, pack_map_pairs, pack_array_header, pack_map_header) has this pattern:

if self._autoreset:
    ret = self._buffer.getvalue()
    self._buffer = BytesIO()
    return ret

Without it, pack_ext_type() always returns None, and the packed ext data stays in the buffer. The next call to pack() then returns both the extension data and the new value concatenated together, corrupting the serialized stream.

packer = msgpack.Packer()
result = packer.pack_ext_type(5, b'\x01\x02\x03')
# result is None, expected bytes

packer.pack(99)
# Returns 7 bytes (ext header + ext data + int) instead of 1 byte

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().
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 when self._autoreset is enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread msgpack/fallback.py
Comment on lines +864 to +867
if self._autoreset:
ret = self._buffer.getvalue()
self._buffer = BytesIO()
return ret
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Comment thread msgpack/fallback.py
Comment on lines +864 to +867
if self._autoreset:
ret = self._buffer.getvalue()
self._buffer = BytesIO()
return ret
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants