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.

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions