module: convert schema-only core module on convertCJSFilenameToURL

Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58612
Fixes: https://github.com/nodejs/node/issues/58607
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

lib/internal/modules/customization_hooks.js

@@ -6,6 +6,7 @@ const {
   ArrayPrototypeSplice,
   ObjectAssign,
   ObjectFreeze,
+  StringPrototypeSlice,
   StringPrototypeStartsWith,
   Symbol,
 } = primordials;
@@ -126,9 +127,12 @@ function registerHooks(hooks) {
  */
 function convertCJSFilenameToURL(filename) {
   if (!filename) { return filename; }
-  const builtinId = BuiltinModule.normalizeRequirableId(filename);
+  let normalizedId = filename;
-  if (builtinId) {
+  if (StringPrototypeStartsWith(filename, 'node:')) {
-    return `node:${builtinId}`;
+    normalizedId = StringPrototypeSlice(filename, 5);
+  }
+  if (BuiltinModule.canBeRequiredByUsers(normalizedId)) {
+    return `node:${normalizedId}`;
   }
   // Handle the case where filename is neither a path, nor a built-in id,
   // which is possible via monkey-patching.

test/module-hooks/test-module-hooks-builtin-require.js

@@ -0,0 +1,48 @@
+'use strict';
+
+// This tests that when builtins that demand the `node:` prefix are
+// required, the URL returned by the default resolution step is still
+// prefixed and valid, and that gets passed to the load hook is still
+// the one with the `node:` prefix. The one with the prefix
+// stripped for internal lookups should not get passed into the hooks.
+
+const common = require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+
+const schemelessBlockList = new Set([
+  'sea',
+  'sqlite',
+  'test',
+  'test/reporters',
+]);
+
+const testModules = [];
+for (const mod of schemelessBlockList) {
+  testModules.push(`node:${mod}`);
+}
+
+const hook = registerHooks({
+  resolve: common.mustCall((specifier, context, nextResolve) => {
+    const result = nextResolve(specifier, context);
+    assert.match(result.url, /^node:/);
+    assert.strictEqual(schemelessBlockList.has(result.url.slice(5, result.url.length)), true);
+    return result;
+  }, testModules.length),
+  load: common.mustCall(function load(url, context, nextLoad) {
+    assert.match(url, /^node:/);
+    assert.strictEqual(schemelessBlockList.has(url.slice(5, url.length)), true);
+    const result = nextLoad(url, context);
+    assert.strictEqual(result.source, null);
+    return {
+      source: 'throw new Error("I should not be thrown because the loader ignores user-supplied source for builtins")',
+      format: 'builtin',
+    };
+  }, testModules.length),
+});
+
+for (const mod of testModules) {
+  require(mod);
+}
+
+hook.deregister();

test/module-hooks/test-module-hooks-load-builtin-require.js

@@ -11,8 +11,9 @@ const { registerHooks } = require('module');
 
 // Pick a builtin that's unlikely to be loaded already - like zlib.
 assert(!process.moduleLoadList.includes('NativeModule zlib'));
+let hook;
 
-const hook = registerHooks({
+hook = registerHooks({
   load: common.mustCall(function load(url, context, nextLoad) {
     assert.strictEqual(url, 'node:zlib');
     const result = nextLoad(url, context);
@@ -27,3 +28,39 @@ const hook = registerHooks({
 assert.strictEqual(typeof require('zlib').createGzip, 'function');
 
 hook.deregister();
+
+// This tests that when builtins that demand the `node:` prefix are
+// required, the URL returned by the default resolution step is still
+// prefixed and valid, and that gets passed to the load hook is still
+// the one with the `node:` prefix. The one with the prefix
+// stripped for internal lookups should not get passed into the hooks.
+const schemelessBlockList = new Set([
+  'sea',
+  'sqlite',
+  'test',
+  'test/reporters',
+]);
+
+const testModules = [];
+for (const mod of schemelessBlockList) {
+  testModules.push(`node:${mod}`);
+}
+
+hook = registerHooks({
+  load: common.mustCall(function load(url, context, nextLoad) {
+    assert.match(url, /^node:/);
+    assert.strictEqual(schemelessBlockList.has(url.slice(5, url.length)), true);
+    const result = nextLoad(url, context);
+    assert.strictEqual(result.source, null);
+    return {
+      source: 'throw new Error("I should not be thrown because the loader ignores user-supplied source for builtins")',
+      format: 'builtin',
+    };
+  }, testModules.length),
+});
+
+for (const mod of testModules) {
+  require(mod);
+}
+
+hook.deregister();