From 24798a75e4511dd6ad8e6843f1f75a38b58bbf99 Mon Sep 17 00:00:00 2001 From: John Behm Date: Thu, 20 Aug 2026 20:17:12 +0200 Subject: [PATCH] refactor: require disjoint decode buffers --- huffman.go | 31 ++---------------- regression_test.go | 78 ---------------------------------------------- 2 files changed, 3 insertions(+), 106 deletions(-) diff --git a/huffman.go b/huffman.go index 61d98d6..9107b05 100644 --- a/huffman.go +++ b/huffman.go @@ -3,7 +3,6 @@ package huffman import ( "encoding/binary" "fmt" - "unsafe" ) const ( @@ -85,9 +84,9 @@ func (huff *Huffman) Decompress(data []byte) ([]byte, error) { // returning the extended slice (like Go's append). Pass a reused buffer's // dst[:0] to avoid allocating a fresh output slice on every call — useful on // hot paths that decompress many packets (e.g. a 50Hz snapshot stream). dst may -// be nil and may overlap data; overlapping source bytes are preserved before -// output is appended. huff is not modified, so a single Huffman value is safe -// for concurrent DecompressTo calls with distinct dst buffers. +// be nil, but dst and data must not share backing storage; passing overlapping +// slices is invalid use. huff is not modified, so a single Huffman value is +// safe for concurrent DecompressTo calls with distinct dst buffers. func (huff *Huffman) DecompressTo(dst, data []byte) ([]byte, error) { if huff == nil || !huff.Dictionary.isInitialized() { return nil, fmt.Errorf("%w: dictionary is nil or uninitialized", ErrHuffmanDecompress) @@ -103,14 +102,6 @@ func (huff *Huffman) DecompressTo(dst, data []byte) ([]byte, error) { lut := &d.decLut nodes := &d.nodes - // Decompression can expand one input byte into several output bytes. If - // dst's writable capacity overlaps data, appending output could therefore - // overwrite compressed bytes before the decoder consumes them. Preserve - // append-style in-place use by copying only in this exceptional case. - if byteSlicesOverlap(dst[len(dst):cap(dst)], data) { - data = append([]byte(nil), data...) - } - // Output sizing. Two competing costs: guessing low means realloc+copy, // guessing high wastes memory and page faults. // @@ -358,22 +349,6 @@ func (huff *Huffman) Compress(data []byte) ([]byte, error) { return dst[:pos], nil } -// byteSlicesOverlap reports whether the two slice ranges share any byte. It -// compares addresses only; it never converts uintptr values back to pointers. -// Using subtraction instead of computing end addresses also avoids uintptr -// overflow on 32 bit platforms. -func byteSlicesOverlap(a, b []byte) bool { - if len(a) == 0 || len(b) == 0 { - return false - } - aStart := uintptr(unsafe.Pointer(unsafe.SliceData(a))) - bStart := uintptr(unsafe.Pointer(unsafe.SliceData(b))) - if aStart <= bStart { - return bStart-aStart < uintptr(len(a)) - } - return aStart-bStart < uintptr(len(b)) -} - // Buffer sizing arithmetic, kept in one place and parameterised by limit (the // platform's maxAlloc) so the 32 bit behaviour is unit-testable on any host. // All of it runs in uint64: on a 32 bit platform len(data)*8 would wrap and diff --git a/regression_test.go b/regression_test.go index 129d708..9839d91 100644 --- a/regression_test.go +++ b/regression_test.go @@ -384,84 +384,6 @@ func TestCopiedDictionaryRemainsUsable(t *testing.T) { } } -func TestDecompressToSupportsOverlappingInput(t *testing.T) { - huff := NewHuffman() - payloads := [][]byte{ - bytes.Repeat([]byte{0}, 64<<10), - bytes.Repeat([]byte{1}, 64<<10), - snapshotLike(62, 64<<10), - randomBytes(63, 64<<10), - } - tests := []struct { - name string - srcOffset int - dstOffset int - dstLen int - }{ - {"same-start", 0, 0, 0}, - {"output-before-input", 64, 0, 8}, - {"input-before-output", 0, 64, 0}, - } - - for payloadIndex, payload := range payloads { - compressed, err := huff.Compress(payload) - if err != nil { - t.Fatal(err) - } - for _, tt := range tests { - t.Run(itoa(payloadIndex)+"/"+tt.name, func(t *testing.T) { - storageLen := tt.srcOffset + len(compressed) - if needed := tt.dstOffset + tt.dstLen + len(payload); storageLen < needed { - storageLen = needed - } - storage := make([]byte, storageLen) - for i := 0; i < tt.dstLen; i++ { - storage[tt.dstOffset+i] = byte(0xa0 + i) - } - prefix := append([]byte(nil), storage[tt.dstOffset:tt.dstOffset+tt.dstLen]...) - copy(storage[tt.srcOffset:], compressed) - - dst := storage[tt.dstOffset : tt.dstOffset+tt.dstLen] - data := storage[tt.srcOffset : tt.srcOffset+len(compressed)] - got, err := huff.DecompressTo(dst, data) - if err != nil { - t.Fatal(err) - } - want := append(prefix, payload...) - if !bytes.Equal(got, want) { - t.Fatalf("got %d bytes, want %d", len(got), len(want)) - } - }) - } - } -} - -func TestByteSlicesOverlap(t *testing.T) { - buf := make([]byte, 32) - other := make([]byte, 32) - tests := []struct { - name string - a []byte - b []byte - want bool - }{ - {"same", buf, buf, true}, - {"contained", buf[4:20], buf[8:12], true}, - {"partial", buf[:16], buf[8:24], true}, - {"reverse-partial", buf[8:24], buf[:16], true}, - {"adjacent", buf[:16], buf[16:], false}, - {"separate", buf, other, false}, - {"empty", buf[:0], buf, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := byteSlicesOverlap(tt.a, tt.b); got != tt.want { - t.Fatalf("byteSlicesOverlap = %v, want %v", got, tt.want) - } - }) - } -} - // TestWriterMatchesCompress: the streaming Writer must emit exactly what the // one-shot Compress emits. func TestWriterMatchesCompress(t *testing.T) {