Skip to content

Commit 6b75c68

Browse files
prklm10claude
andcommitted
fix(core): replace extract-zip with yauzl@3 unzip to fix Node 26 hang (PER-10062)
Chromium archive extraction hangs forever on Node.js 26 because extract-zip@2.0.1 depends on yauzl@2, which relies on undefined stream destroy() behavior that Node 26 tightened — openReadStream callbacks never fire on deflate entries (thejoshwolfe/yauzl#176, fixed in 3.3.1). Since percy exec launches the discovery browser before running the wrapped command, the hang means customer test suites never start and builds finalize with no_snapshots. Replaces extract-zip with a local unzip module built on yauzl@^3.4.0 preserving prior behavior: file modes, directory entries, symlinks, __MACOSX/ skip, zip-slip guard, absolute target requirement. Verified extraction of chrome-mac.zip is byte/mode/symlink-identical to extract-zip output on Node 14, 22, and 26. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a4b6b8a commit 6b75c68

6 files changed

Lines changed: 285 additions & 63 deletions

File tree

packages/core/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
"busboy": "^1.6.0",
5656
"content-disposition": "^0.5.4",
5757
"cross-spawn": "^7.0.3",
58-
"extract-zip": "^2.0.1",
5958
"fast-glob": "^3.2.11",
6059
"fast-xml-parser": "^4.4.1",
6160
"micromatch": "^4.0.8",
@@ -64,7 +63,8 @@
6463
"path-to-regexp": "^6.3.0",
6564
"rimraf": "^3.0.2",
6665
"ws": "^8.17.1",
67-
"yaml": "^2.4.1"
66+
"yaml": "^2.4.1",
67+
"yauzl": "^3.4.0"
6868
},
6969
"optionalDependencies": {
7070
"@percy/cli-doctor": "1.32.4"

packages/core/src/install.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export function chromium({
146146
// default chromium revision by platform (see chromium.revisions)
147147
revision = selectByPlatform(chromium.revisions)
148148
} = {}) {
149-
let extract = (i, o) => import('extract-zip').then(ex => ex.default(i, { dir: o }));
149+
let extract = (i, o) => import('./unzip.js').then(ex => ex.default(i, { dir: o }));
150150

151151
let url = (process.env.PERCY_CHROMIUM_BASE_URL || 'https://storage.googleapis.com/chromium-browser-snapshots/') +
152152
selectByPlatform({

packages/core/src/unzip.js

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import fs from 'fs';
2+
import path from 'path';
3+
import stream from 'stream';
4+
import { promisify } from 'util';
5+
import yauzl from 'yauzl';
6+
7+
const pipeline = promisify(stream.pipeline);
8+
const openZip = promisify(yauzl.open);
9+
10+
// stat mode constants for entry external file attributes
11+
const IFMT = 0o170000;
12+
const IFDIR = 0o040000;
13+
const IFLNK = 0o120000;
14+
15+
// Returns the file mode of an extracted entry, with defaults matching the
16+
// previous extract-zip behavior when the archive does not provide one
17+
function extractedMode(mode, isDir) {
18+
return (mode || (isDir ? 0o755 : 0o644)) & 0o777;
19+
}
20+
21+
// Collects a readable stream into a utf8 string (symlink targets)
22+
async function readAll(readable) {
23+
let chunks = [];
24+
for await (let chunk of readable) chunks.push(chunk);
25+
return Buffer.concat(chunks).toString('utf8');
26+
}
27+
28+
// Extracts a zip archive into a directory, preserving file modes, directory
29+
// entries, and symlinks. Replaces the unmaintained extract-zip package, whose
30+
// yauzl@2 dependency deadlocks on deflate entries under Node 26
31+
// (https://github.com/thejoshwolfe/yauzl/issues/176).
32+
export default async function unzip(archive, { dir }) {
33+
if (!path.isAbsolute(dir)) {
34+
throw new Error('Target directory is expected to be absolute');
35+
}
36+
37+
await fs.promises.mkdir(dir, { recursive: true });
38+
dir = await fs.promises.realpath(dir);
39+
40+
let zipfile = await openZip(archive, { lazyEntries: true, autoClose: false });
41+
let openReadStream = promisify(zipfile.openReadStream.bind(zipfile));
42+
43+
try {
44+
await new Promise((resolve, reject) => {
45+
let done = false;
46+
let finish = err => {
47+
if (done) return;
48+
done = true;
49+
if (err) reject(err); else resolve();
50+
};
51+
52+
zipfile.on('error', finish);
53+
zipfile.on('end', () => finish());
54+
55+
zipfile.on('entry', async entry => {
56+
try {
57+
// skip macOS resource fork entries
58+
if (entry.fileName.startsWith('__MACOSX/')) {
59+
return zipfile.readEntry();
60+
}
61+
62+
let dest = path.join(dir, entry.fileName);
63+
64+
// guard against zip-slip
65+
if (path.relative(dir, dest).split(path.sep).includes('..')) {
66+
throw new Error(`Out of bound path "${dest}" found while processing file ${entry.fileName}`);
67+
}
68+
69+
let mode = (entry.externalFileAttributes >> 16) & 0xFFFF;
70+
let isSymlink = (mode & IFMT) === IFLNK;
71+
let isDir = (mode & IFMT) === IFDIR ||
72+
entry.fileName.endsWith('/') ||
73+
// directories from some windows archivers lack mode bits
74+
((entry.versionMadeBy >> 8) === 0 && entry.externalFileAttributes === 16);
75+
76+
if (isDir) {
77+
await fs.promises.mkdir(dest, { recursive: true, mode: extractedMode(mode, true) });
78+
} else {
79+
await fs.promises.mkdir(path.dirname(dest), { recursive: true });
80+
let contents = await openReadStream(entry);
81+
82+
if (isSymlink) {
83+
await fs.promises.symlink(await readAll(contents), dest);
84+
} else {
85+
await pipeline(contents, fs.createWriteStream(dest, { mode: extractedMode(mode, false) }));
86+
}
87+
}
88+
89+
zipfile.readEntry();
90+
} catch (error) {
91+
finish(error);
92+
}
93+
});
94+
95+
zipfile.readEntry();
96+
});
97+
} finally {
98+
zipfile.close();
99+
}
100+
}

packages/core/test/unit/install.test.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ describe('Unit / Install', () => {
124124
beforeEach(async () => {
125125
dl = await mockRequests('https://storage.googleapis.com');
126126

127-
// stub extract-zip using esm loader mocks
127+
// stub the unzip module using esm loader mocks
128128
extractZip = jasmine.createSpy('exports').and.resolveTo();
129-
global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip });
129+
global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip });
130130

131131
// make getters for jasmine property spy
132132
let { platform, arch } = process;
@@ -210,9 +210,9 @@ describe('Unit / Install', () => {
210210
process.env.PERCY_CHROMIUM_BASE_URL = baseUrl;
211211
dl = await mockRequests('https://storage.test.com');
212212

213-
// stub extract-zip using esm loader mocks
213+
// stub the unzip module using esm loader mocks
214214
extractZip = jasmine.createSpy('exports').and.resolveTo();
215-
global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip });
215+
global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip });
216216

217217
// make getters for jasmine property spy
218218
let { platform, arch } = process;
@@ -415,9 +415,9 @@ describe('Unit / Install in executable', () => {
415415
beforeEach(async () => {
416416
dl = await mockRequests('https://storage.googleapis.com');
417417

418-
// stub extract-zip using esm loader mocks
418+
// stub the unzip module using esm loader mocks
419419
extractZip = jasmine.createSpy('exports').and.resolveTo();
420-
global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip });
420+
global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip });
421421

422422
// make getters for jasmine property spy
423423
let { platform, arch } = process;
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import fs from 'fs';
2+
import os from 'os';
3+
import path from 'path';
4+
import zlib from 'zlib';
5+
import unzip from '../../src/unzip.js';
6+
7+
// minimal crc32 used to craft valid zip fixtures (zlib.crc32 requires node 20+)
8+
function crc32(buf) {
9+
let crc = ~0;
10+
11+
for (let byte of buf) {
12+
crc ^= byte;
13+
for (let i = 0; i < 8; i++) {
14+
crc = (crc >>> 1) ^ (0xEDB88320 & -(crc & 1));
15+
}
16+
}
17+
18+
return ~crc >>> 0;
19+
}
20+
21+
// builds a zip buffer from entry descriptors: { name, data, mode, deflate }
22+
function makeZip(entries) {
23+
let locals = [];
24+
let centrals = [];
25+
let offset = 0;
26+
27+
for (let { name, data = '', mode = 0o644, deflate = false } of entries) {
28+
let nameBuf = Buffer.from(name);
29+
let contents = Buffer.from(data);
30+
let crc = crc32(contents);
31+
let stored = deflate ? zlib.deflateRawSync(contents) : contents;
32+
let method = deflate ? 8 : 0;
33+
34+
let local = Buffer.alloc(30);
35+
local.writeUInt32LE(0x04034b50, 0);
36+
local.writeUInt16LE(20, 4); // version needed
37+
local.writeUInt16LE(method, 8);
38+
local.writeUInt32LE(crc, 14);
39+
local.writeUInt32LE(stored.length, 18);
40+
local.writeUInt32LE(contents.length, 22);
41+
local.writeUInt16LE(nameBuf.length, 26);
42+
locals.push(local, nameBuf, stored);
43+
44+
let central = Buffer.alloc(46);
45+
central.writeUInt32LE(0x02014b50, 0);
46+
central.writeUInt16LE((3 << 8) | 20, 4); // version made by unix
47+
central.writeUInt16LE(20, 6); // version needed
48+
central.writeUInt16LE(method, 10);
49+
central.writeUInt32LE(crc, 16);
50+
central.writeUInt32LE(stored.length, 20);
51+
central.writeUInt32LE(contents.length, 24);
52+
central.writeUInt16LE(nameBuf.length, 28);
53+
central.writeUInt32LE((mode << 16) >>> 0, 38); // external attributes
54+
central.writeUInt32LE(offset, 42);
55+
centrals.push(central, nameBuf);
56+
57+
offset += local.length + nameBuf.length + stored.length;
58+
}
59+
60+
let cd = Buffer.concat(centrals);
61+
let eocd = Buffer.alloc(22);
62+
eocd.writeUInt32LE(0x06054b50, 0);
63+
eocd.writeUInt16LE(entries.length, 8);
64+
eocd.writeUInt16LE(entries.length, 10);
65+
eocd.writeUInt32LE(cd.length, 12);
66+
eocd.writeUInt32LE(offset, 16);
67+
68+
return Buffer.concat([...locals, cd, eocd]);
69+
}
70+
71+
describe('Unit / Unzip', () => {
72+
let tmp, archive;
73+
74+
beforeEach(() => {
75+
tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'percy-unzip-'));
76+
archive = path.join(tmp, 'fixture.zip');
77+
});
78+
79+
afterEach(() => {
80+
fs.rmSync(tmp, { recursive: true, force: true });
81+
});
82+
83+
it('extracts stored and deflated entries', async () => {
84+
let big = 'percy'.repeat(100_000);
85+
86+
fs.writeFileSync(archive, makeZip([
87+
{ name: 'dir/', mode: 0o40755 },
88+
{ name: 'dir/stored.txt', data: 'stored contents' },
89+
{ name: 'dir/deflated.txt', data: big, deflate: true }
90+
]));
91+
92+
await unzip(archive, { dir: path.join(tmp, 'out') });
93+
94+
expect(fs.readFileSync(path.join(tmp, 'out/dir/stored.txt'), 'utf8')).toBe('stored contents');
95+
expect(fs.readFileSync(path.join(tmp, 'out/dir/deflated.txt'), 'utf8')).toBe(big);
96+
});
97+
98+
it('creates intermediate directories for nested entries', async () => {
99+
fs.writeFileSync(archive, makeZip([
100+
{ name: 'a/b/c/file.txt', data: 'nested' }
101+
]));
102+
103+
await unzip(archive, { dir: path.join(tmp, 'out') });
104+
105+
expect(fs.readFileSync(path.join(tmp, 'out/a/b/c/file.txt'), 'utf8')).toBe('nested');
106+
});
107+
108+
it('skips macOS resource fork entries', async () => {
109+
fs.writeFileSync(archive, makeZip([
110+
{ name: 'file.txt', data: 'real' },
111+
{ name: '__MACOSX/file.txt', data: 'fork' }
112+
]));
113+
114+
await unzip(archive, { dir: path.join(tmp, 'out') });
115+
116+
expect(fs.existsSync(path.join(tmp, 'out/file.txt'))).toBe(true);
117+
expect(fs.existsSync(path.join(tmp, 'out/__MACOSX'))).toBe(false);
118+
});
119+
120+
it('rejects entries that would escape the target directory', async () => {
121+
fs.writeFileSync(archive, makeZip([
122+
{ name: '../evil.txt', data: 'oops' }
123+
]));
124+
125+
// yauzl 3 rejects relative paths at parse time; the unzip module also
126+
// guards against zip-slip for entries yauzl allows through
127+
await expectAsync(unzip(archive, { dir: path.join(tmp, 'out') }))
128+
.toBeRejectedWithError(/invalid relative path|Out of bound path/);
129+
130+
expect(fs.existsSync(path.join(tmp, 'evil.txt'))).toBe(false);
131+
});
132+
133+
it('rejects relative target directories', async () => {
134+
fs.writeFileSync(archive, makeZip([
135+
{ name: 'file.txt', data: 'contents' }
136+
]));
137+
138+
await expectAsync(unzip(archive, { dir: 'relative/out' }))
139+
.toBeRejectedWithError('Target directory is expected to be absolute');
140+
});
141+
142+
if (process.platform !== 'win32') {
143+
it('preserves executable file modes', async () => {
144+
fs.writeFileSync(archive, makeZip([
145+
{ name: 'bin/exec.sh', data: '#!/bin/sh\n', mode: 0o100755 },
146+
{ name: 'bin/plain.txt', data: 'plain', mode: 0o100644 }
147+
]));
148+
149+
await unzip(archive, { dir: path.join(tmp, 'out') });
150+
151+
expect(fs.statSync(path.join(tmp, 'out/bin/exec.sh')).mode & 0o777).toBe(0o755);
152+
expect(fs.statSync(path.join(tmp, 'out/bin/plain.txt')).mode & 0o777).toBe(0o644);
153+
});
154+
155+
it('restores symlink entries', async () => {
156+
fs.writeFileSync(archive, makeZip([
157+
{ name: 'target.txt', data: 'linked contents' },
158+
{ name: 'link.txt', data: 'target.txt', mode: 0o120755 }
159+
]));
160+
161+
await unzip(archive, { dir: path.join(tmp, 'out') });
162+
163+
expect(fs.lstatSync(path.join(tmp, 'out/link.txt')).isSymbolicLink()).toBe(true);
164+
expect(fs.readlinkSync(path.join(tmp, 'out/link.txt'))).toBe('target.txt');
165+
expect(fs.readFileSync(path.join(tmp, 'out/link.txt'), 'utf8')).toBe('linked contents');
166+
});
167+
}
168+
});

0 commit comments

Comments
 (0)