Skip to content

LZ4Reader silently corrupts decompressed data (plain-Array destination defeats lz4.js fill/copyWithin fast paths) #7

Description

@omarcoming

LZ4Reader decompresses each compressed chunk into this.currentOutput, which is a plain JS Array (LZ4Reader.js line 35, used at line 94):

this.currentOutput = new Array(0);                                  // line 35
...
lz4.decompressBlock(compressed, this.currentOutput, 0, compressed.length, 0);  // line 94

decompressBlock in src/lz4/lz4.js has two fast paths gated on dst.copyWithin !== undefined && dst.fill !== undefined (line 205). A plain Array has both methods, so the fast paths are taken — but Array.prototype.fill / copyWithin clamp to the array's current length instead of writing past it like a fixed-size Uint8Array would. When the destination index is at/beyond the array's grown length (the common case, since the array only grows via literal writes dst[dIndex++] = ...), the fill/copyWithin silently no-ops, dIndex advances anyway, and the output is left with undefined holes and mis-copied spans. The holes later coerce to 0 in finalOutput.set(...), so readMap() returns silently corrupted world data — no error, just wrong bytes.

On a real size-1500 v9 .map we measured ~967 corrupted bytes after a read/write round-trip before tracking it down to this.

Standalone repro

// npm i rustworld@1.0.3 && node repro.mjs
import * as lz4 from './node_modules/rustworld/src/lz4/lz4.js';

// A 128-byte pseudo-random block repeated at distance 512 forces long
// (mLength > 31) non-overlapping matches -> the copyWithin() fast path.
const N = 4096;
const original = new Uint8Array(N);
const block = new Uint8Array(128);
let seed = 1;
for (let i = 0; i < 128; i++) { seed = (seed * 16807) % 2147483647; block[i] = seed & 0xff; }
for (let i = 0; i < N; i++) {
  original[i] = (i % 512) < 128 ? block[i % 512] : (i * 7) & 0xff;
}

const out = new Uint8Array(N + 64);
const csize = lz4.compressBlock(original, out, 0, N, new Uint32Array(1 << 16));
const compressed = out.subarray(0, csize);

const asArray = [];                       // what LZ4Reader does
lz4.decompressBlock(compressed, asArray, 0, csize, 0);
const asTyped = new Uint8Array(N);        // correct destination
lz4.decompressBlock(compressed, asTyped, 0, csize, 0);

let holes = 0, wrong = 0;
for (let i = 0; i < N; i++) {
  if (asArray[i] === undefined) holes++;
  else if (asArray[i] !== original[i]) wrong++;
}
console.log('Array destination: holes', holes, '| wrong bytes', wrong);
console.log('Uint8Array destination byte-exact?', asTyped.every((b, i) => b === original[i]));

Output (node 24, rustworld 1.0.3):

Array destination: holes 126 | wrong bytes 877
Uint8Array destination byte-exact?  true

The other fast path (mOffset === 1dst.fill) has the same failure mode with run-length data.

Suggested fix

AquireNextChunk already allocates a correctly-sized Uint8Array for exactly this purpose at line 86–88 (this._buffer = new Uint8Array(new ArrayBuffer(originalLength))) but then never uses it in the compressed branch. Decompressing into that buffer instead of currentOutput fixes it:

lz4.decompressBlock(compressed, this._buffer, 0, compressed.length, 0);

with the chunk-stitching below (finalChunks.push) reading from this._buffer.slice(0, this._bufferLength) (a fresh copy per chunk, since _buffer is reused). Any fixed-size typed-array destination is immune because TypedArray.prototype.fill/copyWithin operate within the preallocated length rather than clamping a growable one.

This may also be the underlying cause of subtle downstream failures like corrupted-looking reads in #6, though that one throws earlier (DataView bounds), so I haven't linked them.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions