Skip to content

Commit

Permalink
sea: allow requiring core modules with the "node:" prefix
Browse files Browse the repository at this point in the history
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 <raisinten@gmail.com>
PR-URL: #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>
  • Loading branch information
RaisinTen authored and targos committed Nov 10, 2023
1 parent 9a4b57d commit b8634ee
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 16 deletions.
16 changes: 15 additions & 1 deletion lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ const {
SafeMap,
SafeSet,
String,
StringPrototypeStartsWith,
StringPrototypeSlice,
StringPrototypeStartsWith,
TypeError,
} = primordials;

Expand Down Expand Up @@ -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);
}
Expand Down
13 changes: 3 additions & 10 deletions lib/internal/main/mksnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}'. `,
Expand Down
10 changes: 6 additions & 4 deletions lib/internal/main/single_executable_application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
17 changes: 16 additions & 1 deletion test/fixtures/sea.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);

Expand Down

0 comments on commit b8634ee

Please sign in to comment.