Skip to content

Commit 1a7fc18

Browse files
RaisinTentargos
authored andcommittedMay 12, 2023
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: nodejs/single-executable#69 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #47779 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 7222f9d commit 1a7fc18

File tree

4 files changed

+39
-16
lines changed

4 files changed

+39
-16
lines changed
 

‎lib/internal/bootstrap/realm.js

+14
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,20 @@ class BuiltinModule {
285285
return canBeRequiredByUsersWithoutSchemeList.has(id);
286286
}
287287

288+
static normalizeRequirableId(id) {
289+
let normalizedId = id;
290+
if (StringPrototypeStartsWith(id, 'node:')) {
291+
normalizedId = StringPrototypeSlice(id, 5);
292+
}
293+
294+
if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
295+
(id === normalizedId && !BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
296+
return undefined;
297+
}
298+
299+
return normalizedId;
300+
}
301+
288302
static isBuiltin(id) {
289303
return BuiltinModule.canBeRequiredWithoutScheme(id) || (
290304
typeof id === 'string' &&

‎lib/internal/main/mksnapshot.js

+3-10
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@ const {
77
ObjectSetPrototypeOf,
88
SafeArrayIterator,
99
SafeSet,
10-
StringPrototypeStartsWith,
11-
StringPrototypeSlice,
1210
} = primordials;
1311

1412
const binding = internalBinding('mksnapshot');
15-
const { BuiltinModule } = require('internal/bootstrap/realm');
13+
const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
1614
const {
1715
getEmbedderEntryFunction,
1816
compileSerializeMain,
@@ -98,13 +96,8 @@ function supportedInUserSnapshot(id) {
9896
}
9997

10098
function requireForUserSnapshot(id) {
101-
let normalizedId = id;
102-
if (StringPrototypeStartsWith(id, 'node:')) {
103-
normalizedId = StringPrototypeSlice(id, 5);
104-
}
105-
if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
106-
(id !== normalizedId &&
107-
!BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
99+
const normalizedId = normalizeRequirableId(id);
100+
if (!normalizedId) {
108101
// eslint-disable-next-line no-restricted-syntax
109102
const err = new Error(
110103
`Cannot find module '${id}'. `,

‎lib/internal/util/embedding.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const { codes: { ERR_UNKNOWN_BUILTIN_MODULE } } = require('internal/errors');
3+
const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
34
const { Module, wrapSafe } = require('internal/modules/cjs/loader');
45

56
// This is roughly the same as:
@@ -36,12 +37,12 @@ function embedderRunCjs(contents) {
3637
customDirname);
3738
}
3839

39-
function embedderRequire(path) {
40-
if (!Module.isBuiltin(path)) {
41-
throw new ERR_UNKNOWN_BUILTIN_MODULE(path);
40+
function embedderRequire(id) {
41+
const normalizedId = normalizeRequirableId(id);
42+
if (!normalizedId) {
43+
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
4244
}
43-
44-
return require(path);
45+
return require(normalizedId);
4546
}
4647

4748
module.exports = { embedderRequire, embedderRunCjs };

‎test/fixtures/sea.js

+16-1
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,23 @@ expectWarning('ExperimentalWarning',
99
'Single executable application is an experimental feature and ' +
1010
'might change at any time');
1111

12+
// Should be possible to require core modules that optionally require the
13+
// "node:" scheme.
1214
const { deepStrictEqual, strictEqual, throws } = require('assert');
13-
const { dirname } = require('path');
15+
const { dirname } = require('node:path');
16+
17+
// Should be possible to require a core module that requires using the "node:"
18+
// scheme.
19+
{
20+
const { test } = require('node:test');
21+
strictEqual(typeof test, 'function');
22+
}
23+
24+
// Should not be possible to require a core module without the "node:" scheme if
25+
// it requires using the "node:" scheme.
26+
throws(() => require('test'), {
27+
code: 'ERR_UNKNOWN_BUILTIN_MODULE',
28+
});
1429

1530
deepStrictEqual(process.argv, [process.execPath, process.execPath, '-a', '--b=c', 'd']);
1631

0 commit comments

Comments
 (0)
Please sign in to comment.