From 0539aafe3723ceeb4bda8c79325addfb948e2585 Mon Sep 17 00:00:00 2001 From: Rowan Date: Tue, 8 Sep 2026 12:01:30 -0400 Subject: [PATCH 1/2] BitReader reads a tight packet-sized buffer without an eight-byte slack readBits always loaded an 8-byte little-endian window at the byte cursor, and the only guard on the slack past the data was an assert, so a correct packet in an exactly-sized byte[] threw ArrayIndexOutOfBoundsException on its last fields in a release JVM. ReadStream promises hostile bytes never throw, and its constructor forwards straight to reset. reset now copies the final min(bytes, 8) bytes into a zero-padded 16-byte tail whenever the array has fewer than 8 bytes of slack, and records the byte index the tail is based at; otherwise tailBase is Integer.MAX_VALUE. readBits selects the window source with one well-predicted branch and loads a single window either way: no allocation per read, and an array with slack never takes the tail. The padding is never interpreted, since bits past the end of the data cannot reach the output of a read, so the outputs match slack bytes exactly. This is the shape serialize.go uses. Two stream tests cover the reported shape, new ReadStream(packet, packet.length): a one-byte packet reading its eight bits, and an eleven-byte packet read to its last bit with windows starting inside the last eight bytes and on the last byte, plus the band with one to seven bytes of slack. Both throw on the unfixed reader. Fixes #9 Co-Authored-By: Claude Fable 5.1 --- src/serialize/BitReader.java | 68 ++++++++++++++++++++------- test/serialize/tests/StreamTests.java | 57 ++++++++++++++++++++++ 2 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/serialize/BitReader.java b/src/serialize/BitReader.java index 2ceb337..9275a19 100644 --- a/src/serialize/BitReader.java +++ b/src/serialize/BitReader.java @@ -1,29 +1,43 @@ package serialize; +import java.util.Arrays; + /** * Reads bitpacked integer values from a buffer, reconstructing the exact * sequence of bit reads that wrote it. * - * Branchless: each read loads a 64-bit little-endian window from the current - * byte position and shifts by the bit remainder. There is no scratch state - * and no refill branch. + * Effectively branchless: each read loads a 64-bit little-endian window from + * the current byte position and shifts by the bit remainder. There is no + * scratch state and no refill branch. * - * IMPORTANT — the allocation contract, matching the C++ reference: the byte - * array must extend at least 8 bytes past the end of the data being read, - * because the reader loads 8-byte windows at byte granularity. The bytes past - * the end are loaded but never interpreted: they can never influence a - * decoded value or an accept/reject decision. + * Any buffer size is supported. For the fastest reads, keep at least 8 bytes + * of slack in the array past the data — for example, read packets into a + * large buffer and read them out of it in place — and every window load comes + * straight from the array. Without slack, {@link #reset} copies the final + * bytes of the data into a small zero-padded tail window, and reads near the + * end of the buffer load from that instead. Either way the load is a single + * window: slack (or padding) bytes are loaded but never interpreted, because + * bits past the end of the data cannot reach the output of a read, so they + * can never influence a decoded value or an accept/reject decision. */ public final class BitReader { + // the reach of a window load: 8 data bytes hold any [1,32] field at any + // bit offset. The widest window a legal read can ask for starts at the + // last byte of the data, which is why the tail is two windows long. + private static final int WINDOW_BYTES = 8; + private byte[] data; private long numBits; private long bitsRead; + private int tailBase; // byte index the tail window is based at + private final byte[] tail = new byte[WINDOW_BYTES * 2]; // zero-padded copy of the final data bytes (arrays with less than WINDOW_BYTES of slack) /** * Creates a bit reader over the given buffer. - * @param data the bitpacked data to read. The array must extend at least - * 8 bytes past {@code bytes} — see the class comment. + * @param data the bitpacked data to read. Any array size works; at least + * 8 bytes of slack past {@code bytes} is fastest — see the class + * comment. * @param bytes the number of bytes of bitpacked data to read. */ public BitReader( byte[] data, int bytes ) @@ -33,9 +47,13 @@ public BitReader( byte[] data, int bytes ) /** * Rewinds the reader over the given buffer, the allocation-free reuse - * surface: same contract as the constructor. - * @param data the bitpacked data to read. The array must extend at least - * 8 bytes past {@code bytes} — see the class comment. + * surface: same contract as the constructor. When the array has less than + * 8 bytes of slack past the data, the final bytes are copied into the + * zero-padded tail window so that every read has a whole window of + * readable bytes beneath it. + * @param data the bitpacked data to read. Any array size works; at least + * 8 bytes of slack past {@code bytes} is fastest — see the class + * comment. * @param bytes the number of bytes of bitpacked data to read. */ public void reset( byte[] data, int bytes ) @@ -44,13 +62,28 @@ public void reset( byte[] data, int bytes ) this.data = data; this.numBits = (long) bytes * 8; this.bitsRead = 0; + if ( data.length - bytes < WINDOW_BYTES ) + { + // no slack past the data: the final bytes live in the zero-padded + // tail window, and reads at or past tailBase load from it. The + // padding is never interpreted, so zeros here produce exactly the + // same outputs as slack bytes would. + int copied = Math.min( bytes, WINDOW_BYTES ); + this.tailBase = bytes - copied; + System.arraycopy( data, tailBase, tail, 0, copied ); + Arrays.fill( tail, copied, tail.length, (byte) 0 ); + } + else + { + this.tailBase = Integer.MAX_VALUE; // every window load comes straight from the array + } } private static boolean checkReset( byte[] data, int bytes ) { assert data != null; assert bytes >= 0; - assert data.length - bytes >= 8; // the allocation contract: 8 bytes of slack past the data + assert bytes <= data.length; return true; } @@ -72,8 +105,11 @@ public int readBits( int bits ) // when -ea is absent, and it counts against inlining thresholds assert checkReadBits( bits ); - // loads up to 7 bytes past the last data byte: the allocation contract covers this - long window = (long) BitWriter.LONG_LE.get( data, (int) ( bitsRead >> 3 ) ); + // loads up to 7 bytes past the last data byte: from the array when it + // has the slack, otherwise from the zero-padded tail window. One + // well-predicted branch — an array with slack never takes the tail. + int i = (int) ( bitsRead >> 3 ); + long window = i < tailBase ? (long) BitWriter.LONG_LE.get( data, i ) : (long) BitWriter.LONG_LE.get( tail, i - tailBase ); int output = (int) ( ( window >>> ( (int) ( bitsRead & 7 ) ) ) & ( ( 1L << bits ) - 1 ) ); diff --git a/test/serialize/tests/StreamTests.java b/test/serialize/tests/StreamTests.java index bdc2106..637b414 100644 --- a/test/serialize/tests/StreamTests.java +++ b/test/serialize/tests/StreamTests.java @@ -45,6 +45,63 @@ static void run() check( !reader.serializeBits( value, 8 ), "9th bit refused" ); } ); + // The common shape: getBytesProcessed() bytes are copied out of the + // write buffer into a packet-sized array and read back from that, with + // no slack past the data. Every window load near the end of such a + // buffer would run past the array; the reader takes them from its + // zero-padded tail window instead. Hostile bytes never throw, and + // neither may a correct packet in a tight array. + test( "tight buffer: a one-byte packet with no slack reads its eight bits", () -> { + byte[] packet = { (byte) 0xA5 }; + ReadStream reader = new ReadStream( packet, packet.length ); + IntRef value = new IntRef(); + check( reader.serializeBits( value, 8 ) ); + checkEqual( value.value, 0xA5, "the byte" ); + check( !reader.serializeBits( value, 1 ), "9th bit refused" ); + } ); + + test( "tight buffer: an exactly-sized packet reads to its last bit", () -> { + byte[] buffer = new byte[24]; + WriteStream writer = new WriteStream( buffer, 16 ); + check( writer.serializeBits( new IntRef( 5 ), 3 ) ); + check( writer.serializeBits( new IntRef( 0xDEADBEEF ), 32 ) ); + check( writer.serializeBits( new IntRef( 0x1ABCD ), 17 ) ); + check( writer.serializeBits( new IntRef( 0xCAFEF00D ), 32 ) ); + check( writer.serializeBits( new IntRef( 0xB ), 4 ) ); + checkEqual( writer.getBitsProcessed(), 88, "the fields fill eleven bytes exactly" ); + writer.flush(); + + // the packet, with nothing past it: the last field lands on the last bit + byte[] packet = new byte[(int) writer.getBytesProcessed()]; + System.arraycopy( buffer, 0, packet, 0, packet.length ); + ReadStream reader = new ReadStream( packet, packet.length ); + IntRef value = new IntRef(); + check( reader.serializeBits( value, 3 ) ); + checkEqual( value.value, 5, "3 bits" ); + check( reader.serializeBits( value, 32 ) ); + checkEqual( value.value, 0xDEADBEEF, "32 bits" ); + check( reader.serializeBits( value, 17 ) ); + checkEqual( value.value, 0x1ABCD, "17 bits" ); + check( reader.serializeBits( value, 32 ) ); + checkEqual( value.value, 0xCAFEF00D, "32 bits, a window starting inside the last eight bytes" ); + check( reader.serializeBits( value, 4 ) ); + checkEqual( value.value, 0xB, "4 bits, a window starting on the last byte" ); + checkEqual( reader.getBitsProcessed(), 88, "every bit was read" ); + check( !reader.serializeBits( value, 1 ), "the bit past the end refused" ); + + // and the band between: some slack, but less than a window of it + byte[] partial = new byte[packet.length + 3]; + System.arraycopy( packet, 0, partial, 0, packet.length ); + reader.reset( partial, packet.length ); + check( reader.serializeBits( value, 3 ) ); + check( reader.serializeBits( value, 32 ) ); + check( reader.serializeBits( value, 17 ) ); + check( reader.serializeBits( value, 32 ) ); + checkEqual( value.value, 0xCAFEF00D, "32 bits with three bytes of slack" ); + check( reader.serializeBits( value, 4 ) ); + checkEqual( value.value, 0xB, "the last field with three bytes of slack" ); + } ); + test( "serializeBits64: low 32-bit group first, then the remainder", () -> { byte[] buffer = new byte[24]; WriteStream writer = new WriteStream( buffer, 16 ); From 68bda8581035699fdd72b82f88d536f13dd2ed8d Mon Sep 17 00:00:00 2001 From: Rowan Date: Tue, 8 Sep 2026 12:08:20 -0400 Subject: [PATCH 2/2] Say on the public surface that any array size works ReadStream's Javadoc still required eight bytes of slack, and BitReader's header still said it had no scratch state; both now say what the code does. Co-Authored-By: Claude Fable 5.1 --- src/serialize/BitReader.java | 3 ++- src/serialize/ReadStream.java | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/serialize/BitReader.java b/src/serialize/BitReader.java index 9275a19..e645908 100644 --- a/src/serialize/BitReader.java +++ b/src/serialize/BitReader.java @@ -8,7 +8,8 @@ * * Effectively branchless: each read loads a 64-bit little-endian window from * the current byte position and shifts by the bit remainder. There is no - * scratch state and no refill branch. + * refill branch; the only scratch state is the sixteen-byte tail a tight + * buffer is read through. * * Any buffer size is supported. For the fastest reads, keep at least 8 bytes * of slack in the array past the data — for example, read packets into a diff --git a/src/serialize/ReadStream.java b/src/serialize/ReadStream.java index e77c169..51f39ca 100644 --- a/src/serialize/ReadStream.java +++ b/src/serialize/ReadStream.java @@ -31,9 +31,8 @@ public final class ReadStream implements BitStream private boolean failed; /** - * @param buffer the buffer to read from. The array must extend at least - * 8 bytes past {@code bytes}: the bit reader loads 64-bit windows - * at byte granularity. See {@link BitReader}. + * @param buffer the buffer to read from. Any array size works; at least + * 8 bytes of slack past {@code bytes} is fastest. See {@link BitReader}. * @param bytes the number of bytes of packet data to read. */ public ReadStream( byte[] buffer, int bytes ) @@ -44,8 +43,8 @@ public ReadStream( byte[] buffer, int bytes ) /** * Rewinds the stream over the given buffer, the allocation-free reuse * surface: same contract as the constructor. - * @param buffer the buffer to read from. The array must extend at least - * 8 bytes past {@code bytes} — see {@link BitReader}. + * @param buffer the buffer to read from. Any array size works; at least + * 8 bytes of slack past {@code bytes} is fastest. See {@link BitReader}. * @param bytes the number of bytes of packet data to read. */ public void reset( byte[] buffer, int bytes )