Skip to content

Commit

Permalink
improve zlib for nodejs_compat
Browse files Browse the repository at this point in the history
  • Loading branch information
@anonrig
anonrig committed Aug 26, 2024
1 parent 2c6af65 commit f29bf49
Show file tree
Hide file tree
Showing 6 changed files with 264 additions and 77 deletions.
40 changes: 24 additions & 16 deletions src/node/internal/internal_zlib_base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ function processCallback(this: zlibUtil.ZlibStream): void {
return;
}

const [availOutAfter, availInAfter] = state as unknown as [number, number];
const availOutAfter = state[0] as number;
const availInAfter = state[1] as number;

const inDelta = handle.availInBefore - availInAfter;
self.bytesWritten += inDelta;
Expand Down Expand Up @@ -119,9 +120,10 @@ function processCallback(this: zlibUtil.ZlibStream): void {
handle.availInBefore = availInAfter;

if (!streamBufferIsFull) {
ok(this.buffer, 'Buffer should not have been null');
this.write(
handle.flushFlag,
this.buffer as NodeJS.TypedArray, // in
this.buffer, // in
handle.inOff, // in_off
handle.availInBefore, // in_len
self._outBuffer, // out
Expand All @@ -132,10 +134,11 @@ function processCallback(this: zlibUtil.ZlibStream): void {
// eslint-disable-next-line @typescript-eslint/unbound-method
const oldRead = self._read;
self._read = (n): void => {
ok(this.buffer, 'Buffer should not have been null');
self._read = oldRead;
this.write(
handle.flushFlag,
this.buffer as NodeJS.TypedArray, // in
this.buffer, // in
handle.inOff, // in_off
handle.availInBefore, // in_len
self._outBuffer, // out
Expand Down Expand Up @@ -171,7 +174,7 @@ function processCallback(this: zlibUtil.ZlibStream): void {
// Z_NO_FLUSH (< Z_TREES) < Z_BLOCK < Z_PARTIAL_FLUSH <
// Z_SYNC_FLUSH < Z_FULL_FLUSH < Z_FINISH
const flushiness: number[] = [];
const kFlushFlagList = [
const kFlushFlagList: number[] = [
CONST_Z_NO_FLUSH,
CONST_Z_BLOCK,
CONST_Z_PARTIAL_FLUSH,
Expand Down Expand Up @@ -239,16 +242,16 @@ function processChunkSync(
});

while (true) {
// TODO(soon): This was `writeSync` before, but it's not anymore.
handle?.write(
ok(handle, 'Handle should have been defined');
handle.writeSync(
flushFlag,
chunk, // in
inOff, // in_off
availInBefore, // in_len
buffer, // out
offset, // out_off
availOutBefore
); // out_len
availOutBefore // out_len
);
if (error) throw error;
else if (self[kError]) throw self[kError];

Expand Down Expand Up @@ -461,8 +464,7 @@ export class ZlibBase extends Transform {
}
} else if (this.writableEnded) {
if (callback) {
/* eslint-disable-next-line @typescript-eslint/no-unsafe-call */
queueMicrotask(callback);
this.once('end', callback);
}
} else {
this.write(kFlushBuffers[kind as number], 'utf8', callback);
Expand Down Expand Up @@ -517,16 +519,17 @@ export class ZlibBase extends Transform {
}

#processChunk(chunk: Buffer, flushFlag: number, cb: () => void): void {
if (this._handle == null) {
if (!this._handle) {
/* eslint-disable-next-line @typescript-eslint/no-unsafe-call */
queueMicrotask(cb);
return;
}

this._handle.buffer = null;
this._handle.buffer = chunk;
this._handle.cb = cb;
this._handle.availOutBefore = this._chunkSize - this._outOffset;
this._handle.availInBefore = chunk.byteLength;
this._handle.inOff = 0;
this._handle.flushFlag = flushFlag;

this._handle.write(
Expand Down Expand Up @@ -603,7 +606,7 @@ export class Zlib extends ZlibBase {
);
dictionary = options.dictionary;

if (dictionary != null && !isArrayBufferView(dictionary)) {
if (dictionary !== undefined && !isArrayBufferView(dictionary)) {
if (isAnyArrayBuffer(dictionary)) {
dictionary = Buffer.from(dictionary);
} else {
Expand All @@ -618,13 +621,18 @@ export class Zlib extends ZlibBase {

const writeState = new Uint32Array(2);
const handle = new zlibUtil.ZlibStream(mode);

handle.initialize(
windowBits,
level,
memLevel,
strategy,
writeState,
processCallback,

() => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
queueMicrotask(processCallback.bind(handle));
},
dictionary
);
super(options ?? {}, mode, handle);
Expand All @@ -635,7 +643,7 @@ export class Zlib extends ZlibBase {
this._writeState = writeState;
}

public params(level: number, strategy: number, callback: () => never): void {
public params(level: number, strategy: number, callback: () => void): void {
checkRangesOrGetDefault(
level,
'level',
Expand All @@ -656,7 +664,7 @@ export class Zlib extends ZlibBase {
);
} else {
/* eslint-disable-next-line @typescript-eslint/no-unsafe-call */
queueMicrotask(callback);
queueMicrotask(() => callback());
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/node/internal/zlib.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ export class ZlibStream {
outputOffset: number,
outputLength: number
): void;
public writeSync(
flushFlag: number,
inputBuffer: NodeJS.TypedArray,
inputOffset: number,
inputLength: number,
outputBuffer: NodeJS.TypedArray,
outputOffset: number,
outputLength: number
): void;
public params(level: number, strategy: number): void;
public reset(): void;

Expand Down
69 changes: 64 additions & 5 deletions src/workerd/api/node/tests/zlib-nodejs-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,12 +599,12 @@ export const testFailedInit = {

{
const stream = zlib.createGzip({ level: NaN });
assert.strictEqual(stream._level, zlib.constants.Z_DEFAULT_COMPRESSION);
strictEqual(stream._level, zlib.constants.Z_DEFAULT_COMPRESSION);
}

{
const stream = zlib.createGzip({ strategy: NaN });
assert.strictEqual(stream._strategy, zlib.constants.Z_DEFAULT_STRATEGY);
strictEqual(stream._strategy, zlib.constants.Z_DEFAULT_STRATEGY);
}
},
};
Expand All @@ -619,7 +619,7 @@ export const zlibDestroyTest = {
{
const ts = zlib.createGzip();
ts.destroy();
assert.strictEqual(ts._handle, null);
strictEqual(ts._handle, null);

const { promise, resolve, reject } = Promise.withResolvers();
promises.push(promise);
Expand All @@ -640,7 +640,7 @@ export const zlibDestroyTest = {
decompress.on('error', (err) => {
errorCount++;
decompress.close();
assert.strictEqual(errorCount, 1, 'Error should only be emitted once');
strictEqual(errorCount, 1, 'Error should only be emitted once');
resolve();
});

Expand Down Expand Up @@ -697,6 +697,65 @@ export const closeAfterError = {
// },
// };

// Tests are taken from:
// https://github.com/nodejs/node/blob/9edf4a0856681a7665bd9dcf2ca7cac252784b98/test/parallel/test-zlib-bytes-read.js
export const testZlibBytesRead = {
async test() {
const expectStr = 'abcdefghijklmnopqrstuvwxyz'.repeat(2);
const expectBuf = Buffer.from(expectStr);

function createWriter(target, buffer) {
const writer = { size: 0 };
const write = () => {
target.write(Buffer.from([buffer[writer.size++]]), () => {
if (writer.size < buffer.length) {
target.flush(write);
} else {
target.end();
}
});
};
write();
return writer;
}

// This test is simplified a lot because of test runner limitations.
// TODO(soon): Add createBrotliCompress once it is implemented.
for (const method of ['createGzip', 'createDeflate', 'createDeflateRaw']) {
assert(method in zlib, `${method} is not available in "node:zlib"`);
const { promise, resolve, reject } = Promise.withResolvers();
let compData = Buffer.alloc(0);
const comp = zlib[method]();
const compWriter = createWriter(comp, expectBuf);
comp.on('data', function (d) {
compData = Buffer.concat([compData, d]);
strictEqual(
this.bytesWritten,
compWriter.size,
`Should get write size on ${method} data.`
);
});
comp.on('error', reject);
comp.on('end', function () {
strictEqual(
this.bytesWritten,
compWriter.size,
`Should get write size on ${method} end.`
);
strictEqual(
this.bytesWritten,
expectStr.length,
`Should get data size on ${method} end.`
);

resolve();
});

await promise;
}
},
};

// Node.js tests relevant to zlib
//
// - [ ] test-zlib-brotli-16GB.js
Expand Down Expand Up @@ -729,7 +788,7 @@ export const closeAfterError = {
// - [ ] test-zlib-from-concatenated-gzip.js
// - [ ] test-zlib-not-string-or-buffer.js
// - [ ] test-zlib-write-after-end.js
// - [ ] test-zlib-bytes-read.js
// - [x] test-zlib-bytes-read.js
// - [ ] test-zlib-destroy-pipe.js
// - [ ] test-zlib-from-gzip.js
// - [ ] test-zlib-object-write.js
Expand Down
Loading

0 comments on commit f29bf49

Please sign in to comment.