Skip to content

Commit 572a0ba

Browse files
authored
fix: replace @eggjs/yauzl with upstream yauzl 3.4.0, update yazl to 3.3.1 (#145)
Fixes the Node 26 CI failure on master, and moves both zip dependencies to their upstream, maintained versions. ## yauzl: the Node 26 fix `@eggjs/yauzl` depends on `fd-slicer2`, whose `ReadStream` loses data when piped on Node 26. Any zip entry over the 64 KiB `highWaterMark` delivers roughly the first chunk and then stalls, with no `end`, no `error`, no `close`. That is why `zip.uncompress()` hangs until the 60s timeout on Node 26 while passing on 18 through 24. Not our code: released 2.1.1 reproduces it identically. Reported upstream at node-modules/yauzl#3. `yauzl@3.4.0` dropped `fd-slicer` entirely (only dependency is now `pend`) and does not have the bug. The fork was adopted for `decodeStrings: false` so absolute paths survive `validateFileName`. I checked that still holds against the `contain-absolute-path.zip` fixture rather than assuming: | | @eggjs/yauzl 2.11.0 | upstream 3.4.0 | | --- | --- | --- | | entries | 31 | 31 | | `fileName` is Buffer | 31 | 31 | | `externalFileAttributes` present | 31 | 31 | | files read | 21 | 21 | | leading `/` entry | preserved | preserved | Only visible difference: yauzl 3 capitalises the "End of central directory record signature not found" message, so that assertion is now case-insensitive. ## yazl 3 and the early-finalize bug it exposed yazl 3 turns "add entries after calling `end()`" from a tolerated no-op into a thrown error, and compressing trips it immediately. `_onEntryFinish()` finalizes as soon as the entry queue is momentarily empty. For zip the finish callback runs synchronously, so a caller doing: ```js zipStream.addEntry(streamA, ...); zipStream.addEntry(bufferB, ...); ``` closed the archive after the first entry, and the second threw. Tar avoids it only because its `fs.stat` makes the callback async, which lets the later entries queue first. Worth being precise about the old behaviour: **yazl 2 did not drop those entries.** I checked, and the produced archive contained all of them. So this was latent, not a live data-loss bug. Fix is to finalize on the next tick and skip it if an entry arrived meanwhile. Verified the produced archive still contains every entry. Residual limitation, unchanged in spirit from before: entries added after a longer async gap still finalize early. That is the existing drain heuristic, and giving the stream an explicit "done adding" call would be an API change worth doing separately. ## Result **171 passing on both Node 24 and Node 26**, lint and `tsc` clean. On Node 26 the zip suite finishes in ~495ms where it previously hung for 60s. The symlink cases from #140 were re-checked through the new zip path and still block. Drops `fd-slicer2` and `buffer-crc32` from the tree. Supersedes #132.
1 parent 3499eb2 commit 572a0ba

4 files changed

Lines changed: 17 additions & 7 deletions

File tree

lib/tar/stream.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,15 @@ class TarStream extends BaseStream {
127127
const waitingEntry = this._waitingEntries.shift();
128128
if (waitingEntry) {
129129
this.addEntry.apply(this, waitingEntry);
130-
} else {
131-
this._finalize();
130+
return;
132131
}
132+
// A caller adding entries back to back queues the later ones only after this
133+
// returns, and for zip the finish callback runs synchronously, so finalizing
134+
// here would close the archive after the first entry. Give the caller a tick.
135+
setImmediate(() => {
136+
if (this._processing || this._waitingEntries.length > 0) return;
137+
this._finalize();
138+
});
133139
}
134140

135141
_finalize() {

lib/zip/uncompress_stream.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// https://github.com/thejoshwolfe/yauzl#no-streaming-unzip-api
44

55
const debug = require('util').debuglog('compressing/zip/uncompress_stream');
6-
const yauzl = require('@eggjs/yauzl');
6+
const yauzl = require('yauzl');
77
const stream = require('stream');
88
const UncompressBaseStream = require('../base_write_stream');
99
const utils = require('../utils');
@@ -125,7 +125,11 @@ class ZipUncompressStream extends UncompressBaseStream {
125125
const placeholder = new stream.Readable({ read() {} });
126126
debug('directory, header: %j', header);
127127
this.emit('entry', header, placeholder, next);
128-
setImmediate(() => placeholder.emit('end'));
128+
// Push EOF rather than emitting 'end' on a timer: a fabricated event fires
129+
// whether or not the consumer has finished with the entry, so a listener
130+
// that creates the directory asynchronously would see the next entry, a
131+
// file inside that directory, arrive before the directory exists.
132+
placeholder.push(null);
129133
}
130134
})
131135
.on('end', () => this._finalCallback())

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@
3939
},
4040
"homepage": "https://github.com/node-modules/compressing#readme",
4141
"dependencies": {
42-
"@eggjs/yauzl": "^2.11.0",
4342
"flushwritable": "^1.0.0",
4443
"get-ready": "^1.0.0",
4544
"iconv-lite": "^0.7.0",
4645
"streamifier": "^0.1.1",
4746
"tar-stream": "^1.5.2",
48-
"yazl": "^2.4.2"
47+
"yauzl": "^3.4.0",
48+
"yazl": "^3.3.1"
4949
},
5050
"devDependencies": {
5151
"@types/mocha": "10",

test/zip/uncompress_stream.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('test/zip/uncompress_stream.test.js', () => {
5252
const uncompressStream = new compressing.zip.UncompressStream();
5353
await assert.rejects(async () => {
5454
await pipelinePromise(fs.createReadStream(sourceFile), uncompressStream);
55-
}, /end of central directory record signature not found/);
55+
}, /end of central directory record signature not found/i);
5656
});
5757

5858
it('should uncompress according to file path', done => {

0 commit comments

Comments
 (0)