Skip to content

Appending an empty buffer corrupts subsequent positional reads #12

Description

@mcollina

Summary

Appending a zero-length buffer inserts an empty chunk that corrupts every subsequent positional read. length, slice() and subarray() stay correct, so the buffer looks fine until something reads it by offset.

Reproduced on @platformatic/dynamic-buffer@0.3.1, Node.js v24.18.0.

Reproduction

import { DynamicBuffer } from '@platformatic/dynamic-buffer'

const b = new DynamicBuffer()
b.append(Buffer.from([1, 2]))
b.append(Buffer.alloc(0))   // <-- empty chunk
b.append(Buffer.from([3, 4]))

b.length                    // 4          correct
b.slice(0).toString('hex')  // '01020304' correct

b.readInt8(2)               // 0          expected 3
b.readInt8(3)               // 4          correct
b.readInt16BE(1)            // 512        expected 515
b.readInt32BE(0)            // 16908288   expected 16909060

Without the Buffer.alloc(0) line, readInt8(0..3) returns 1,2,3,4 as expected.

The pattern of the failure — index 2 reads as 0, index 3 is fine — suggests the offset-to-chunk resolution counts the empty chunk as occupying a position, so one byte is skipped and reads as zero. Multi-byte reads spanning the boundary inherit the same error.

Impact

@platformatic/kafka serializes Kafka protocol frames through DynamicBuffer, and several protocol fields are legitimately empty without being null — empty (non-null) record keys and values, empty BYTES/COMPACT_BYTES fields. Writer.appendString, Writer.appendBytes and Writer.appendVarIntBytes all have to guard their appends with if (value.length > 0) to avoid this, which is easy to forget on the next call site added.

Nothing is corrupt on the wire today, because the write path drains via slice()/subarray() and crc32c() reads the flattened .buffer. The hazard is on anything that reads a DynamicBuffer positionally, which is what Reader does.

Suggested fix

Skip zero-length buffers in append()/prepend() (and appendFrom/prependFrom) rather than pushing an empty chunk, so callers do not each have to guard. Alternatively fix the offset-to-chunk resolution to tolerate empty chunks.

Either way it would be worth a regression test that interleaves empty and non-empty appends and then reads every offset back.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions