Skip to content

Commit

Permalink
ref: fix unzipping larger files in node 18+ (#524)
Browse files Browse the repository at this point in the history
* ref: add regression test for unzipping larger file in node 18+

* switch from unzipper to extract-zip
  • Loading branch information
asottile-sentry committed Mar 14, 2024
1 parent 798f07c commit b22cfa5
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 140 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"@types/async": "^3.0.1",
"@types/aws4": "^1.5.1",
"@types/cli-table": "^0.3.0",
"@types/extract-zip": "^2.0.1",
"@types/git-url-parse": "^9.0.0",
"@types/is-ci": "^2.0.0",
"@types/jest": "^29.5.2",
Expand All @@ -45,7 +46,6 @@
"@types/shell-quote": "^1.6.0",
"@types/tar": "^4.0.0",
"@types/tmp": "^0.0.33",
"@types/unzipper": "^0.10.3",
"@types/yargs": "^15.0.3",
"@typescript-eslint/eslint-plugin": "^5.19.0",
"@typescript-eslint/parser": "^5.19.0",
Expand All @@ -59,6 +59,7 @@
"eslint": "^7.2.0",
"eslint-config-prettier": "^6.11.0",
"eslint-formatter-github-annotations": "^0.1.0",
"extract-zip": "^2.0.1",
"fast-xml-parser": "^4.2.4",
"git-url-parse": "^11.4.4",
"is-ci": "^2.0.0",
Expand All @@ -83,7 +84,6 @@
"tmp": "0.1.0",
"ts-jest": "^29.1.1",
"typescript": "^5.1.6",
"unzipper": "0.10.11",
"yargs": "15.4.1"
},
"scripts": {
Expand Down
26 changes: 25 additions & 1 deletion src/utils/__tests__/system.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import * as fs from 'fs';

import { logger } from '../../logger';
import { withTempFile } from '../files';
import { withTempDir, withTempFile } from '../files';

import {
calculateChecksum,
extractZipArchive,
hasExecutable,
HashAlgorithm,
HashOutputFormat,
Expand Down Expand Up @@ -174,3 +175,26 @@ describe('isExecutableInPath', () => {
expect(hasExecutable('./bin/non-existing-binary')).toBe(false);
});
});

describe('extractZipArchive', () => {
// zip with `t.txt` and `5000` iterations of `f'{string.ascii_letters}\n'}`
test('it can extract a larger zip', async () => {
await withTempDir(async tmpdir => {
const zip = `${tmpdir}/out.zip`;

const zipf = await fs.promises.open(zip, 'w');
await zipf.writeFile(Buffer.from([80, 75, 3, 4, 10, 0, 0, 0, 0, 0, 99, 150, 109, 88, 220, 199, 60, 159, 40, 11, 4, 0, 40, 11, 4, 0, 5, 0, 28, 0, 116, 46, 116, 120, 116, 85, 84, 9, 0, 3, 153, 245, 241, 101, 140, 245, 241, 101, 117, 120, 11, 0, 1, 4, 0, 0, 0, 0, 4, 0, 0, 0, 0]));
for (let i = 0; i < 5000; i += 1) {
await zipf.writeFile('abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ\n');
}
await zipf.writeFile(Buffer.from([80, 75, 1, 2, 30, 3, 10, 0, 0, 0, 0, 0, 99, 150, 109, 88, 220, 199, 60, 159, 40, 11, 4, 0, 40, 11, 4, 0, 5, 0, 24, 0, 0, 0, 0, 0, 0, 0, 0, 0, 164, 129, 0, 0, 0, 0, 116, 46, 116, 120, 116, 85, 84, 5, 0, 3, 153, 245, 241, 101, 117, 120, 11, 0, 1, 4, 0, 0, 0, 0, 4, 0, 0, 0, 0, 80, 75, 5, 6, 0, 0, 0, 0, 1, 0, 1, 0, 75, 0, 0, 0, 103, 11, 4, 0, 0, 0]));
await zipf.close();

await extractZipArchive(zip, `${tmpdir}/out`);

// should not have corrupted our file
const checksum = await calculateChecksum(`${tmpdir}/out/t.txt`);
expect(checksum).toBe('7687e11d941faf48d4cf1692c2473a599ad0d7030e1e5c639a31b2f59cd646ba');
});
});
});
8 changes: 2 additions & 6 deletions src/utils/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as fs from 'fs';
import * as path from 'path';
import split from 'split';
import tar from 'tar';
import unzipper from 'unzipper';
import extract from 'extract-zip';

import { logger } from '../logger';

Expand Down Expand Up @@ -344,11 +344,7 @@ export async function extractZipArchive(
filePath: string,
dir: string
): Promise<void> {
const archive = await unzipper.Open.file(filePath);
await archive.extract({
path: dir,
concurrency: 2,
});
await extract(filePath, {dir: dir});
}

/**
Expand Down
Loading

0 comments on commit b22cfa5

Please sign in to comment.