From 197530e4b9ad0f948e1ef3ba4ea2fbb0043f18c0 Mon Sep 17 00:00:00 2001
From: Sergey Chernyshev <schernyshev@openjdk.org>
Date: Thu, 25 Apr 2024 01:53:40 +0200
Subject: [PATCH] module: refactor to use `normalizeRequirableId` in the CJS

`BuiltinModule.normalizeRequirableId()` was introduced in
https://github.com/nodejs/node/pull/47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
---
 graal-nodejs/lib/internal/bootstrap/realm.js     | 15 +++++++--------
 graal-nodejs/lib/internal/modules/cjs/loader.js  | 16 ++--------------
 .../test/fixtures/errors/force_colors.snapshot   |  8 ++++----
 3 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/graal-nodejs/lib/internal/bootstrap/realm.js b/graal-nodejs/lib/internal/bootstrap/realm.js
index 291c6749a39..81144c280a1 100644
--- a/graal-nodejs/lib/internal/bootstrap/realm.js
+++ b/graal-nodejs/lib/internal/bootstrap/realm.js
@@ -300,17 +300,16 @@ class BuiltinModule {
   }
 
   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;
+      const normalizedId = StringPrototypeSlice(id, 5);
+      if (BuiltinModule.canBeRequiredByUsers(normalizedId)) {
+        return normalizedId;
+      }
+    } else if (BuiltinModule.canBeRequiredWithoutScheme(id)) {
+      return id;
     }
 
-    return normalizedId;
+    return undefined;
   }
 
   static getSchemeOnlyModuleNames() {
diff --git a/graal-nodejs/lib/internal/modules/cjs/loader.js b/graal-nodejs/lib/internal/modules/cjs/loader.js
index f6ec328e09f..44fab4de4c0 100644
--- a/graal-nodejs/lib/internal/modules/cjs/loader.js
+++ b/graal-nodejs/lib/internal/modules/cjs/loader.js
@@ -782,12 +782,7 @@ if (isWindows) {
 }
 
 Module._resolveLookupPaths = function(request, parent) {
-  if ((
-    StringPrototypeStartsWith(request, 'node:') &&
-    BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
-  ) || (
-    BuiltinModule.canBeRequiredWithoutScheme(request)
-  )) {
+  if (BuiltinModule.normalizeRequirableId(request)) {
     debug('looking for %j in []', request);
     return null;
   }
@@ -979,14 +974,7 @@ Module._load = function(request, parent, isMain) {
 };
 
 Module._resolveFilename = function(request, parent, isMain, options) {
-  if (
-    (
-      StringPrototypeStartsWith(request, 'node:') &&
-      BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
-    ) || (
-      BuiltinModule.canBeRequiredWithoutScheme(request)
-    )
-  ) {
+  if (BuiltinModule.normalizeRequirableId(request)) {
     return request;
   }
 
diff --git a/graal-nodejs/test/fixtures/errors/force_colors.snapshot b/graal-nodejs/test/fixtures/errors/force_colors.snapshot
index 0a0cfd1040f..08f0757ee07 100644
--- a/graal-nodejs/test/fixtures/errors/force_colors.snapshot
+++ b/graal-nodejs/test/fixtures/errors/force_colors.snapshot
@@ -4,10 +4,10 @@ throw new Error('Should include grayed stack trace')
 
 Error: Should include grayed stack trace
     at Object.<anonymous> (/test*force_colors.js:1:7)
-    at Module._compile (node:internal*modules*cjs*loader:1257:14)
-    at Module._extensions..js (node:internal*modules*cjs*loader:1311:10)
-    at Module.load (node:internal*modules*cjs*loader:1115:32)
-    at Module._load (node:internal*modules*cjs*loader:955:12)
+    at Module._compile (node:internal*modules*cjs*loader:1245:14)
+    at Module._extensions..js (node:internal*modules*cjs*loader:1299:10)
+    at Module.load (node:internal*modules*cjs*loader:1103:32)
+    at Module._load (node:internal*modules*cjs*loader:950:12)
     at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:88:12)
     at node:internal*main*run_main_module:23:47