Merge pull request #3150 from github/koesie10/archive-filesystem-yauzl

Switch to `yauzl` in the archive filesystem provider

extensions/ql-vscode/package-lock.json

@@ -44,6 +44,7 @@
         "vscode-languageclient": "^8.0.2",
         "vscode-test-adapter-api": "^1.7.0",
         "vscode-test-adapter-util": "^0.7.0",
+        "yauzl": "^2.10.0",
         "zip-a-folder": "^3.1.3"
       },
       "devDependencies": {
@@ -95,6 +96,7 @@
         "@types/vscode": "^1.82.0",
         "@types/webpack": "^5.28.0",
         "@types/webpack-env": "^1.18.0",
+        "@types/yauzl": "^2.10.3",
         "@typescript-eslint/eslint-plugin": "^6.2.1",
         "@typescript-eslint/parser": "^6.14.0",
         "@vscode/test-electron": "^2.2.0",
@@ -8080,6 +8082,15 @@
       "integrity": "sha512-I4q9QU9MQv4oEOz4tAHJtNz1cwuLxn2F3xcc2iV5WdqLPpUnj30aUuxt1mAxYTG+oe8CZMV/+6rU4S4gRDzqtQ==",
       "dev": true
     },
+    "node_modules/@types/yauzl": {
+      "version": "2.10.3",
+      "resolved": "https://registry.npmjs.org/@types/yauzl/-/yauzl-2.10.3.tgz",
+      "integrity": "sha512-oJoftv0LSuaDZE3Le4DbKX+KS9G36NzOeSap90UIK0yMA/NhKJhqlSGtNDORNRaIbQfzjXDrQa0ytJ6mNRGz/Q==",
+      "dev": true,
+      "dependencies": {
+        "@types/node": "*"
+      }
+    },
     "node_modules/@typescript-eslint/eslint-plugin": {
       "version": "6.13.2",
       "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-6.13.2.tgz",
@@ -15012,7 +15023,6 @@
       "version": "1.1.0",
       "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.1.0.tgz",
       "integrity": "sha512-cE1qsB/VwyQozZ+q1dGxR8LBYNZeofhEdUNGSMbQD3Gw2lAzX9Zb3uIU6Ebc/Fmyjo9AWWfnn0AUCHqtevs/8g==",
-      "dev": true,
       "dependencies": {
         "pend": "~1.2.0"
       }
@@ -24673,8 +24683,7 @@
     "node_modules/pend": {
       "version": "1.2.0",
       "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz",
-      "integrity": "sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==",
-      "dev": true
+      "integrity": "sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg=="
     },
     "node_modules/picocolors": {
       "version": "1.0.0",
@@ -30756,7 +30765,6 @@
       "version": "2.10.0",
       "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz",
       "integrity": "sha512-p4a9I6X6nu6IhoGmBqAcbJy1mlC4j27vEPZX9F4L4/vZT3Lyq1VkFHw/V/PUcB9Buo+DG3iHkT0x3Qya58zc3g==",
-      "dev": true,
       "dependencies": {
         "buffer-crc32": "~0.2.3",
         "fd-slicer": "~1.1.0"

extensions/ql-vscode/package.json

@@ -1946,6 +1946,7 @@
     "vscode-languageclient": "^8.0.2",
     "vscode-test-adapter-api": "^1.7.0",
     "vscode-test-adapter-util": "^0.7.0",
+    "yauzl": "^2.10.0",
     "zip-a-folder": "^3.1.3"
   },
   "devDependencies": {
@@ -1997,6 +1998,7 @@
     "@types/vscode": "^1.82.0",
     "@types/webpack": "^5.28.0",
     "@types/webpack-env": "^1.18.0",
+    "@types/yauzl": "^2.10.3",
     "@typescript-eslint/eslint-plugin": "^6.2.1",
     "@typescript-eslint/parser": "^6.14.0",
     "@vscode/test-electron": "^2.2.0",

extensions/ql-vscode/src/common/unzip.ts

@@ -0,0 +1,84 @@
+import { Entry as ZipEntry, open, Options as ZipOptions, ZipFile } from "yauzl";
+import { Readable } from "stream";
+
+// We can't use promisify because it picks up the wrong overload.
+export function openZip(
+  path: string,
+  options: ZipOptions = {},
+): Promise<ZipFile> {
+  return new Promise((resolve, reject) => {
+    open(path, options, (err, zipFile) => {
+      if (err) {
+        reject(err);
+        return;
+      }
+
+      resolve(zipFile);
+    });
+  });
+}
+
+export function excludeDirectories(entries: ZipEntry[]): ZipEntry[] {
+  return entries.filter((entry) => !/\/$/.test(entry.fileName));
+}
+
+export function readZipEntries(zipFile: ZipFile): Promise<ZipEntry[]> {
+  return new Promise((resolve, reject) => {
+    const files: ZipEntry[] = [];
+
+    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);
+      }
+
+      zipFile.readEntry();
+    });
+
+    zipFile.on("end", () => {
+      resolve(files);
+    });
+
+    zipFile.on("error", (err) => {
+      reject(err);
+    });
+  });
+}
+
+function openZipReadStream(
+  zipFile: ZipFile,
+  entry: ZipEntry,
+): Promise<Readable> {
+  return new Promise((resolve, reject) => {
+    zipFile.openReadStream(entry, (err, readStream) => {
+      if (err) {
+        reject(err);
+        return;
+      }
+
+      resolve(readStream);
+    });
+  });
+}
+
+export async function openZipBuffer(
+  zipFile: ZipFile,
+  entry: ZipEntry,
+): Promise<Buffer> {
+  const readable = await openZipReadStream(zipFile, entry);
+  return new Promise((resolve, reject) => {
+    const chunks: Buffer[] = [];
+    readable.on("data", (chunk) => {
+      chunks.push(chunk);
+    });
+    readable.on("error", (err) => {
+      reject(err);
+    });
+    readable.on("end", () => {
+      resolve(Buffer.concat(chunks));
+    });
+  });
+}

extensions/ql-vscode/src/common/vscode/archive-filesystem-provider.ts

@@ -1,7 +1,13 @@
 import { pathExists } from "fs-extra";
-import * as unzipper from "unzipper";
+import { Entry as ZipEntry, ZipFile } from "yauzl";
 import * as vscode from "vscode";
 import { extLogger } from "../logging/vscode";
+import {
+  excludeDirectories,
+  openZip,
+  openZipBuffer,
+  readZipEntries,
+} from "../unzip";
 
 // All path operations in this file must be on paths *within* the zip
 // archive.
@@ -177,7 +183,8 @@ function ensureDir(map: DirectoryHierarchyMap, dir: string) {
 }
 
 type Archive = {
-  unzipped: unzipper.CentralDirectory;
+  zipFile: ZipFile;
+  entries: ZipEntry[];
   dirMap: DirectoryHierarchyMap;
 };
 
@@ -185,12 +192,22 @@ async function parse_zip(zipPath: string): Promise<Archive> {
   if (!(await pathExists(zipPath))) {
     throw vscode.FileSystemError.FileNotFound(zipPath);
   }
+  const zipFile = await openZip(zipPath, {
+    lazyEntries: true,
+    autoClose: false,
+    strictFileNames: true,
+  });
+
+  const entries = excludeDirectories(await readZipEntries(zipFile));
+
   const archive: Archive = {
-    unzipped: await unzipper.Open.file(zipPath),
+    zipFile,
+    entries,
     dirMap: new Map(),
   };
-  archive.unzipped.files.forEach((f) => {
-    ensureFile(archive.dirMap, path.resolve("/", f.path));
+
+  entries.forEach((f) => {
+    ensureFile(archive.dirMap, path.resolve("/", f.fileName));
   });
   return archive;
 }
@@ -276,22 +293,16 @@ export class ArchiveFileSystemProvider implements vscode.FileSystemProvider {
     // use '/' as path separator throughout
     const reqPath = ref.pathWithinSourceArchive;
 
-    const file = archive.unzipped.files.find((f) => {
-      const absolutePath = path.resolve("/", f.path);
+    const file = archive.entries.find((f) => {
+      const absolutePath = path.resolve("/", f.fileName);
       return (
         absolutePath === reqPath ||
         absolutePath === path.join("/src_archive", reqPath)
       );
     });
     if (file !== undefined) {
-      if (file.type === "File") {
-        return new File(reqPath, await file.buffer());
-      } else {
-        // file.type === 'Directory'
-        // I haven't observed this case in practice. Could it happen
-        // with a zip file that contains empty directories?
-        return new Directory(reqPath);
-      }
+      const buffer = await openZipBuffer(archive.zipFile, file);
+      return new File(reqPath, buffer);
     }
     if (archive.dirMap.has(reqPath)) {
       return new Directory(reqPath);

extensions/ql-vscode/test/vscode-tests/no-workspace/common/vscode/archive-filesystem-provider.test.ts

@@ -47,11 +47,10 @@ describe("archive-filesystem-provider", () => {
       pathWithinSourceArchive: "folder1",
     });
     const files = await archiveProvider.readDirectory(uri);
-    expect(files).toEqual([
-      ["folder2", FileType.Directory],
-      ["textFile.txt", FileType.File],
-      ["textFile2.txt", FileType.File],
-    ]);
+    expect(files).toHaveLength(3);
+    expect(files).toContainEqual(["folder2", FileType.Directory]);
+    expect(files).toContainEqual(["textFile.txt", FileType.File]);
+    expect(files).toContainEqual(["textFile2.txt", FileType.File]);
   });
 
   it("should handle a missing directory", async () => {