From aa683a314db0e7d601b7ecaf2d288f2a4a93506f Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 22 Aug 2024 13:20:43 -0400 Subject: [PATCH] test zlib destroy and reset before write --- src/node/internal/internal_zlib_base.ts | 21 +++-- .../api/node/tests/zlib-nodejs-test.js | 84 ++++++++++++++++++- src/workerd/api/node/zlib-util.c++ | 4 +- 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/src/node/internal/internal_zlib_base.ts b/src/node/internal/internal_zlib_base.ts index 4d15a2ac69d..73fe2f78e28 100644 --- a/src/node/internal/internal_zlib_base.ts +++ b/src/node/internal/internal_zlib_base.ts @@ -74,6 +74,7 @@ function processCallback(this: zlibUtil.ZlibStream): void { // eslint-disable-next-line @typescript-eslint/no-this-alias const handle = this; const self = this[owner_symbol]; + ok(self, 'Owner symbol should exist'); const state = self._writeState; if (self.destroyed) { @@ -204,6 +205,7 @@ function zlibOnError( message: string ): void { const self = this[owner_symbol]; + ok(self, 'Owner symbol should exist'); const error = new NodeError(code, message); // @ts-expect-error Err number is expected. error.errno = errno; @@ -301,7 +303,7 @@ function processChunkSync( function _close(engine: ZlibBase): void { engine._handle?.close(); - engine._handle = undefined; + engine._handle = null; } type ZlibDefaultOptions = { @@ -323,11 +325,11 @@ export class ZlibBase extends Transform { public _outBuffer: Buffer; public _outOffset: number = 0; public _chunkSize: number; - public _defaultFlushFlag: number | undefined; - public _finishFlushFlag: number | undefined; - public _defaultFullFlushFlag: number | undefined; + public _defaultFlushFlag: number; + public _finishFlushFlag: number; + public _defaultFullFlushFlag: number; public _info: unknown; - public _handle: zlibUtil.ZlibStream | undefined; + public _handle: zlibUtil.ZlibStream | null = null; public _writeState = new Uint32Array(2); public [kError]: NodeError | undefined; @@ -449,7 +451,7 @@ export class ZlibBase extends Transform { ): void { if (typeof kind === 'function' || (kind == null && !callback)) { callback = kind as (() => void) | undefined; - kind = this._defaultFlushFlag as number; + kind = this._defaultFlushFlag; } if (this.writableFinished) { @@ -496,12 +498,9 @@ export class ZlibBase extends Transform { // For the last chunk, also apply `_finishFlushFlag`. if (this.writableEnded && this.writableLength === chunk.byteLength) { - flushFlag = maxFlush( - flushFlag as number, - this._finishFlushFlag as number - ); + flushFlag = maxFlush(flushFlag, this._finishFlushFlag); } - this.#processChunk(chunk, flushFlag as number, cb); + this.#processChunk(chunk, flushFlag, cb); } // This function is left for backwards compatibility. diff --git a/src/workerd/api/node/tests/zlib-nodejs-test.js b/src/workerd/api/node/tests/zlib-nodejs-test.js index 0cc072a3505..b2a8652eaac 100644 --- a/src/workerd/api/node/tests/zlib-nodejs-test.js +++ b/src/workerd/api/node/tests/zlib-nodejs-test.js @@ -612,6 +612,86 @@ export const testFailedInit = { }, }; +// Tests are taken from: +// https://github.com/nodejs/node/blob/561bc87c7607208f0d3db6dcd9231efeb48cfe2f/test/parallel/test-zlib-destroy.js +export const zlibDestroyTest = { + async test() { + const promises = []; + // Verify that the zlib transform does clean up + // the handle when calling destroy. + { + const ts = zlib.createGzip(); + ts.destroy(); + assert.strictEqual(ts._handle, null); + + const { promise, resolve, reject } = Promise.withResolvers(); + promises.push(promise); + ts.on('error', reject); + ts.on('close', () => { + ts.close(() => { + resolve(); + }); + }); + } + + { + // Ensure 'error' is only emitted once. + const decompress = zlib.createGunzip(15); + const { promise, resolve, reject } = Promise.withResolvers(); + promises.push(promise); + let errorCount = 0; + decompress.on('error', (err) => { + errorCount++; + decompress.close(); + assert.strictEqual(errorCount, 1, 'Error should only be emitted once'); + resolve(); + }); + + decompress.write('something invalid'); + decompress.destroy(new Error('asd')); + } + + await Promise.all(promises); + }, +}; + +// Tests are taken from: +// https://github.com/nodejs/node/blob/561bc87c7607208f0d3db6dcd9231efeb48cfe2f/test/parallel/test-zlib-reset-before-write.js +export const testZlibResetBeforeWrite = { + async test() { + const promises = []; + + { + const { promise, resolve, reject } = Promise.withResolvers(); + promises.push(promise); + // Should not error when resetting after a write() but before a flush() + const gz = zlib.createGzip(); + gz.write('hello', () => { + gz.reset(); + gz.end('world', () => { + resolve(); + }); + }); + gz.on('error', reject); + } + + { + const { promise, resolve, reject } = Promise.withResolvers(); + promises.push(promise); + // Should not error when resetting after a write() but before a flush() + const gz = zlib.createGzip(); + gz.write('hello'); + gz.reset(); + gz.end('world', () => { + resolve(); + }); + gz.on('error', reject); + } + + await Promise.all(promises); + }, +}; + // Node.js tests relevant to zlib // // - [ ] test-zlib-brotli-16GB.js @@ -640,7 +720,7 @@ export const testFailedInit = { // - [ ] test-zlib-no-stream.js // - [ ] test-zlib-write-after-close.js // - [ ] test-zlib-brotli-kmaxlength-rangeerror.js -// - [ ] test-zlib-destroy.js +// - [x] test-zlib-destroy.js // - [ ] test-zlib-from-concatenated-gzip.js // - [ ] test-zlib-not-string-or-buffer.js // - [ ] test-zlib-write-after-end.js @@ -666,4 +746,4 @@ export const testFailedInit = { // - [ ] test-zlib-const.js // - [x] test-zlib-failed-init.js // - [ ] test-zlib-invalid-input.js -// - [ ] test-zlib-reset-before-write.js +// - [x] test-zlib-reset-before-write.js diff --git a/src/workerd/api/node/zlib-util.c++ b/src/workerd/api/node/zlib-util.c++ index 77632c6c9b5..3fbf6f06f7f 100644 --- a/src/workerd/api/node/zlib-util.c++ +++ b/src/workerd/api/node/zlib-util.c++ @@ -429,10 +429,10 @@ void ZlibUtil::ZlibStream::write(jsg::Lock& js, } // Check bounds - JSG_REQUIRE(inputOffset > 0 && inputOffset < input.size(), Error, + JSG_REQUIRE(inputOffset >= 0 && inputOffset < input.size(), Error, "Offset should be smaller than size and bigger than 0"_kj); JSG_REQUIRE(input.size() >= inputLength, Error, "Invalid inputLength"_kj); - JSG_REQUIRE(outputOffset > 0 && outputOffset < output.size(), Error, + JSG_REQUIRE(outputOffset >= 0 && outputOffset < output.size(), Error, "Offset should be smaller than size and bigger than 0"_kj); JSG_REQUIRE(output.size() >= outputLength, Error, "Invalid outputLength"_kj);