Skip to content

Fix inverted offset check in encode_into - #1310

Open
cig13zs wants to merge 1 commit into
msgspec:mainfrom
cig13zs:fix-encode-into-offset
Open

cig13zs wants to merge 1 commit into
msgspec:mainfrom
cig13zs:fix-encode-into-offset

Conversation

@cig13zs

@cig13zs cig13zs commented Oct 24, 2026

Copy link
Copy Markdown

Encoder.encode_into(msg, buf, offset) resizes buf down to max(8, 1.5 * offset) whenever offset < len(buf), so passing an offset throws away the capacity of a preallocated buffer. Leaving the offset out keeps it:

import msgspec

enc = msgspec.msgpack.Encoder()
buf = bytearray(1000)                      # __sizeof__() == 1057
enc.encode_into({"key": "value"}, buf, 4)
buf.__sizeof__()                           # 79 (still 1057 without the offset)

The length-prefix example in the perf tips uses this pattern (bytearray(64) then encode_into(msg, buffer, 4)), and the docstring says the underlying buffer won't be truncated.

When offset > len(buf) nothing is resized up front. The buffer is grown later by ms_resize_bytearray, and the bytes between the old end and offset are left uninitialized (encode_into(1, bytearray(b"abc"), 64) leaves non-zero leftover bytes in buf[3:64]).

#753 meant to grow the buffer when the offset is past the end, but the check reads offset < buf_size. This flips it to > and zero-fills the gap, so in-bounds offsets leave the buffer alone.

Tests: in test_encode_into_offset (json and msgpack) the out-of-bounds case now checks the gap is zeros, and a new case checks that an in-bounds offset keeps the capacity. Both fail on main. pytest tests/unit passes on Python 3.13 (6582 passed, 158 skipped), and ruff check, ruff format --diff and codespell are clean.

#1160 changes the same function but keeps this block as it is, so whichever goes in second gets a small conflict.

This branch has not been deployed

No deployments
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.

1 participant