From b8634eeb1601a1ce0edadd6221a83d6d978ce900 Mon Sep 17 00:00:00 2001
From: Darshan Sen <raisinten@gmail.com>
Date: Thu, 4 May 2023 20:02:35 +0530
Subject: [PATCH] sea: allow requiring core modules with the "node:" prefix

Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: https://github.com/nodejs/single-executable/issues/69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
---
 lib/internal/bootstrap/realm.js                 | 16 +++++++++++++++-
 lib/internal/main/mksnapshot.js                 | 13 +++----------
 .../main/single_executable_application.js       | 10 ++++++----
 test/fixtures/sea.js                            | 17 ++++++++++++++++-
 4 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js
index 4eb71e3818db85..291c6749a39076 100644
--- a/lib/internal/bootstrap/realm.js
+++ b/lib/internal/bootstrap/realm.js
@@ -63,8 +63,8 @@ const {
   SafeMap,
   SafeSet,
   String,
-  StringPrototypeStartsWith,
   StringPrototypeSlice,
+  StringPrototypeStartsWith,
   TypeError,
 } = primordials;
 
@@ -299,6 +299,20 @@ class BuiltinModule {
     return ArrayFrom(canBeRequiredByUsersWithoutSchemeList);
   }
 
+  static normalizeRequirableId(id) {
+    let normalizedId = id;
+    if (StringPrototypeStartsWith(id, 'node:')) {
+      normalizedId = StringPrototypeSlice(id, 5);
+    }
+
+    if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
+        (id === normalizedId && !BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
+      return undefined;
+    }
+
+    return normalizedId;
+  }
+
   static getSchemeOnlyModuleNames() {
     return ArrayFrom(schemelessBlockList);
   }
diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js
index ec4e01da051754..d39f29236385b4 100644
--- a/lib/internal/main/mksnapshot.js
+++ b/lib/internal/main/mksnapshot.js
@@ -7,12 +7,10 @@ const {
   ObjectSetPrototypeOf,
   SafeArrayIterator,
   SafeSet,
-  StringPrototypeStartsWith,
-  StringPrototypeSlice,
 } = primordials;
 
 const binding = internalBinding('mksnapshot');
-const { BuiltinModule } = require('internal/bootstrap/realm');
+const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
 const {
   compileSerializeMain,
 } = binding;
@@ -97,13 +95,8 @@ function supportedInUserSnapshot(id) {
 }
 
 function requireForUserSnapshot(id) {
-  let normalizedId = id;
-  if (StringPrototypeStartsWith(id, 'node:')) {
-    normalizedId = StringPrototypeSlice(id, 5);
-  }
-  if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
-      (id !== normalizedId &&
-        !BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
+  const normalizedId = normalizeRequirableId(id);
+  if (!normalizedId) {
     // eslint-disable-next-line no-restricted-syntax
     const err = new Error(
       `Cannot find module '${id}'. `,
diff --git a/lib/internal/main/single_executable_application.js b/lib/internal/main/single_executable_application.js
index d9604cff720d2f..f2995757b21fef 100644
--- a/lib/internal/main/single_executable_application.js
+++ b/lib/internal/main/single_executable_application.js
@@ -7,6 +7,7 @@ const { getSingleExecutableCode } = internalBinding('sea');
 const { emitExperimentalWarning } = require('internal/util');
 const { Module, wrapSafe } = require('internal/modules/cjs/loader');
 const { codes: { ERR_UNKNOWN_BUILTIN_MODULE } } = require('internal/errors');
+const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
 
 prepareMainThreadExecution(false, true);
 markBootstrapComplete();
@@ -33,12 +34,13 @@ customModule.paths = Module._nodeModulePaths(customModule.path);
 
 const customExports = customModule.exports;
 
-function customRequire(path) {
-  if (!Module.isBuiltin(path)) {
-    throw new ERR_UNKNOWN_BUILTIN_MODULE(path);
+function customRequire(id) {
+  const normalizedId = normalizeRequirableId(id);
+  if (!normalizedId) {
+    throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
   }
 
-  return require(path);
+  return require(normalizedId);
 }
 
 customRequire.main = customModule;
diff --git a/test/fixtures/sea.js b/test/fixtures/sea.js
index efdc32708b9898..2cd82c709ea157 100644
--- a/test/fixtures/sea.js
+++ b/test/fixtures/sea.js
@@ -9,8 +9,23 @@ expectWarning('ExperimentalWarning',
               'Single executable application is an experimental feature and ' +
               'might change at any time');
 
+// Should be possible to require core modules that optionally require the
+// "node:" scheme.
 const { deepStrictEqual, strictEqual, throws } = require('assert');
-const { dirname } = require('path');
+const { dirname } = require('node:path');
+
+// Should be possible to require a core module that requires using the "node:"
+// scheme.
+{
+  const { test } = require('node:test');
+  strictEqual(typeof test, 'function');
+}
+
+// Should not be possible to require a core module without the "node:" scheme if
+// it requires using the "node:" scheme.
+throws(() => require('test'), {
+  code: 'ERR_UNKNOWN_BUILTIN_MODULE',
+});
 
 deepStrictEqual(process.argv, [process.execPath, process.execPath, '-a', '--b=c', 'd']);