Merge remote-tracking branch 'origin/main' into koesie10/variant-analysis-yauzl

Author: koesie10

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

@@ -5,7 +5,10 @@ import { WriteStream } from "fs";
 import { createWriteStream, ensureDir } from "fs-extra";
 
 // We can't use promisify because it picks up the wrong overload.
-function openZip(path: string, options: ZipOptions = {}): Promise<ZipFile> {
+export function openZip(
+  path: string,
+  options: ZipOptions = {},
+): Promise<ZipFile> {
   return new Promise((resolve, reject) => {
     open(path, options, (err, zipFile) => {
       if (err) {
@@ -18,7 +21,11 @@ function openZip(path: string, options: ZipOptions = {}): Promise<ZipFile> {
   });
 }
 
-function readZipEntries(zipFile: ZipFile): Promise<ZipEntry[]> {
+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[] = [];
 
@@ -60,6 +67,25 @@ function openZipReadStream(
   });
 }
 
+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));
+    });
+  });
+}
+
 async function copyStream(
   readable: Readable,
   writeStream: WriteStream,

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,20 +183,31 @@ function ensureDir(map: DirectoryHierarchyMap, dir: string) {
 }
 
 type Archive = {
-  unzipped: unzipper.CentralDirectory;
+  zipFile: ZipFile;
+  entries: ZipEntry[];
   dirMap: DirectoryHierarchyMap;
 };
 
 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/src/databases/database-fetcher.ts

@@ -1,6 +1,5 @@
 import fetch, { Response } from "node-fetch";
 import { zip } from "zip-a-folder";
-import { Open } from "unzipper";
 import { Uri, window, InputBoxOptions } from "vscode";
 import { CodeQLCliServer } from "../codeql-cli/cli";
 import {
@@ -46,7 +45,7 @@ export async function promptImportInternetDatabase(
   databaseManager: DatabaseManager,
   storagePath: string,
   progress: ProgressCallback,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
 ): Promise<DatabaseItem | undefined> {
   const databaseUrl = await window.showInputBox({
     prompt: "Enter URL of zipfile of database to download",
@@ -101,7 +100,7 @@ export async function promptImportGithubDatabase(
   storagePath: string,
   credentials: Credentials | undefined,
   progress: ProgressCallback,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   language?: string,
   makeSelected = true,
   addSourceArchiveFolder = addDatabaseSourceToWorkspace(),
@@ -180,7 +179,7 @@ export async function downloadGitHubDatabase(
   storagePath: string,
   credentials: Credentials | undefined,
   progress: ProgressCallback,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   language?: string,
   makeSelected = true,
   addSourceArchiveFolder = addDatabaseSourceToWorkspace(),
@@ -235,7 +234,7 @@ export async function downloadGitHubDatabaseFromUrl(
   progress: ProgressCallback,
   databaseManager: DatabaseManager,
   storagePath: string,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   makeSelected = true,
   addSourceArchiveFolder = true,
 ): Promise<DatabaseItem | undefined> {
@@ -279,14 +278,15 @@ export async function downloadGitHubDatabaseFromUrl(
  * @param databaseUrl the file url of the archive to import
  * @param databaseManager the DatabaseManager
  * @param storagePath where to store the unzipped database.
+ * @param cli the CodeQL CLI server
  */
 export async function importArchiveDatabase(
   commandManager: AppCommandManager,
   databaseUrl: string,
   databaseManager: DatabaseManager,
   storagePath: string,
   progress: ProgressCallback,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
 ): Promise<DatabaseItem | undefined> {
   try {
     const item = await databaseArchiveFetcher(
@@ -333,6 +333,7 @@ export async function importArchiveDatabase(
  * @param nameOverride a name for the database that overrides the default
  * @param origin the origin of the database
  * @param progress callback to send progress messages to
+ * @param cli the CodeQL CLI server
  * @param makeSelected make the new database selected in the databases panel (default: true)
  * @param addSourceArchiveFolder whether to add a workspace folder containing the source archive to the workspace
  */
@@ -344,7 +345,7 @@ async function databaseArchiveFetcher(
   nameOverride: string | undefined,
   origin: DatabaseOrigin,
   progress: ProgressCallback,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   makeSelected = true,
   addSourceArchiveFolder = addDatabaseSourceToWorkspace(),
 ): Promise<DatabaseItem> {
@@ -443,34 +444,24 @@ function validateUrl(databaseUrl: string) {
 async function readAndUnzip(
   zipUrl: string,
   unzipPath: string,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   progress?: ProgressCallback,
 ) {
-  // TODO: Providing progress as the file is unzipped is currently blocked
-  // on https://github.com/ZJONSSON/node-unzipper/issues/222
   const zipFile = Uri.parse(zipUrl).fsPath;
   progress?.({
     maxStep: 10,
     step: 9,
     message: `Unzipping into ${basename(unzipPath)}`,
   });
-  if (cli) {
-    // Use the `database unbundle` command if the installed cli version supports it
-    await cli.databaseUnbundle(zipFile, unzipPath);
-  } else {
-    // Must get the zip central directory since streaming the
-    // zip contents may not have correct local file headers.
-    // Instead, we can only rely on the central directory.
-    const directory = await Open.file(zipFile);
-    await directory.extract({ path: unzipPath });
-  }
+
+  await cli.databaseUnbundle(zipFile, unzipPath);
 }
 
 async function fetchAndUnzip(
   databaseUrl: string,
   requestHeaders: { [key: string]: string },
   unzipPath: string,
-  cli?: CodeQLCliServer,
+  cli: CodeQLCliServer,
   progress?: ProgressCallback,
 ) {
   // Although it is possible to download and stream directly to an unzipped directory,

extensions/ql-vscode/src/databases/local-databases-ui.ts

@@ -233,7 +233,7 @@ export class DatabaseUI extends DisposableObject {
     private app: App,
     private databaseManager: DatabaseManager,
     languageContext: LanguageContextStore,
-    private readonly queryServer: QueryRunner | undefined,
+    private readonly queryServer: QueryRunner,
     private readonly storagePath: string,
     readonly extensionPath: string,
   ) {
@@ -402,10 +402,7 @@ export class DatabaseUI extends DisposableObject {
         workspace.workspaceFolders[0].uri.fsPath,
         "tutorial-queries",
       );
-      const cli = this.queryServer?.cliServer;
-      if (!cli) {
-        throw new Error("No CLI server found");
-      }
+      const cli = this.queryServer.cliServer;
       await cli.packInstall(tutorialQueriesPath);
     }
   }
@@ -528,7 +525,7 @@ export class DatabaseUI extends DisposableObject {
           this.databaseManager,
           this.storagePath,
           progress,
-          this.queryServer?.cliServer,
+          this.queryServer.cliServer,
         );
       },
       {
@@ -548,7 +545,7 @@ export class DatabaseUI extends DisposableObject {
           this.storagePath,
           credentials,
           progress,
-          this.queryServer?.cliServer,
+          this.queryServer.cliServer,
         );
       },
       {
@@ -704,7 +701,7 @@ export class DatabaseUI extends DisposableObject {
               this.databaseManager,
               this.storagePath,
               progress,
-              this.queryServer?.cliServer,
+              this.queryServer.cliServer,
             );
           } else {
             await this.databaseManager.openDatabase(uri, {
@@ -836,7 +833,7 @@ export class DatabaseUI extends DisposableObject {
             this.databaseManager,
             this.storagePath,
             progress,
-            this.queryServer?.cliServer,
+            this.queryServer.cliServer,
           );
         }
       },

extensions/ql-vscode/src/model-editor/yaml.ts

@@ -181,25 +181,40 @@ function createDataExtensionYamlsByGrouping(
   >,
   createFilename: (method: Method) => string,
 ): Record<string, string> {
-  const methodsByFilename: Record<string, Record<string, ModeledMethod[]>> = {};
+  const actualFilenameByCanonicalFilename: Record<string, string> = {};
+
+  const methodsByCanonicalFilename: Record<
+    string,
+    Record<string, ModeledMethod[]>
+  > = {};
 
   // We only want to generate a yaml file when it's a known external API usage
   // and there are new modeled methods for it. This avoids us overwriting other
   // files that may contain data we don't know about.
   for (const method of methods) {
     if (method.signature in newModeledMethods) {
-      methodsByFilename[createFilename(method)] = {};
+      const filename = createFilename(method);
+      const canonicalFilename = canonicalizeFilename(filename);
+
+      methodsByCanonicalFilename[canonicalFilename] = {};
+      actualFilenameByCanonicalFilename[canonicalFilename] = filename;
     }
   }
 
   // First populate methodsByFilename with any existing modeled methods.
   for (const [filename, methodsBySignature] of Object.entries(
     existingModeledMethods,
   )) {
-    if (filename in methodsByFilename) {
+    const canonicalFilename = canonicalizeFilename(filename);
+
+    if (canonicalFilename in methodsByCanonicalFilename) {
       for (const [signature, methods] of Object.entries(methodsBySignature)) {
-        methodsByFilename[filename][signature] = [...methods];
+        methodsByCanonicalFilename[canonicalFilename][signature] = [...methods];
       }
+
+      // Ensure that if a file exists on disk, we use the same capitalization
+      // as the original file.
+      actualFilenameByCanonicalFilename[canonicalFilename] = filename;
     }
   }
 
@@ -209,19 +224,25 @@ function createDataExtensionYamlsByGrouping(
     const newMethods = newModeledMethods[method.signature];
     if (newMethods) {
       const filename = createFilename(method);
+      const canonicalFilename = canonicalizeFilename(filename);
 
       // Override any existing modeled methods with the new ones.
-      methodsByFilename[filename][method.signature] = [...newMethods];
+      methodsByCanonicalFilename[canonicalFilename][method.signature] = [
+        ...newMethods,
+      ];
     }
   }
 
   const result: Record<string, string> = {};
 
-  for (const [filename, methods] of Object.entries(methodsByFilename)) {
-    result[filename] = createDataExtensionYaml(
-      language,
-      Object.values(methods).flatMap((methods) => methods),
-    );
+  for (const [canonicalFilename, methods] of Object.entries(
+    methodsByCanonicalFilename,
+  )) {
+    result[actualFilenameByCanonicalFilename[canonicalFilename]] =
+      createDataExtensionYaml(
+        language,
+        Object.values(methods).flatMap((methods) => methods),
+      );
   }
 
   return result;
@@ -299,6 +320,13 @@ export function createFilenameForPackage(
   return `${prefix}${packageName}${suffix}.yml`;
 }
 
+function canonicalizeFilename(filename: string) {
+  // We want to canonicalize filenames so that they are always in the same format
+  // for comparison purposes. This is important because we want to avoid overwriting
+  // data extension YAML files on case-insensitive file systems.
+  return filename.toLowerCase();
+}
+
 function validateModelExtensionFile(data: unknown): data is ModelExtensionFile {
   modelExtensionFileSchemaValidate(data);