module: fixing url change in load sync hook chain

Fixes: nodejs#56376
PR-URL: nodejs#56402
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: awto

lib/internal/modules/cjs/loader.js

@@ -1144,7 +1144,7 @@ function getDefaultLoad(url, filename) {
   return function defaultLoad(urlFromHook, context) {
     // If the url is the same as the original one, save the conversion.
     const isLoadingOriginalModule = (urlFromHook === url);
-    const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(url);
+    const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(urlFromHook);
     const source = defaultLoadImpl(filenameFromHook, context.format);
     // Format from context is directly returned, because format detection should only be
     // done after the entire load chain is completed.

test/module-hooks/test-module-hooks-load-url-change-import.mjs

@@ -0,0 +1,25 @@
+import { mustCall } from '../common/index.mjs';
+import assert from 'node:assert';
+import { registerHooks } from 'node:module';
+import { fileURL } from '../common/fixtures.mjs';
+
+// This tests shows the url parameter in `load` can be changed in the `nextLoad` call
+// It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`
+
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    assert.strictEqual(specifier, 'foo');
+    return {
+      url: 'foo://bar',
+      shortCircuit: true,
+    };
+  },
+  load: mustCall(function load(url, context, nextLoad) {
+    assert.strictEqual(url, 'foo://bar');
+    return nextLoad(fileURL('module-hooks', 'redirected-fs.js').href, context);
+  }),
+});
+
+assert.strictEqual((await import('foo')).exports_for_test, 'redirected fs');
+
+hook.deregister();

test/module-hooks/test-module-hooks-load-url-change-require.js

@@ -0,0 +1,30 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+const fixtures = require('../common/fixtures');
+
+// This tests shows the url parameter in `load` can be changed in the `nextLoad` call
+// It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`
+
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    assert.strictEqual(specifier, 'foo');
+    return {
+      url: 'foo://bar',
+      shortCircuit: true,
+    };
+  },
+  load: common.mustCall(function load(url, context, nextLoad) {
+    assert.strictEqual(url, 'foo://bar');
+    return nextLoad(
+      fixtures.fileURL('module-hooks', 'redirected-fs.js').href,
+      context,
+    );
+  }),
+});
+
+assert.strictEqual(require('foo').exports_for_test, 'redirected fs');
+
+hook.deregister();