Skip to content

Commit 6e2bc09

Browse files
authored
feat(wasm): wire memlimit option through unxzAsync/unxz (#111)
* feat(wasm): wire memlimit option through unxzAsync/unxz LZMAOptions previously had no memlimit field, so WASM decoders silently ignored any caller-provided limit (TODO at src/wasm/decompress.ts:26). - Add `memlimit?: number | bigint` to LZMAOptions with full TSDoc - Thread through unxzAsync + callback variant to streamBufferDecode - Adjust XzStream._opts type to keep memlimit optional in the stream class (Required<LZMAOptions> would break otherwise) - 8 new tests in test/wasm/decompress-memlimit.test.ts covering both number and bigint inputs, sync/callback variants, and the default Native parity: the N-API binding (InitializeDecoder) still hardcodes UINT64_MAX and ignores opts.memlimit. Tracked as a separate MEDIUM TODO; out of scope here to keep the change tight. * fix(wasm): address review findings on memlimit option (PR #111) Applies opus + Copilot review findings (4M + 4S): - Validate `number` memlimit before BigInt() coercion. Reject NaN, Infinity, fractional, and negative values with LZMAOptionsError instead of letting RangeError leak from BigInt() (F-001). - Replace `Omit<Required<LZMAOptions>, 'memlimit'> & Pick<...>` smell on XzStream._opts with an internal ResolvedLZMAOptions interface that lists the genuinely-optional memlimit explicitly (F-002). - Rewrite callback-variant memlimit tests as async so makeFixture() rejections propagate to vitest instead of hanging the Promise (C-001 / C-002). - Assert byte-equality vs original buffer in callback success test (was only `toBeDefined()`) (C-003). - Rework memlimit TSDoc: WASM/native asymmetry warning moved to top, Default split into "WASM default: 256 MiB" and "Native: ignored" (F-003 / C-005). - Refresh stale "Required<LZMAOptions>" comment in lzma.ts to match the new ResolvedLZMAOptions type (F-004 / C-004). - Fix fixture comment "~4 MB" -> "~8 MiB" (preset 6 dict size) and explain why the AC holds regardless of exact size (F-005). - Document the default-path test coverage gap in a 3-line code comment — proves no-throw, not that default IS specifically 256 MiB (F-006). 584/584 tests pass (12 memlimit, +4 vs round 1). * fix(wasm): address Copilot round-2 findings on memlimit (PR #111) 5 findings from Copilot review of round-1 fixes (3M + 1S + 1L): - Remove `xzAsync` from `LZMAOptions.memlimit` honored-by list and refresh internal lzma.ts comment — xzAsync is compression-only and doesn't read memlimit (C-2-001 / C-2-002). - Lift `validateMemlimit` to run as the first statement of every decoder ingestion path: `decoderInit`, `autoDecoderInit`, and `streamBufferDecode` — round 1 only guarded `streamBufferDecode`, leaving the streaming decoder entry points able to throw raw RangeError on NaN/Infinity/fractional or accept negative values (C-2-005). +12 tests prove rejection on each path. - Reject `number > Number.MAX_SAFE_INTEGER` in `validateMemlimit` — values above 2^53-1 lose precision when coerced to bigint and produce an unintended limit. TSDoc directs callers to use bigint for large limits (C-2-004). +4 tests cover both sides of the boundary. - Replace hardcoded `errno: 8` with `LZMA_OPTIONS_ERROR` constant imported from `src/errors.ts` (C-2-003). Pre-push opus senior review (§2.10 round-3 gate): SAFE-TO-PUSH — all five claims verified against working tree, zero new S/M/L. 600/600 tests pass (16 added vs round 1). Polish item deferred to TODO.md (LOW): bigint branch lacks UINT64_MAX upper-bound guard; benign while native ignores memlimit; becomes load-bearing once native parity is wired. * docs(wasm): tighten memlimit comments and dedupe JSDoc (PR #111) Round-3 Copilot doc-only findings (3L + 1S): - Remove three duplicate JSDoc blocks from `decoderInit`, `autoDecoderInit`, and `validateMemlimit`. Earlier rounds used `insert_symbol` instead of `write_symbol`, leaving the original short doc next to the new validation-aware block. Keep the complete block in each case (C-3-001/2/3). - Fix a missed-spot stale comment in `src/lzma.ts:370` still listing `xzAsync` as honouring memlimit. `xzAsync` is compression-only; the round-2 cleanup at line 311 missed this second occurrence (C-3-004). Sweep confirms no other "xzAsync honours memlimit" claims remain across `src/lzma.ts`, `src/wasm/bindings.ts`, and `src/types.ts`. 600+ tests pass, tsc clean. No behaviour change.
1 parent f8f21d0 commit 6e2bc09

8 files changed

Lines changed: 371 additions & 9 deletions

File tree

.changeset/wasm-memlimit-option.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'node-liblzma': minor
3+
---
4+
5+
Add `memlimit` option to `LZMAOptions` and wire it through `unxzAsync`/`unxz` (WASM).
6+
7+
Callers can now set a memory usage limit for WASM decompression:
8+
9+
```ts
10+
await unxzAsync(buf, { memlimit: 64 * 1024 * 1024 }); // 64 MiB limit
11+
```
12+
13+
When the compressed stream would require more memory than the limit, the promise rejects with `LZMAMemoryLimitError` (`errno === LZMA_MEMLIMIT_ERROR === 6`).
14+
15+
**Accepted types:** `number | bigint` (both coerced to `bigint` for the WASM C ABI).
16+
**Default:** `BigInt(256 * 1024 * 1024)` (256 MiB — unchanged from existing behaviour).
17+
18+
**Native parity:** The native Node.js binding (`InitializeDecoder`) still hardcodes `UINT64_MAX` and ignores `memlimit`. This is WASM-only for now; native tracking in TODO.md.

TODO.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,20 @@ _None_
1010

1111
## Pending - MEDIUM
1212

13-
_None_
13+
- [ ] [tar-xz] True streaming for Node `extract()`/`list()` — replace `Buffer.concat` accumulation (extract.ts:59,91 + list.ts:26) with incremental header→content parsing so memory stays O(largest entry) instead of O(archive). Public README v6.0.0 advertises this as a "planned optimization" — Priority: M
14+
- [ ] [Native] Wire `memlimit` in `src/bindings/node-liblzma.cpp` `InitializeDecoder` — currently hardcodes `UINT64_MAX`; should read `opts.memlimit` and call `lzma_stream_decoder(stream, memlimit, flags)`. WASM already supports it. — Priority: M
15+
- [] [WASM] Wire `memlimit` through `LZMAOptions` and `unxzAsync` — moved to In Progress (2026-04-28) → ✅ completed (2026-04-28)
1416

1517
## Pending - LOW (Nice to Have)
1618

17-
_None_
19+
- [ ] [WASM] `validateMemlimit` symmetry — bigint branch has no UINT64_MAX upper-bound guard (only the `number` branch checks `MAX_SAFE_INTEGER`). Currently benign because native side hardcodes UINT64_MAX and `lzma_stream_decoder` reads `uint64_t` (so `2n ** 65n` would silently wrap to a benign-but-unintended ceiling, no security/leak/crash). Becomes load-bearing once `[Native] Wire memlimit` lands — fix together. Priority: L (defer until native parity work).
1820

1921
## Completed
2022

23+
- [x][WASM] PR #111 Round 3 Copilot fixes — C-3-001/2/3 duplicate JSDoc blocks removed from decoderInit/autoDecoderInit/validateMemlimit, C-3-004 stale xzAsync/unxzAsync comment fixed in lzma.ts:370; tsc+memlimit+full suite pass (2026-04-28)
24+
- [x][WASM] PR #111 Round 2 Copilot fixes — C-2-001 TSDoc xzAsync removed from honored-by list, C-2-002 stale lzma.ts comment, C-2-003 LZMA_OPTIONS_ERROR constant replaces magic 8, C-2-004 MAX_SAFE_INTEGER guard + TSDoc, C-2-005 validateMemlimit lifted to decoderInit+autoDecoderInit; 12 new tests, 474+99+27=600 tests pass (2026-04-28)
25+
- [x][WASM] PR #111 Round 1 review fixes — F-001 memlimit validation (NaN/Inf/frac/neg → LZMAOptionsError), F-002 ResolvedLZMAOptions internal type, C-001/C-002 async callback fixture pattern, C-003 byte-equality assertion, F-003 TSDoc reorder, F-004 stale comment, F-005 fixture comment magnitude, F-006 default-path caveat; 4 new tests (12 total in decompress-memlimit.test.ts), 458+99+27=584 tests pass (2026-04-28)
26+
- [x][WASM] Wire `memlimit` through `LZMAOptions``unxzAsync`/`unxz``LZMAMemoryLimitError` thrown when limit exceeded; 8 new tests in `test/wasm/decompress-memlimit.test.ts`; TSDoc with parity note (2026-04-28)
2127
- [x][tar-xz v6] Universal stream-first redesign: `create()`/`extract()`/`list()` with `AsyncIterable<Uint8Array>`, identical Node/Browser signatures, `tar-xz/file` subpath for fs helpers — published as `tar-xz@6.0.0` + `nxz-cli@6.0.0` (2026-04-27)
2228
- [x][tar-xz v6] Security hardening: 18 path/symlink TOCTOU vectors audited and closed (leaf check, ENOENT walk, hardlink linkSource, NUL/empty rejection, setuid mask, fd-based fs ops with O_NOFOLLOW, pipeline error propagation) — 8 Copilot review rounds + 1 consolidated audit (2026-04-27)
2329
- [x][Infra] Independent versioning per workspace package: `release.yml`/`publish.yml` accept `target_package` input, no cross-package version sync; proven in prod — `tar-xz@6.0.0` published without bumping `node-liblzma` (still at 5.0.0) (2026-04-27)

src/errors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ export class LZMAFormatError extends LZMAError {
6666
* Options error - thrown when invalid or unsupported options are provided
6767
*/
6868
export class LZMAOptionsError extends LZMAError {
69-
constructor(errno: number) {
70-
super('Invalid or unsupported options', errno);
69+
constructor(errno: number, message?: string) {
70+
super(message ?? 'Invalid or unsupported options', errno);
7171
this.name = 'LZMAOptionsError';
7272
}
7373
}

src/lzma.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,29 @@ export type {
301301
*
302302
* Emits `progress` event after each chunk with `{bytesRead, bytesWritten}` info.
303303
*/
304+
305+
/**
306+
* Internal resolved options for XzStream instances.
307+
*
308+
* All fields are required (defaults applied in constructor) EXCEPT memlimit,
309+
* which is genuinely optional: the native binding ignores it (UINT64_MAX
310+
* hardcoded; see TODO "[Native] Wire memlimit in src/bindings/node-liblzma.cpp").
311+
* Only the WASM Buffer API decompression paths (unxz/unxzAsync/streamBufferDecode) honour memlimit; xzAsync is compression-only and ignores this field.
312+
*/
313+
interface ResolvedLZMAOptions {
314+
check: number;
315+
preset: number;
316+
filters: number[];
317+
mode: number;
318+
threads: number;
319+
chunkSize: number;
320+
flushFlag: number;
321+
/** Honoured only by the WASM Buffer API; native streams ignore this field. */
322+
memlimit?: number | bigint;
323+
}
324+
304325
export abstract class XzStream extends Transform {
305-
protected _opts: Required<LZMAOptions>;
326+
protected _opts: ResolvedLZMAOptions;
306327
protected _chunkSize: number;
307328
protected _flushFlag: number;
308329
protected lzma: NativeLZMA;
@@ -344,6 +365,10 @@ export abstract class XzStream extends Transform {
344365
threads: opts.threads ?? 1,
345366
chunkSize: opts.chunkSize ?? liblzma.BUFSIZ,
346367
flushFlag: opts.flushFlag ?? liblzma.LZMA_RUN,
368+
// memlimit is genuinely optional in ResolvedLZMAOptions: the native binding ignores it
369+
// (UINT64_MAX hardcoded; see TODO "[Native] Wire memlimit in src/bindings/node-liblzma.cpp").
370+
// Only the WASM decompression APIs (unxz/unxzAsync/streamBufferDecode) honour this field. xzAsync is compression-only and ignores it.
371+
memlimit: opts.memlimit,
347372
};
348373

349374
this._chunkSize = this._opts.chunkSize;

src/types.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,32 @@ export interface LZMAOptions {
5353
chunkSize?: number;
5454
/** Flush flag to use */
5555
flushFlag?: number;
56+
/**
57+
* Memory usage limit for decompression, in bytes.
58+
*
59+
* **Honored only by `unxz`/`unxzAsync` (WASM Buffer API decompression).**
60+
* `xzAsync` is compression-only and does not read `memlimit`.
61+
* Currently **silently ignored** by native streams (`Xz`/`Unxz`/`createXz`/`createUnxz`) —
62+
* see TODO `[Native] Wire memlimit in src/bindings/node-liblzma.cpp`.
63+
*
64+
* Accepted types: `number` or `bigint` (both are validated then coerced to
65+
* `bigint` before being passed to the WASM C ABI, which maps `uint64_t` to
66+
* `BigInt`). The `number` form must be a finite, non-negative integer; passing
67+
* `NaN`, `Infinity`, a fractional value, or a negative number throws
68+
* `LZMAOptionsError` before any decompression is attempted.
69+
* For values ≥ `Number.MAX_SAFE_INTEGER` (2^53 - 1), use `bigint` to avoid
70+
* precision loss on coercion; passing a `number` above this threshold also
71+
* throws `LZMAOptionsError`.
72+
*
73+
* WASM default: `BigInt(256 * 1024 * 1024)` (256 MiB).
74+
* Native: ignored (UINT64_MAX hardcoded — see follow-up TODO above).
75+
*
76+
* When the compressed stream requires more memory than this limit,
77+
* decompression throws `LZMAMemoryLimitError` with
78+
* `code === LZMA_MEMLIMIT_ERROR` (numeric constant `6`, re-exported from
79+
* `src/errors.ts`).
80+
*/
81+
memlimit?: number | bigint;
5682
}
5783

5884
/**

src/wasm/bindings.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* with proper memory management and error handling.
66
*/
77

8-
import { createLZMAError, LZMAError } from '../errors.js';
8+
import { createLZMAError, LZMAError, LZMA_OPTIONS_ERROR, LZMAOptionsError } from '../errors.js';
99
import { copyFromWasm, copyToWasm, type WasmLzmaStream, wasmAlloc, wasmFree } from './memory.js';
1010
import {
1111
LZMA_BUF_ERROR,
@@ -116,12 +116,14 @@ export function encoderInit(
116116
*
117117
* @param stream - Allocated WasmLzmaStream
118118
* @param memlimit - Memory limit in bytes (default: 256MB)
119+
* @throws LZMAOptionsError if memlimit is invalid
119120
* @throws LZMAError on initialization failure
120121
*/
121122
export function decoderInit(
122123
stream: WasmLzmaStream,
123124
memlimit: number | bigint = DEFAULT_MEMLIMIT
124125
): void {
126+
validateMemlimit(memlimit);
125127
const module = getModule();
126128
const limit = typeof memlimit === 'number' ? BigInt(memlimit) : memlimit;
127129
const ret = module._lzma_stream_decoder(stream.ptr, limit, 0);
@@ -137,12 +139,14 @@ export function decoderInit(
137139
*
138140
* @param stream - Allocated WasmLzmaStream
139141
* @param memlimit - Memory limit in bytes (default: 256MB)
142+
* @throws LZMAOptionsError if memlimit is invalid
140143
* @throws LZMAError on initialization failure
141144
*/
142145
export function autoDecoderInit(
143146
stream: WasmLzmaStream,
144147
memlimit: number | bigint = DEFAULT_MEMLIMIT
145148
): void {
149+
validateMemlimit(memlimit);
146150
const module = getModule();
147151
const limit = typeof memlimit === 'number' ? BigInt(memlimit) : memlimit;
148152
const ret = module._lzma_auto_decoder(stream.ptr, limit, 0);
@@ -244,6 +248,49 @@ export function easyBufferEncode(
244248
* @returns Decompressed data
245249
* @throws LZMAError on decompression failure
246250
*/
251+
252+
/**
253+
* Validate a memlimit value before coercion to BigInt.
254+
*
255+
* BigInt() throws a native RangeError for NaN, Infinity, and non-integer
256+
* numbers (e.g. 1.5). Negative integers produce a huge unsigned value when
257+
* interpreted by the C ABI (uint64_t wrap-around). Numbers above
258+
* Number.MAX_SAFE_INTEGER (2^53 - 1) lose precision on coercion to BigInt.
259+
* All these cases are rejected here with LZMAOptionsError so callers always
260+
* get an LZMAError subclass.
261+
*
262+
* @throws LZMAOptionsError if the value is invalid
263+
*/
264+
function validateMemlimit(memlimit: number | bigint): void {
265+
if (typeof memlimit === 'bigint') {
266+
if (memlimit < 0n) {
267+
throw new LZMAOptionsError(LZMA_OPTIONS_ERROR, 'memlimit must be a non-negative value');
268+
}
269+
return;
270+
}
271+
if (!Number.isFinite(memlimit)) {
272+
throw new LZMAOptionsError(
273+
LZMA_OPTIONS_ERROR,
274+
'memlimit must be a finite number (NaN and Infinity are not allowed)'
275+
);
276+
}
277+
if (!Number.isInteger(memlimit)) {
278+
throw new LZMAOptionsError(
279+
LZMA_OPTIONS_ERROR,
280+
'memlimit must be an integer (fractional values are not allowed)'
281+
);
282+
}
283+
if (memlimit < 0) {
284+
throw new LZMAOptionsError(LZMA_OPTIONS_ERROR, 'memlimit must be a non-negative value');
285+
}
286+
if (memlimit > Number.MAX_SAFE_INTEGER) {
287+
throw new LZMAOptionsError(
288+
LZMA_OPTIONS_ERROR,
289+
'memlimit number exceeds MAX_SAFE_INTEGER (use bigint for values >= 2^53)'
290+
);
291+
}
292+
}
293+
247294
export function streamBufferDecode(
248295
input: Uint8Array,
249296
memlimit: number | bigint = DEFAULT_MEMLIMIT
@@ -261,6 +308,7 @@ export function streamBufferDecode(
261308
let outPtr = wasmAlloc(module, outSize);
262309

263310
try {
311+
validateMemlimit(memlimit);
264312
const limit = typeof memlimit === 'number' ? BigInt(memlimit) : memlimit;
265313
module.setValue(memlimitPtr, limit, 'i64');
266314

src/wasm/decompress.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,11 @@ import { toUint8Array } from './utils.js';
1919
*/
2020
export async function unxzAsync(
2121
buffer: Uint8Array | ArrayBuffer | string,
22-
_opts?: LZMAOptions
22+
opts?: LZMAOptions
2323
): Promise<Uint8Array> {
2424
await initModule();
2525
const input = toUint8Array(buffer);
26-
// TODO: pass opts.memlimit when LZMAOptions supports it
27-
return streamBufferDecode(input);
26+
return streamBufferDecode(input, opts?.memlimit);
2827
}
2928

3029
/**

0 commit comments

Comments
 (0)