Skip to content

fix: IOWriter write_binary encodes small payloads as fixstr instead of bin8#29

Merged
nuskey8 merged 2 commits intonuskey8:mainfrom
paq:cross-path-test
Apr 8, 2026
Merged

fix: IOWriter write_binary encodes small payloads as fixstr instead of bin8#29
nuskey8 merged 2 commits intonuskey8:mainfrom
paq:cross-path-test

Conversation

@paq
Copy link
Copy Markdown
Contributor

@paq paq commented Apr 8, 2026

Issue

The IOWriter's write_binary method had a 0..=31 arm that used fixstr format (0xb0 | len) instead of the bin8 marker. This caused small binary payloads (<=31 bytes) to be encoded as strings rather than binary data, making the output inconsistent with SliceWriter and VecWriter.

IOWriter

zerompk/zerompk/src/write.rs

Lines 1665 to 1698 in c36e38f

#[inline(always)]
fn write_binary(&mut self, data: &[u8]) -> Result<()> {
let len = data.len();
match len {
0..=31 => {
self.write_all(&[0xb0 | (len as u8)])?;
self.write_all(data)?;
Ok(())
}
32..=255 => {
self.write_all(&[BIN8_MARKER, len as u8])?;
self.write_all(data)?;
Ok(())
}
256..=65535 => {
let len_bytes = (len as u16).to_be_bytes();
self.write_all(&[BIN16_MARKER, len_bytes[0], len_bytes[1]])?;
self.write_all(data)?;
Ok(())
}
_ => {
let len_bytes = (len as u32).to_be_bytes();
self.write_all(&[
BIN32_MARKER,
len_bytes[0],
len_bytes[1],
len_bytes[2],
len_bytes[3],
])?;
self.write_all(data)?;
Ok(())
}
}
}

SliceWriter

fn write_binary(&mut self, data: &[u8]) -> Result<()> {
let len = data.len();
match len {
// Bin8
0..=255 => {
let buf = self.take_slice(2 + len)?;
unsafe {
let ptr = buf.as_mut_ptr();
*ptr = BIN8_MARKER;
*ptr.add(1) = len as u8;
core::ptr::copy_nonoverlapping(data.as_ptr(), ptr.add(2), len);
}
Ok(())
}
// Bin16
256..=65535 => {
let buf = self.take_slice(3 + len)?;
unsafe {
let ptr = buf.as_mut_ptr();
*ptr = BIN16_MARKER;
core::ptr::copy_nonoverlapping(
(len as u16).to_be_bytes().as_ptr(),
ptr.add(1),
2,
);
core::ptr::copy_nonoverlapping(data.as_ptr(), ptr.add(3), len);
}
Ok(())
}
// Bin32
_ => {
let buf = self.take_slice(5 + len)?;
unsafe {
let ptr = buf.as_mut_ptr();
*ptr = BIN32_MARKER;
core::ptr::copy_nonoverlapping(
(len as u32).to_be_bytes().as_ptr(),
ptr.add(1),
4,
);
core::ptr::copy_nonoverlapping(data.as_ptr(), ptr.add(5), len);
}
Ok(())
}
}
}

VecWriter

zerompk/zerompk/src/write.rs

Lines 1085 to 1124 in c36e38f

fn write_binary(&mut self, data: &[u8]) -> Result<()> {
let len = data.len();
match len {
0..=255 => {
self.buffer.reserve(2 + len);
unsafe {
let ptr = self.buffer.as_mut_ptr().add(self.buffer.len());
*ptr = BIN8_MARKER;
*ptr.add(1) = len as u8;
ptr.add(2).copy_from_nonoverlapping(data.as_ptr(), len);
self.buffer.set_len(self.buffer.len() + 2 + len);
}
Ok(())
}
256..=65535 => {
self.buffer.reserve(3 + len);
unsafe {
let ptr = self.buffer.as_mut_ptr().add(self.buffer.len());
*ptr = BIN16_MARKER;
let len_bytes = (len as u16).to_be_bytes();
ptr.add(1).copy_from_nonoverlapping(len_bytes.as_ptr(), 2);
ptr.add(3).copy_from_nonoverlapping(data.as_ptr(), len);
self.buffer.set_len(self.buffer.len() + 3 + len);
}
Ok(())
}
_ => {
self.buffer.reserve(5 + len);
unsafe {
let ptr = self.buffer.as_mut_ptr().add(self.buffer.len());
*ptr = BIN32_MARKER;
let len_bytes = (len as u32).to_be_bytes();
ptr.add(1).copy_from_nonoverlapping(len_bytes.as_ptr(), 4);
ptr.add(5).copy_from_nonoverlapping(data.as_ptr(), len);
self.buffer.set_len(self.buffer.len() + 5 + len);
}
Ok(())
}
}
}

Fix

Merged the 0..=31 and 32..=255 arms into a single 0..=255 arm using BIN8_MARKER, matching SliceWriter and VecWriter.

Tests

  • Add cross path test (cross_path_test.rs) to verify identical output across all three writer paths (SliceWriter, VecWriter, IOWriter) for every Write trait method.
    • cross_path_binary_empty and cross_path_binary_bin8 caught this bug.

Copilot AI review requested due to automatic review settings April 8, 2026 08:03
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 fixes MessagePack binary encoding in the IOWriter implementation so that small binary payloads are encoded using the proper bin8 format (instead of an incorrect fixstr marker), aligning IOWriter output with the existing SliceWriter and VecWriter behavior. It also adds a cross-path integration test to ensure all Write methods produce identical bytes across writer backends.

Changes:

  • Fix IOWriter::write_binary to encode 0..=255 byte payloads with BIN8_MARKER (bin8) rather than an incorrect fixstr marker.
  • Add cross_path_test.rs to compare serialized output across VecWriter, SliceWriter, and (when enabled) IOWriter for all Write trait methods.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
zerompk/src/write.rs Corrects IOWriter binary encoding for small payloads to use bin8, matching other writer implementations.
zerompk/tests/cross_path_test.rs Adds cross-writer parity tests that would catch writer-path inconsistencies like the original binary encoding bug.

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

@nuskey8 nuskey8 merged commit 2dc9140 into nuskey8:main Apr 8, 2026
4 of 6 checks passed
@nuskey8
Copy link
Copy Markdown
Owner

nuskey8 commented Apr 8, 2026

Merged the PR. Thanks!

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