diff --git a/src/node/internal/internal_zlib_base.ts b/src/node/internal/internal_zlib_base.ts index 4d15a2ac69d..443fd43c01e 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; @@ -398,7 +400,7 @@ export class ZlibBase extends Transform { super({ autoDestroy: true, ...opts } as unknown); // Error handler by processCallback() and zlibOnError() - handle.setErrorHandler(zlibOnError); + handle.setErrorHandler(zlibOnError.bind(handle)); handle[owner_symbol] = this as never; this._handle = handle; this._outBuffer = Buffer.allocUnsafe(chunkSize); @@ -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..56fa27a5e7e 100644 --- a/src/workerd/api/node/tests/zlib-nodejs-test.js +++ b/src/workerd/api/node/tests/zlib-nodejs-test.js @@ -612,6 +612,94 @@ 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-close-after-error.js +export const closeAfterError = { + async test() { + const decompress = zlib.createGunzip(15); + const { promise, resolve } = Promise.withResolvers(); + let errorHasBeenCalled = false; + + decompress.on('error', () => { + errorHasBeenCalled = true; + strictEqual(decompress._closed, true); + decompress.close(); + }); + + strictEqual(decompress._closed, false); + decompress.write('something invalid'); + decompress.on('close', resolve); + + await promise; + + assert(errorHasBeenCalled, 'Error handler should have been called'); + }, +}; + +// Tests are taken from: +// https://github.com/nodejs/node/blob/561bc87c7607208f0d3db6dcd9231efeb48cfe2f/test/parallel/test-zlib-write-after-close.js +// TODO(soon): Enable the test once `gzip` implementation lands. +// export const writeAfterClose = { +// async test() { +// const { promise, resolve } = Promise.withResolvers(); +// let closeCalled = false; +// zlib.gzip('hello', function (err, out) { +// const unzip = zlib.createGunzip(); +// unzip.close(() => (closeCalled = true)); +// unzip.write('asd', (err) => { +// strictEqual(err.code, 'ERR_STREAM_DESTROYED'); +// strictEqual(err.name, 'Error'); +// resolve(); +// }); +// }); +// await promise; +// assert(closeCalled, 'Close should have been called'); +// }, +// }; + // Node.js tests relevant to zlib // // - [ ] test-zlib-brotli-16GB.js @@ -638,9 +726,9 @@ export const testFailedInit = { // - [ ] test-zlib-deflate-raw-inherits.js // - [ ] test-zlib-flush-write-sync-interleaved.js // - [ ] test-zlib-no-stream.js -// - [ ] test-zlib-write-after-close.js +// - [x] 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 @@ -649,7 +737,7 @@ export const testFailedInit = { // - [ ] test-zlib-from-gzip.js // - [ ] test-zlib-object-write.js // - [ ] test-zlib-write-after-flush.js -// - [ ] test-zlib-close-after-error.js +// - [x] test-zlib-close-after-error.js // - [ ] test-zlib-dictionary-fail.js // - [ ] test-zlib-from-gzip-with-trailing-garbage.js // - [ ] test-zlib-params.js diff --git a/src/workerd/api/node/zlib-util.c++ b/src/workerd/api/node/zlib-util.c++ index 77632c6c9b5..c5558425989 100644 --- a/src/workerd/api/node/zlib-util.c++ +++ b/src/workerd/api/node/zlib-util.c++ @@ -305,7 +305,7 @@ void ZlibContext::close() { status = inflateEnd(&stream); break; default: - KJ_UNREACHABLE; + break; } JSG_REQUIRE( @@ -359,7 +359,6 @@ void CompressionStream::writeStream(jsg::Lock& js, JSG_REQUIRE(!writing, Error, "Writing is in progress"_kj); JSG_REQUIRE(!pending_close, Error, "Pending close"_kj); - // Ref(); context.setBuffers(input, inputLength, output, outputLength); context.setFlush(flush); @@ -369,7 +368,6 @@ void CompressionStream::writeStream(jsg::Lock& js, context.getAfterWriteOffsets(writeResult); writing = false; } - // Unref(); } template @@ -429,10 +427,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);