Repository: nodejs/node
Pull request: module: convert schema-only core module on `convertCJSFilenameToURL`

Fixes: https://github.com/nodejs/node/issues/58607

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/modules/customization_hooks.js
patch: @@ -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);
-  if (builtinId) {
-    return `node:${builtinId}`;
+  let normalizedId = filename;
+  if (StringPrototypeStartsWith(filename, 'node:')) {
+    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.

filename: test/module-hooks/test-module-hooks-builtin-require.js
patch: @@ -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();

filename: test/module-hooks/test-module-hooks-load-builtin-require.js
patch: @@ -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();