esm: fix http(s) import via custom loader

PR-URL: https://github.com/nodejs/node/pull/43130
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

5 files changed, 154 insertions, 14 deletions

diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js
index f6c2fc8827a..eeca8fc5e8e 100644
--- a/lib/internal/modules/esm/fetch_module.js
+++ b/lib/internal/modules/esm/fetch_module.js
@@ -238,6 +238,17 @@ function fetchModule(parsed, { parentURL }) {
   return fetchWithRedirects(parsed);
 }
 
+/**
+ * Checks if the given canonical URL exists in the fetch cache
+ *
+ * @param {string} key
+ * @returns {boolean}
+ */
+function inFetchCache(key) {
+  return cacheForGET.has(key);
+}
+
 module.exports = {
-  fetchModule: fetchModule,
+  fetchModule,
+  inFetchCache,
 };
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 8bc22ef697d..c867595bccc 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -57,6 +57,7 @@ const { translators } = require(
 const { getOptionValue } = require('internal/options');
 const {
   fetchModule,
+  inFetchCache,
 } = require('internal/modules/esm/fetch_module');
 
 
@@ -338,23 +339,35 @@ class ESMLoader {
    * would have a cache key of https://example.com/foo and baseURL
    * of https://example.com/bar
    *
-   * MUST BE SYNCHRONOUS for import.meta initialization
-   * MUST BE CALLED AFTER receiving the url body due to I/O
-   * @param {string} url
-   * @returns {string}
+   * ! MUST BE SYNCHRONOUS for import.meta initialization
+   * ! MUST BE CALLED AFTER receiving the url body due to I/O
+   * @param {URL['href']} url
+   * @returns {string|Promise<URL['href']>}
    */
   getBaseURL(url) {
-    if (
+    if (getOptionValue('--experimental-network-imports') && (
       StringPrototypeStartsWith(url, 'http:') ||
       StringPrototypeStartsWith(url, 'https:')
-    ) {
-      // The request & response have already settled, so they are in
-      // fetchModule's cache, in which case, fetchModule returns
+    )) {
+      // When using network-imports, the request & response have already settled
+      // so they are in fetchModule's cache, in which case, fetchModule returns
       // immediately and synchronously
-      url = fetchModule(new URL(url), { parentURL: url }).resolvedHREF;
-      // This should only occur if the module hasn't been fetched yet
-      if (typeof url !== 'string') { // [2]
-        throw new ERR_INTERNAL_ASSERTION(`Base url for module ${url} not loaded.`);
+      // Unless a custom loader bypassed the fetch cache, in which case we just
+      // use the original url
+      if (inFetchCache(url)) {
+        const module = fetchModule(new URL(url), { parentURL: url });
+        if (typeof module?.resolvedHREF === 'string') {
+          return module.resolvedHREF;
+        }
+        // Internal error
+        throw new ERR_INTERNAL_ASSERTION(
+          `Base url for module ${url} not loaded.`
+        );
+      } else {
+        // A custom loader was used instead of network-imports.
+        // Adding support for a response URL resolve return in custom loaders is
+        // pending.
+        return url;
       }
     }
     return url;
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
index e012eebc4ac..d654958ad14 100644
--- a/lib/internal/modules/esm/module_job.js
+++ b/lib/internal/modules/esm/module_job.js
@@ -76,7 +76,11 @@ class ModuleJob {
       // these `link` callbacks depending on each other.
       const dependencyJobs = [];
       const promises = this.module.link(async (specifier, assertions) => {
-        const baseURL = this.loader.getBaseURL(url);
+        const base = await this.loader.getBaseURL(url);
+        const baseURL = typeof base === 'string' ?
+          base :
+          base.resolvedHREF;
+
         const jobPromise = this.loader.getModuleJob(specifier, baseURL, assertions);
         ArrayPrototypePush(dependencyJobs, jobPromise);
         const job = await jobPromise;
diff --git a/test/es-module/test-esm-loader-http-imports.mjs b/test/es-module/test-esm-loader-http-imports.mjs
new file mode 100644
index 00000000000..5f6cc47f388
--- /dev/null
+++ b/test/es-module/test-esm-loader-http-imports.mjs
@@ -0,0 +1,72 @@
+import { mustCall } from '../common/index.mjs';
+import fixtures from '../common/fixtures.js';
+import { strictEqual } from 'node:assert';
+import { spawn } from 'node:child_process';
+import http from 'node:http';
+import path from 'node:path';
+import { promisify } from 'node:util';
+
+
+const files = {
+  'main.mjs': 'export * from "./lib.mjs";',
+  'lib.mjs': 'export { sum } from "./sum.mjs";',
+  'sum.mjs': 'export function sum(a, b) { return a + b }',
+};
+
+const requestListener = ({ url }, rsp) => {
+  const filename = path.basename(url);
+  const content = files[filename];
+
+  if (content) {
+    return rsp
+      .writeHead(200, { 'Content-Type': 'application/javascript' })
+      .end(content);
+  }
+
+  return rsp
+    .writeHead(404)
+    .end();
+};
+
+const server = http.createServer(requestListener);
+
+await promisify(server.listen.bind(server))({
+  host: '127.0.0.1',
+  port: 0,
+});
+
+const {
+  address: host,
+  port,
+} = server.address();
+
+{ // Verify nested HTTP imports work
+  const child = spawn( // ! `spawn` MUST be used (vs `spawnSync`) to avoid blocking the event loop
+    process.execPath,
+    [
+      '--no-warnings',
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'http-loader.mjs'),
+      '--input-type=module',
+      '--eval',
+      `import * as main from 'http://${host}:${port}/main.mjs'; console.log(main)`,
+    ]
+  );
+
+  let stderr = '';
+  let stdout = '';
+
+  child.stderr.setEncoding('utf8');
+  child.stderr.on('data', (data) => stderr += data);
+  child.stdout.setEncoding('utf8');
+  child.stdout.on('data', (data) => stdout += data);
+
+  child.on('close', mustCall((code, signal) => {
+    strictEqual(stderr, '');
+    strictEqual(stdout, '[Module: null prototype] { sum: [Function: sum] }\n');
+    strictEqual(code, 0);
+    strictEqual(signal, null);
+
+    server.close();
+  }));
+}
diff --git a/test/fixtures/es-module-loaders/http-loader.mjs b/test/fixtures/es-module-loaders/http-loader.mjs
new file mode 100644
index 00000000000..f0add5d5b41
--- /dev/null
+++ b/test/fixtures/es-module-loaders/http-loader.mjs
@@ -0,0 +1,40 @@
+import { get } from 'http';
+
+export function resolve(specifier, context, nextResolve) {
+  const { parentURL = null } = context;
+
+  if (specifier.startsWith('http://')) {
+    return {
+      shortCircuit: true,
+      url: specifier,
+    };
+  } else if (parentURL?.startsWith('http://')) {
+    return {
+      shortCircuit: true,
+      url: new URL(specifier, parentURL).href,
+    };
+  }
+
+  return nextResolve(specifier, context);
+}
+
+export function load(url, context, nextLoad) {
+  if (url.startsWith('http://')) {
+    return new Promise((resolve, reject) => {
+      get(url, (rsp) => {
+        let data = '';
+        rsp.on('data', (chunk) => data += chunk);
+        rsp.on('end', () => {
+          resolve({
+            format: 'module',
+            shortCircuit: true,
+            source: data,
+          });
+        });
+      })
+      .on('error', reject);
+    });
+  }
+
+  return nextLoad(url, context);
+}