Merge pull request #272 from NodeSecure/fix-entry-files-analyser

Fix entry files analyser

Author: fraxken

src/EntryFilesAnalyser.js

@@ -1,10 +1,12 @@
 // Import Node.js Dependencies
 import fs from "node:fs/promises";
 import path from "node:path";
+import { fileURLToPath } from "node:url";
 
 // Import Internal Dependencies
 import { AstAnalyser } from "./AstAnalyser.js";
 
+// CONSTANTS
 const kDefaultExtensions = ["js", "cjs", "mjs", "node"];
 
 export class EntryFilesAnalyser {
@@ -16,9 +18,11 @@ export class EntryFilesAnalyser {
   */
   constructor(options = {}) {
     this.astAnalyzer = options.astAnalyzer ?? new AstAnalyser();
-    this.allowedExtensions = options.loadExtensions
+    const rawAllowedExtensions = options.loadExtensions
       ? options.loadExtensions(kDefaultExtensions)
       : kDefaultExtensions;
+
+    this.allowedExtensions = new Set(rawAllowedExtensions);
   }
 
   /**
@@ -36,7 +40,7 @@ export class EntryFilesAnalyser {
   }
 
   async* #analyzeFile(file) {
-    const filePath = file instanceof URL ? file.pathname : file;
+    const filePath = file instanceof URL ? fileURLToPath(file) : file;
     const report = await this.astAnalyzer.analyseFile(file);
 
     yield { url: filePath, ...report };
@@ -45,12 +49,16 @@ export class EntryFilesAnalyser {
       return;
     }
 
-    yield* this.#analyzeDeps(report.dependencies, path.dirname(filePath));
+    yield* this.#analyzeDeps(
+      report.dependencies,
+      path.dirname(filePath)
+    );
   }
 
   async* #analyzeDeps(deps, basePath) {
     for (const [name] of deps) {
       const depPath = await this.#getInternalDepPath(name, basePath);
+
       if (depPath && !this.analyzedDeps.has(depPath)) {
         this.analyzedDeps.add(depPath);
 
@@ -62,20 +70,25 @@ export class EntryFilesAnalyser {
   async #getInternalDepPath(name, basePath) {
     const depPath = path.join(basePath, name);
     const existingExt = path.extname(name);
-    if (existingExt !== "") {
-      if (!this.allowedExtensions.includes(existingExt.slice(1))) {
-        return null;
-      }
 
-      if (await this.#fileExists(depPath)) {
-        return depPath;
+    if (existingExt === "") {
+      for (const ext of this.allowedExtensions) {
+        const depPathWithExt = `${depPath}.${ext}`;
+
+        const fileExist = await this.#fileExists(depPathWithExt);
+        if (fileExist) {
+          return depPathWithExt;
+        }
       }
     }
+    else {
+      if (!this.allowedExtensions.has(existingExt.slice(1))) {
+        return null;
+      }
 
-    for (const ext of this.allowedExtensions) {
-      const depPathWithExt = `${depPath}.${ext}`;
-      if (await this.#fileExists(depPathWithExt)) {
-        return depPathWithExt;
+      const fileExist = await this.#fileExists(depPath);
+      if (fileExist) {
+        return depPath;
       }
     }
 
@@ -84,7 +97,7 @@ export class EntryFilesAnalyser {
 
   async #fileExists(path) {
     try {
-      await fs.access(path, fs.constants.F_OK);
+      await fs.access(path, fs.constants.R_OK);
 
       return true;
     }

test/AstAnalyser.spec.js

@@ -1,11 +1,11 @@
+/* eslint-disable max-nested-callbacks */
 // Import Node.js Dependencies
 import { describe, it } from "node:test";
 import assert from "node:assert";
 import { readFileSync } from "node:fs";
 
 // Import Internal Dependencies
-import { AstAnalyser } from "../src/AstAnalyser.js";
-import { JsSourceParser } from "../src/JsSourceParser.js";
+import { AstAnalyser, JsSourceParser } from "../index.js";
 import { SourceFile } from "../src/SourceFile.js";
 import {
   customProbes,
@@ -255,7 +255,7 @@ describe("AstAnalyser", (t) => {
   });
 
   describe("analyseFile", () => {
-    it("remove the packageName from the dependencies list", async () => {
+    it("remove the packageName from the dependencies list", async() => {
       const result = await getAnalyser().analyseFile(
         new URL("depName.js", FIXTURE_URL),
         { module: false, packageName: "foobar" }
@@ -268,7 +268,7 @@ describe("AstAnalyser", (t) => {
       );
     });
 
-    it("should fail with a parsing error", async () => {
+    it("should fail with a parsing error", async() => {
       const result = await getAnalyser().analyseFile(
         new URL("parsingError.js", FIXTURE_URL),
         { module: false, packageName: "foobar" }
@@ -286,18 +286,18 @@ describe("AstAnalyser", (t) => {
       const url = new URL("depName.js", FIXTURE_URL);
 
       describe("initialize", () => {
-        it("should throw if initialize is not a function", async () => {
+        it("should throw if initialize is not a function", async() => {
           const res = await analyser.analyseFile(
             url, {
-            initialize: "foo"
-          });
+              initialize: "foo"
+            });
 
           assert.strictEqual(res.ok, false);
           assert.strictEqual(res.warnings[0].value, "options.initialize must be a function");
           assert.strictEqual(res.warnings[0].kind, "parsing-error");
         });
 
-        it("should call the initialize function", async (t) => {
+        it("should call the initialize function", async(t) => {
           const initialize = t.mock.fn();
 
           await analyser.analyseFile(url, {
@@ -307,7 +307,7 @@ describe("AstAnalyser", (t) => {
           assert.strictEqual(initialize.mock.callCount(), 1);
         });
 
-        it("should pass the source file as first argument", async (t) => {
+        it("should pass the source file as first argument", async(t) => {
           const initialize = t.mock.fn();
 
           await analyser.analyseFile(url, {
@@ -319,18 +319,18 @@ describe("AstAnalyser", (t) => {
       });
 
       describe("finalize", () => {
-        it("should throw if finalize is not a function", async () => {
+        it("should throw if finalize is not a function", async() => {
           const res = await analyser.analyseFile(
             url, {
-            finalize: "foo"
-          });
+              finalize: "foo"
+            });
 
           assert.strictEqual(res.ok, false);
           assert.strictEqual(res.warnings[0].value, "options.finalize must be a function");
           assert.strictEqual(res.warnings[0].kind, "parsing-error");
         });
 
-        it("should call the finalize function", async (t) => {
+        it("should call the finalize function", async(t) => {
           const finalize = t.mock.fn();
 
           await analyser.analyseFile(url, {
@@ -340,7 +340,7 @@ describe("AstAnalyser", (t) => {
           assert.strictEqual(finalize.mock.callCount(), 1);
         });
 
-        it("should pass the source file as first argument", async (t) => {
+        it("should pass the source file as first argument", async(t) => {
           const finalize = t.mock.fn();
 
           await analyser.analyseFile(url, {
@@ -352,7 +352,7 @@ describe("AstAnalyser", (t) => {
       });
 
 
-      it("intialize should be called before finalize", async () => {
+      it("intialize should be called before finalize", async() => {
         const calls = [];
 
         await analyser.analyseFile(url, {
@@ -411,8 +411,8 @@ describe("AstAnalyser", (t) => {
     it("should remove multiple HTML comments", () => {
       const preparedSource = getAnalyser().prepareSource(
         "<!-- const yo = 5; -->\nconst yo = 'foo'\n<!-- const yo = 5; -->", {
-        removeHTMLComments: true
-      });
+          removeHTMLComments: true
+        });
 
       assert.strictEqual(preparedSource, "\nconst yo = 'foo'\n");
     });

test/Deobfuscator.spec.js

@@ -7,7 +7,7 @@ import { walk } from "estree-walker";
 
 // Import Internal Dependencies
 import { Deobfuscator } from "../src/Deobfuscator.js";
-import { JsSourceParser } from "../src/JsSourceParser.js";
+import { JsSourceParser } from "../index.js";
 
 describe("Deobfuscator", () => {
   describe("identifiers and counters", () => {

test/EntryFilesAnalyser.spec.js

@@ -1,10 +1,10 @@
 // Import Node.js Dependencies
 import { describe, it } from "node:test";
 import assert from "node:assert";
+import { fileURLToPath } from "node:url";
 
 // Import Internal Dependencies
-import { EntryFilesAnalyser } from "../src/EntryFilesAnalyser.js";
-import { AstAnalyser } from "../src/AstAnalyser.js";
+import { EntryFilesAnalyser, AstAnalyser } from "../index.js";
 
 const FIXTURE_URL = new URL("fixtures/entryFiles/", import.meta.url);
 
@@ -15,19 +15,24 @@ describe("EntryFilesAnalyser", () => {
     const deepEntryUrl = new URL("deps/deepEntry.js", FIXTURE_URL);
 
     t.mock.method(AstAnalyser.prototype, "analyseFile");
-    const generator = entryFilesAnalyser.analyse([entryUrl, deepEntryUrl]);
 
-    // First entry
-    await assertReport(generator, entryUrl);
-    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL));
-    await assertReport(generator, new URL("shared.js", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep2.js", FIXTURE_URL));
-
-    // Second entry
-    await assertReport(generator, deepEntryUrl);
-    await assertReport(generator, new URL("deps/dep3.js", FIXTURE_URL));
-
-    await assertAllReportsYielded(generator);
+    const generator = entryFilesAnalyser.analyse([
+      entryUrl,
+      deepEntryUrl
+    ]);
+    const reports = await fromAsync(generator);
+
+    assert.deepEqual(
+      reports.map((report) => report.url),
+      [
+        entryUrl,
+        new URL("deps/dep1.js", FIXTURE_URL),
+        new URL("shared.js", FIXTURE_URL),
+        new URL("deps/dep2.js", FIXTURE_URL),
+        deepEntryUrl,
+        new URL("deps/dep3.js", FIXTURE_URL)
+      ].map((url) => fileURLToPath(url))
+    );
 
     // Check that shared dependencies are not analyzed several times
     const calls = AstAnalyser.prototype.analyseFile.mock.calls;
@@ -39,62 +44,79 @@ describe("EntryFilesAnalyser", () => {
     const entryUrl = new URL("entryWithInvalidDep.js", FIXTURE_URL);
 
     const generator = entryFilesAnalyser.analyse([entryUrl]);
-
-    await assertReport(generator, entryUrl);
-
-    const invalidDepReport = await generator.next();
-    assert.ok(!invalidDepReport.value.ok);
-    assert.strictEqual(invalidDepReport.value.url, new URL("deps/invalidDep.js", FIXTURE_URL).pathname);
-    assert.strictEqual(invalidDepReport.value.warnings[0].kind, "parsing-error");
-
-    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL));
-    await assertReport(generator, new URL("shared.js", FIXTURE_URL));
-
-    await assertAllReportsYielded(generator);
+    const reports = await fromAsync(generator);
+
+    assert.deepEqual(
+      reports.map((report) => report.url),
+      [
+        entryUrl,
+        new URL("deps/invalidDep.js", FIXTURE_URL),
+        new URL("deps/dep1.js", FIXTURE_URL),
+        new URL("shared.js", FIXTURE_URL)
+      ].map((url) => fileURLToPath(url))
+    );
+
+    const invalidReports = reports.filter((report) => !report.ok);
+    assert.strictEqual(invalidReports.length, 1);
+    assert.strictEqual(
+      invalidReports[0].warnings[0].kind,
+      "parsing-error"
+    );
   });
 
   it("should extends default extensions", async() => {
     const entryFilesAnalyser = new EntryFilesAnalyser({
       loadExtensions: (exts) => [...exts, "jsx"]
     });
     const entryUrl = new URL("entryWithVariousDepExtensions.js", FIXTURE_URL);
-    const generator = entryFilesAnalyser.analyse([entryUrl]);
 
-    await assertReport(generator, entryUrl);
-    await assertReport(generator, new URL("deps/default.js", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/default.cjs", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep.cjs", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/default.mjs", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep.mjs", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/default.node", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep.node", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep.jsx", FIXTURE_URL));
-
-    await assertAllReportsYielded(generator);
+    const generator = entryFilesAnalyser.analyse([entryUrl]);
+    const reports = await fromAsync(generator);
+
+    assert.deepEqual(
+      reports.map((report) => report.url),
+      [
+        entryUrl,
+        new URL("deps/default.js", FIXTURE_URL),
+        new URL("deps/default.cjs", FIXTURE_URL),
+        new URL("deps/dep.cjs", FIXTURE_URL),
+        new URL("deps/default.mjs", FIXTURE_URL),
+        new URL("deps/dep.mjs", FIXTURE_URL),
+        new URL("deps/default.node", FIXTURE_URL),
+        new URL("deps/dep.node", FIXTURE_URL),
+        new URL("deps/default.jsx", FIXTURE_URL),
+        new URL("deps/dep.jsx", FIXTURE_URL)
+      ].map((url) => fileURLToPath(url))
+    );
   });
 
   it("should override default extensions", async() => {
     const entryFilesAnalyser = new EntryFilesAnalyser({
       loadExtensions: () => ["jsx"]
     });
     const entryUrl = new URL("entryWithVariousDepExtensions.js", FIXTURE_URL);
-    const generator = entryFilesAnalyser.analyse([entryUrl]);
-
-    await assertReport(generator, entryUrl);
-    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL));
-    await assertReport(generator, new URL("deps/dep.jsx", FIXTURE_URL));
 
-    await assertAllReportsYielded(generator);
+    const generator = entryFilesAnalyser.analyse([entryUrl]);
+    const reports = await fromAsync(generator);
+
+    assert.deepEqual(
+      reports.map((report) => report.url),
+      [
+        entryUrl,
+        new URL("deps/default.jsx", FIXTURE_URL),
+        new URL("deps/dep.jsx", FIXTURE_URL)
+      ].map((url) => fileURLToPath(url))
+    );
   });
+});
 
-  async function assertReport(generator, expectedUrl) {
-    const report = await generator.next();
-    assert.strictEqual(report.value.url, expectedUrl.pathname);
-    assert.ok(report.value.ok);
-  }
+// TODO: replace with Array.fromAsync when droping Node.js 20
+async function fromAsync(asyncIter) {
+  const items = [];
 
-  async function assertAllReportsYielded(generator) {
-    assert.strictEqual((await generator.next()).value, undefined);
+  for await (const item of asyncIter) {
+    items.push(item);
   }
-});
+
+  return items;
+}