From 2e6eb1c7bcd3340d03045627a93b6a9009305c27 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 13:19:48 +0100 Subject: [PATCH 1/7] Add simple tests for yauzl-based unzip functions --- extensions/ql-vscode/src/common/unzip.ts | 7 +- .../test/unit-tests/common/unzip.test.ts | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+), 6 deletions(-) create mode 100644 extensions/ql-vscode/test/unit-tests/common/unzip.test.ts diff --git a/extensions/ql-vscode/src/common/unzip.ts b/extensions/ql-vscode/src/common/unzip.ts index acd24510f..77052fe52 100644 --- a/extensions/ql-vscode/src/common/unzip.ts +++ b/extensions/ql-vscode/src/common/unzip.ts @@ -28,12 +28,7 @@ export function readZipEntries(zipFile: ZipFile): Promise { zipFile.readEntry(); zipFile.on("entry", (entry: ZipEntry) => { - if (/\/$/.test(entry.fileName)) { - // Directory file names end with '/' - // We don't need to do anything for directories. - } else { - files.push(entry); - } + files.push(entry); zipFile.readEntry(); }); diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts new file mode 100644 index 000000000..931517fac --- /dev/null +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -0,0 +1,113 @@ +import { resolve } from "path"; +import { + excludeDirectories, + openZip, + openZipBuffer, + readZipEntries, +} from "../../../src/common/unzip"; + +const zipWithSingleFilePath = resolve( + __dirname, + "../../vscode-tests/no-workspace/data/archive-filesystem-provider-test/single_file.zip", +); +const zipWithFolderPath = resolve( + __dirname, + "../../vscode-tests/no-workspace/data/archive-filesystem-provider-test/zip_with_folder.zip", +); + +describe("openZip", () => { + it("can open a zip file", async () => { + const zipFile = await openZip(zipWithFolderPath, { + lazyEntries: false, + }); + + expect(zipFile.entryCount).toEqual(8); + }); +}); + +describe("readZipEntries", () => { + it("can read the entries when there is a single file", async () => { + const zipFile = await openZip(zipWithSingleFilePath, { + lazyEntries: true, + }); + const entries = await readZipEntries(zipFile); + + expect(entries.map((entry) => entry.fileName).sort()).toEqual([ + "src_archive/", + "src_archive/aFileName.txt", + ]); + }); + + it("can read the entries when there are multiple folders", async () => { + const zipFile = await openZip(zipWithFolderPath, { + lazyEntries: true, + }); + const entries = await readZipEntries(zipFile); + + expect(entries.map((entry) => entry.fileName).sort()).toEqual([ + "__MACOSX/._folder1", + "__MACOSX/folder1/._textFile.txt", + "__MACOSX/folder1/._textFile2.txt", + "folder1/", + "folder1/folder2/", + "folder1/folder2/textFile3.txt", + "folder1/textFile.txt", + "folder1/textFile2.txt", + ]); + }); +}); + +describe("excludeDirectories", () => { + it("excludes directories when there is a single file", async () => { + const zipFile = await openZip(zipWithSingleFilePath, { + lazyEntries: true, + }); + const entries = await readZipEntries(zipFile); + const entriesWithoutDirectories = excludeDirectories(entries); + + expect( + entriesWithoutDirectories.map((entry) => entry.fileName).sort(), + ).toEqual(["src_archive/aFileName.txt"]); + }); + + it("excludes directories when there are multiple folders", async () => { + const zipFile = await openZip(zipWithFolderPath, { + lazyEntries: true, + }); + const entries = await readZipEntries(zipFile); + const entriesWithoutDirectories = excludeDirectories(entries); + + expect( + entriesWithoutDirectories.map((entry) => entry.fileName).sort(), + ).toEqual([ + "__MACOSX/._folder1", + "__MACOSX/folder1/._textFile.txt", + "__MACOSX/folder1/._textFile2.txt", + "folder1/folder2/textFile3.txt", + "folder1/textFile.txt", + "folder1/textFile2.txt", + ]); + }); +}); + +describe("openZipBuffer", () => { + it("can read an entry in the zip file", async () => { + const zipFile = await openZip(zipWithFolderPath, { + lazyEntries: true, + autoClose: false, + }); + const entries = await readZipEntries(zipFile); + + const entry = entries.find( + (entry) => entry.fileName === "folder1/textFile.txt", + ); + expect(entry).toBeDefined(); + if (!entry) { + return; + } + + const buffer = await openZipBuffer(zipFile, entry); + expect(buffer).toHaveLength(12); + expect(buffer.toString("utf8")).toEqual("I am a text\n"); + }); +}); From 1055515d59b349f05b48c558d58b3c004f182078 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 14:43:49 +0100 Subject: [PATCH 2/7] Use different ZIP file for unzip tests --- .../test/unit-tests/common/unzip.test.ts | 82 ++++++------------ .../test/unit-tests/data/unzip/test-zip.zip | Bin 0 -> 1339 bytes 2 files changed, 26 insertions(+), 56 deletions(-) create mode 100644 extensions/ql-vscode/test/unit-tests/data/unzip/test-zip.zip diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index 931517fac..265e32209 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -6,72 +6,44 @@ import { readZipEntries, } from "../../../src/common/unzip"; -const zipWithSingleFilePath = resolve( - __dirname, - "../../vscode-tests/no-workspace/data/archive-filesystem-provider-test/single_file.zip", -); -const zipWithFolderPath = resolve( - __dirname, - "../../vscode-tests/no-workspace/data/archive-filesystem-provider-test/zip_with_folder.zip", -); +const zipPath = resolve(__dirname, "../data/unzip/test-zip.zip"); describe("openZip", () => { it("can open a zip file", async () => { - const zipFile = await openZip(zipWithFolderPath, { + const zipFile = await openZip(zipPath, { lazyEntries: false, }); - expect(zipFile.entryCount).toEqual(8); + expect(zipFile.entryCount).toEqual(11); }); }); describe("readZipEntries", () => { - it("can read the entries when there is a single file", async () => { - const zipFile = await openZip(zipWithSingleFilePath, { + it("can read the entries when there are multiple directories", async () => { + const zipFile = await openZip(zipPath, { lazyEntries: true, }); const entries = await readZipEntries(zipFile); expect(entries.map((entry) => entry.fileName).sort()).toEqual([ - "src_archive/", - "src_archive/aFileName.txt", - ]); - }); - - it("can read the entries when there are multiple folders", async () => { - const zipFile = await openZip(zipWithFolderPath, { - lazyEntries: true, - }); - const entries = await readZipEntries(zipFile); - - expect(entries.map((entry) => entry.fileName).sort()).toEqual([ - "__MACOSX/._folder1", - "__MACOSX/folder1/._textFile.txt", - "__MACOSX/folder1/._textFile2.txt", - "folder1/", - "folder1/folder2/", - "folder1/folder2/textFile3.txt", - "folder1/textFile.txt", - "folder1/textFile2.txt", + "directory/", + "directory/file.txt", + "directory/file2.txt", + "directory2/", + "directory2/file.txt", + "empty-directory/", + "tools/", + "tools/osx64/", + "tools/osx64/java-aarch64/", + "tools/osx64/java-aarch64/bin/", + "tools/osx64/java-aarch64/bin/java", ]); }); }); describe("excludeDirectories", () => { - it("excludes directories when there is a single file", async () => { - const zipFile = await openZip(zipWithSingleFilePath, { - lazyEntries: true, - }); - const entries = await readZipEntries(zipFile); - const entriesWithoutDirectories = excludeDirectories(entries); - - expect( - entriesWithoutDirectories.map((entry) => entry.fileName).sort(), - ).toEqual(["src_archive/aFileName.txt"]); - }); - - it("excludes directories when there are multiple folders", async () => { - const zipFile = await openZip(zipWithFolderPath, { + it("excludes directories", async () => { + const zipFile = await openZip(zipPath, { lazyEntries: true, }); const entries = await readZipEntries(zipFile); @@ -80,26 +52,24 @@ describe("excludeDirectories", () => { expect( entriesWithoutDirectories.map((entry) => entry.fileName).sort(), ).toEqual([ - "__MACOSX/._folder1", - "__MACOSX/folder1/._textFile.txt", - "__MACOSX/folder1/._textFile2.txt", - "folder1/folder2/textFile3.txt", - "folder1/textFile.txt", - "folder1/textFile2.txt", + "directory/file.txt", + "directory/file2.txt", + "directory2/file.txt", + "tools/osx64/java-aarch64/bin/java", ]); }); }); describe("openZipBuffer", () => { it("can read an entry in the zip file", async () => { - const zipFile = await openZip(zipWithFolderPath, { + const zipFile = await openZip(zipPath, { lazyEntries: true, autoClose: false, }); const entries = await readZipEntries(zipFile); const entry = entries.find( - (entry) => entry.fileName === "folder1/textFile.txt", + (entry) => entry.fileName === "directory/file.txt", ); expect(entry).toBeDefined(); if (!entry) { @@ -107,7 +77,7 @@ describe("openZipBuffer", () => { } const buffer = await openZipBuffer(zipFile, entry); - expect(buffer).toHaveLength(12); - expect(buffer.toString("utf8")).toEqual("I am a text\n"); + expect(buffer).toHaveLength(13); + expect(buffer.toString("utf8")).toEqual("I am a file\n\n"); }); }); diff --git a/extensions/ql-vscode/test/unit-tests/data/unzip/test-zip.zip b/extensions/ql-vscode/test/unit-tests/data/unzip/test-zip.zip new file mode 100644 index 0000000000000000000000000000000000000000..aea49e203d22ffc2a1631dc33cba74206415a349 GIT binary patch literal 1339 zcmWIWW@h1H0D%uBlf%IbC;<{p$t+4uF3B&d)DJ*WH-DnKu`o~`h7s!0GILUm^hzp9 zJQWgi6%zCEOEOZ66hNX}TnIDX0qtr@@><3VGy{Z%Q0xM!Mi_yi`4^f)xsf#+A-NQ! z@W7#JS011O5C*vx?ouOt6l;o8lZ#SIkev7e%@zTm=G5GRl1g2;5lB`&KvT^IR9%vv zpHqxS4ai9lHTlIAW+r%4N}{REN-Rs%O-w9G&Op)j1kDk$__QTu=7DV#0V?8P;47UR z?t0W~ttODC4aACsw1S)ve9iyVnaloOI_Dld3JT*mc|s>lSie zVIr)@mNXH%ZGpOB2^F^(vl~q(T0%w`gDoi|j5&nQ7)Zhf n8HGD(gNy>2!SJ^6FCn8qP7Ls71!ZUkZXi6s1PpE Date: Tue, 19 Dec 2023 14:58:16 +0100 Subject: [PATCH 3/7] Add tests for unzipToDirectory --- extensions/ql-vscode/src/common/files.ts | 7 +- .../test/unit-tests/common/unzip.test.ts | 119 +++++++++++++++++- 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/common/files.ts b/extensions/ql-vscode/src/common/files.ts index e59e85e4a..93658ad1d 100644 --- a/extensions/ql-vscode/src/common/files.ts +++ b/extensions/ql-vscode/src/common/files.ts @@ -91,18 +91,23 @@ export async function readDirFullPaths(path: string): Promise { * Symbolic links are ignored. * * @param dir the directory to walk + * @param includeDirectories whether to include directories in the results * * @return An iterator of the full path to all files recursively found in the directory. */ export async function* walkDirectory( dir: string, + includeDirectories = false, ): AsyncIterableIterator { const seenFiles = new Set(); for await (const d of await opendir(dir)) { const entry = join(dir, d.name); seenFiles.add(entry); if (d.isDirectory()) { - yield* walkDirectory(entry); + if (includeDirectories) { + yield entry; + } + yield* walkDirectory(entry, includeDirectories); } else if (d.isFile()) { yield entry; } diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index 265e32209..f511422b1 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -1,10 +1,21 @@ -import { resolve } from "path"; +import { createHash } from "crypto"; +import { join, relative, resolve } from "path"; +import { + createReadStream, + pathExists, + readdir, + readFile, + stat, +} from "fs-extra"; +import { dir, DirectoryResult } from "tmp-promise"; import { excludeDirectories, openZip, openZipBuffer, readZipEntries, + unzipToDirectory, } from "../../../src/common/unzip"; +import { walkDirectory } from "../../../src/common/files"; const zipPath = resolve(__dirname, "../data/unzip/test-zip.zip"); @@ -81,3 +92,109 @@ describe("openZipBuffer", () => { expect(buffer.toString("utf8")).toEqual("I am a file\n\n"); }); }); + +describe("unzipToDirectory", () => { + let tmpDir: DirectoryResult; + + beforeEach(async () => { + tmpDir = await dir({ + unsafeCleanup: true, + }); + }); + + afterEach(async () => { + await tmpDir?.cleanup(); + }); + + it("extracts all files", async () => { + await unzipToDirectory(zipPath, tmpDir.path); + + const allFiles = []; + for await (const file of walkDirectory(tmpDir.path, true)) { + allFiles.push(file); + } + + expect( + allFiles.map((filePath) => relative(tmpDir.path, filePath)).sort(), + ).toEqual([ + "directory", + "directory/file.txt", + "directory/file2.txt", + "directory2", + "directory2/file.txt", + "empty-directory", + "tools", + "tools/osx64", + "tools/osx64/java-aarch64", + "tools/osx64/java-aarch64/bin", + "tools/osx64/java-aarch64/bin/java", + ]); + + await expectFile(join(tmpDir.path, "directory", "file.txt"), { + mode: 0o100644, + contents: "I am a file\n\n", + }); + await expectFile(join(tmpDir.path, "directory", "file2.txt"), { + mode: 0o100644, + contents: "I am another file\n\n", + }); + await expectFile(join(tmpDir.path, "directory2", "file.txt"), { + mode: 0o100600, + contents: "I am secret\n", + }); + await expectFile( + join(tmpDir.path, "tools", "osx64", "java-aarch64", "bin", "java"), + { + mode: 0o100755, + hash: "68b832b5c0397c5baddbbb0a76cf5407b7ea5eee8f84f9ab9488f04a52e529eb", + }, + ); + + expect(await pathExists(join(tmpDir.path, "empty-directory"))).toBe(true); + expect(await readdir(join(tmpDir.path, "empty-directory"))).toEqual([]); + }); +}); + +async function expectFile( + filePath: string, + { + mode: expectedMode, + hash: expectedHash, + contents: expectedContents, + }: { + mode: number; + hash?: string; + contents?: string; + }, +) { + const stats = await stat(filePath); + expect(stats.mode).toEqual(expectedMode); + + if (expectedHash) { + const hash = await computeHash(filePath); + expect(hash).toEqual(expectedHash); + } + + if (expectedContents) { + const contents = await readFile(filePath); + expect(contents.toString("utf-8")).toEqual(expectedContents); + } +} + +async function computeHash(filePath: string) { + const input = createReadStream(filePath); + const hash = createHash("sha256"); + + input.pipe(hash); + + await new Promise((resolve, reject) => { + input.on("error", (err) => { + reject(err); + }); + input.on("end", () => { + resolve(undefined); + }); + }); + + return hash.digest("hex"); +} From f45b7905910309e6059eb4f5363d205804adc5b3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 15:14:13 +0100 Subject: [PATCH 4/7] Fix file path in tests on Windows --- extensions/ql-vscode/test/unit-tests/common/unzip.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index f511422b1..ee09e0f4e 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -1,5 +1,5 @@ import { createHash } from "crypto"; -import { join, relative, resolve } from "path"; +import { join, relative, resolve, sep } from "path"; import { createReadStream, pathExists, @@ -115,7 +115,9 @@ describe("unzipToDirectory", () => { } expect( - allFiles.map((filePath) => relative(tmpDir.path, filePath)).sort(), + allFiles + .map((filePath) => relative(tmpDir.path, filePath).split(sep).join("/")) + .sort(), ).toEqual([ "directory", "directory/file.txt", From a80098d7dffa5043522567a3200efce886f6682b Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 15:17:57 +0100 Subject: [PATCH 5/7] Fix race condition in unzip tests --- .../test/unit-tests/common/unzip.test.ts | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index ee09e0f4e..aea638ac1 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -1,12 +1,7 @@ import { createHash } from "crypto"; +import { open } from "fs/promises"; import { join, relative, resolve, sep } from "path"; -import { - createReadStream, - pathExists, - readdir, - readFile, - stat, -} from "fs-extra"; +import { pathExists, readdir } from "fs-extra"; import { dir, DirectoryResult } from "tmp-promise"; import { excludeDirectories, @@ -169,34 +164,26 @@ async function expectFile( contents?: string; }, ) { - const stats = await stat(filePath); + const file = await open(filePath, "r"); + + const stats = await file.stat(); expect(stats.mode).toEqual(expectedMode); + const contents = await file.readFile(); + if (expectedHash) { - const hash = await computeHash(filePath); + const hash = await computeHash(contents); expect(hash).toEqual(expectedHash); } if (expectedContents) { - const contents = await readFile(filePath); expect(contents.toString("utf-8")).toEqual(expectedContents); } } -async function computeHash(filePath: string) { - const input = createReadStream(filePath); +async function computeHash(contents: Buffer) { const hash = createHash("sha256"); - - input.pipe(hash); - - await new Promise((resolve, reject) => { - input.on("error", (err) => { - reject(err); - }); - input.on("end", () => { - resolve(undefined); - }); - }); + hash.update(contents); return hash.digest("hex"); } From be4059864ec17061a36f8d230f1865ca13fa438f Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 15:18:35 +0100 Subject: [PATCH 6/7] Do not test file modes on Windows --- extensions/ql-vscode/test/unit-tests/common/unzip.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index aea638ac1..5a2d91109 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -166,8 +166,11 @@ async function expectFile( ) { const file = await open(filePath, "r"); - const stats = await file.stat(); - expect(stats.mode).toEqual(expectedMode); + // Windows doesn't really support file modes + if (process.platform !== "win32") { + const stats = await file.stat(); + expect(stats.mode).toEqual(expectedMode); + } const contents = await file.readFile(); From f1fe4a20f2de11833fe67c964913482f1813b6d5 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 19 Dec 2023 15:27:07 +0100 Subject: [PATCH 7/7] `chmod` tmp dir before cleanup This should fix any permissions errors we get due to the ZIP file containing files with different permissions. --- extensions/ql-vscode/test/unit-tests/common/unzip.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts index 5a2d91109..1c8feab1a 100644 --- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts +++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts @@ -1,7 +1,7 @@ import { createHash } from "crypto"; import { open } from "fs/promises"; import { join, relative, resolve, sep } from "path"; -import { pathExists, readdir } from "fs-extra"; +import { chmod, pathExists, readdir } from "fs-extra"; import { dir, DirectoryResult } from "tmp-promise"; import { excludeDirectories, @@ -98,6 +98,10 @@ describe("unzipToDirectory", () => { }); afterEach(async () => { + for await (const file of walkDirectory(tmpDir.path)) { + await chmod(file, 0o777); + } + await tmpDir?.cleanup(); });