From 90e408143adc0573fcf8911c100d798559a8790b Mon Sep 17 00:00:00 2001 From: 0hmX Date: Tue, 13 May 2025 23:36:27 +0530 Subject: [PATCH 1/3] zlib: add error handling for trailing junk after stream end Introduce `ERR_TRAILING_JUNK_AFTER_STREAM_END` error to handle unexpected data after the end of a compressed stream. This ensures proper error reporting when decompressing deflate or gzip streams with trailing junk. Added tests to verify the behavior. Fixes: https://github.com/nodejs/node/issues/58247 --- doc/api/errors.md | 9 +++++ lib/internal/errors.js | 2 ++ lib/internal/webstreams/compression.js | 16 +++++++-- lib/zlib.js | 12 +++++++ test/parallel/test-zlib-type-error.js | 46 ++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-zlib-type-error.js diff --git a/doc/api/errors.md b/doc/api/errors.md index ccf8d16655cec4..f868d44f4ab638 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -3012,6 +3012,15 @@ category. The `node:trace_events` module could not be loaded because Node.js was compiled with the `--without-v8-platform` flag. + + +### `ERR_TRAILING_JUNK_AFTER_STREAM_END` + +Trailing junk found after the end of the compressed stream. +This error is thrown when extra, unexpected data is detected +after the end of a compressed stream (for example, in zlib +or gzip decompression). + ### `ERR_TRANSFORM_ALREADY_TRANSFORMING` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 87200b40d45bdc..18925ee5b2801c 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1803,6 +1803,8 @@ E('ERR_TRACE_EVENTS_CATEGORY_REQUIRED', 'At least one category is required', TypeError); E('ERR_TRACE_EVENTS_UNAVAILABLE', 'Trace events are unavailable', Error); +E('ERR_TRAILING_JUNK_AFTER_STREAM_END', 'Trailing junk found after the end of the compressed stream', TypeError); + // This should probably be a `RangeError`. E('ERR_TTY_INIT_FAILED', 'TTY initialization failed', SystemError); E('ERR_UNAVAILABLE_DURING_EXIT', 'Cannot call function in process exit ' + diff --git a/lib/internal/webstreams/compression.js b/lib/internal/webstreams/compression.js index 2299b9c49e05bd..b33b7134096c8b 100644 --- a/lib/internal/webstreams/compression.js +++ b/lib/internal/webstreams/compression.js @@ -99,16 +99,28 @@ class DecompressionStream { }); switch (format) { case 'deflate': - this.#handle = lazyZlib().createInflate(); + this.#handle = lazyZlib().createInflate({ + rejectGarbageAfterEnd: true, + }); break; case 'deflate-raw': this.#handle = lazyZlib().createInflateRaw(); break; case 'gzip': - this.#handle = lazyZlib().createGunzip(); + this.#handle = lazyZlib().createGunzip({ + rejectGarbageAfterEnd: true, + }); break; } this.#transform = newReadableWritablePairFromDuplex(this.#handle); + + this.#handle.on('error', (err) => { + if (this.#transform?.writable && + !this.#transform.writable.locked && + typeof this.#transform.writable.abort === 'function') { + this.#transform.writable.abort(err); + } + }); } /** diff --git a/lib/zlib.js b/lib/zlib.js index 058e03c7180620..0a1573416abf6f 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -42,6 +42,7 @@ const { ERR_BUFFER_TOO_LARGE, ERR_INVALID_ARG_TYPE, ERR_OUT_OF_RANGE, + ERR_TRAILING_JUNK_AFTER_STREAM_END, ERR_ZSTD_INVALID_PARAM, }, genericNodeError, @@ -266,6 +267,8 @@ function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) { this._defaultFullFlushFlag = fullFlush; this._info = opts?.info; this._maxOutputLength = maxOutputLength; + + this._rejectGarbageAfterEnd = opts?.rejectGarbageAfterEnd === true; } ObjectSetPrototypeOf(ZlibBase.prototype, Transform.prototype); ObjectSetPrototypeOf(ZlibBase, Transform); @@ -570,6 +573,14 @@ function processCallback() { // stream has ended early. // This applies to streams where we don't check data past the end of // what was consumed; that is, everything except Gunzip/Unzip. + + if (self._rejectGarbageAfterEnd) { + const err = new ERR_TRAILING_JUNK_AFTER_STREAM_END(); + self.destroy(err); + this.cb(err); + return; + } + self.push(null); } @@ -662,6 +673,7 @@ function Zlib(opts, mode) { this._level = level; this._strategy = strategy; + this._mode = mode; } ObjectSetPrototypeOf(Zlib.prototype, ZlibBase.prototype); ObjectSetPrototypeOf(Zlib, ZlibBase); diff --git a/test/parallel/test-zlib-type-error.js b/test/parallel/test-zlib-type-error.js new file mode 100644 index 00000000000000..3432d75e346ef1 --- /dev/null +++ b/test/parallel/test-zlib-type-error.js @@ -0,0 +1,46 @@ +'use strict'; +require('../common'); +const assert = require('assert').strict; +const test = require('node:test'); +const { DecompressionStream } = require('stream/web'); + +async function expectTypeError(promise) { + let threw = false; + try { + await promise; + } catch (err) { + threw = true; + assert(err instanceof TypeError, `Expected TypeError, got ${err}`); + } + assert(threw, 'Expected promise to reject'); +} + +test('DecompressStream deflat emits error on trailing data', async () => { + const valid = new Uint8Array([120, 156, 75, 4, 0, 0, 98, 0, 98]); // deflate('a') + const empty = new Uint8Array(1); + const invalid = new Uint8Array([...valid, ...empty]); + const double = new Uint8Array([...valid, ...valid]); + + for (const chunk of [[invalid], [valid, empty], [valid, valid], [valid, double]]) { + await expectTypeError( + Array.fromAsync( + new Blob([chunk]).stream().pipeThrough(new DecompressionStream('deflate')) + ) + ); + } +}); + +test('DecompressStream gzip emits error on trailing data', async () => { + const valid = new Uint8Array([31, 139, 8, 0, 0, 0, 0, 0, 0, 19, 75, 4, + 0, 67, 190, 183, 232, 1, 0, 0, 0]); // gzip('a') + const empty = new Uint8Array(1); + const invalid = new Uint8Array([...valid, ...empty]); + const double = new Uint8Array([...valid, ...valid]); + for (const chunk of [[invalid], [valid, empty], [valid, valid], [double]]) { + await expectTypeError( + Array.fromAsync( + new Blob([chunk]).stream().pipeThrough(new DecompressionStream('gzip')) + ) + ); + } +}); From f8532f0ac58bc4320af4137d23779ccec73a1fe8 Mon Sep 17 00:00:00 2001 From: 0hmX Date: Thu, 15 May 2025 21:25:53 +0530 Subject: [PATCH 2/3] test: remove expected failures for decompression-corrupt-input.tentative.any.js All subtests in decompression-corrupt input.tentative.any.js now pass, so the "fail.expected" entry has been removed from the status file. This reflects the current passing state of the test suite. --- test/wpt/status/compression.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/wpt/status/compression.json b/test/wpt/status/compression.json index 0cae222832e823..be073427810f0d 100644 --- a/test/wpt/status/compression.json +++ b/test/wpt/status/compression.json @@ -11,14 +11,6 @@ "compression-with-detach.tentative.window.js": { "requires": ["crypto"] }, - "decompression-corrupt-input.tentative.any.js": { - "fail": { - "expected": [ - "trailing junk for 'deflate' should give an error", - "trailing junk for 'gzip' should give an error" - ] - } - }, "idlharness-shadowrealm.window.js": { "skip": "ShadowRealm support is not enabled" }, From b43863c6758129925ef534ee88bed85a8650a82f Mon Sep 17 00:00:00 2001 From: 0hmX Date: Thu, 15 May 2025 21:32:14 +0530 Subject: [PATCH 3/3] docs: fix formatting in errors.md documentation Corrected the formatting of the `ERR_TRAILING_JUNK_AFTER_STREAM_END` section by removing trailing spaces and ensuring consistent line breaks for better readability. --- doc/api/errors.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index f868d44f4ab638..c87875ddff51be 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -3016,9 +3016,9 @@ with the `--without-v8-platform` flag. ### `ERR_TRAILING_JUNK_AFTER_STREAM_END` -Trailing junk found after the end of the compressed stream. -This error is thrown when extra, unexpected data is detected -after the end of a compressed stream (for example, in zlib +Trailing junk found after the end of the compressed stream. +This error is thrown when extra, unexpected data is detected +after the end of a compressed stream (for example, in zlib or gzip decompression).