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>
  • Loading branch information
RaisinTen authored May 4, 2023
1 parent b7def8e commit c868aad
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 16 deletions.
14 changes: 14 additions & 0 deletions lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,20 @@ class BuiltinModule {
return canBeRequiredByUsersWithoutSchemeList.has(id);
}

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 isBuiltin(id) {
return BuiltinModule.canBeRequiredWithoutScheme(id) || (
typeof id === 'string' &&
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 {
getEmbedderEntryFunction,
compileSerializeMain,
Expand Down Expand Up @@ -98,13 +96,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
11 changes: 6 additions & 5 deletions lib/internal/util/embedding.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const { codes: { ERR_UNKNOWN_BUILTIN_MODULE } } = require('internal/errors');
const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
const { Module, wrapSafe } = require('internal/modules/cjs/loader');

// This is roughly the same as:
Expand Down Expand Up @@ -36,12 +37,12 @@ function embedderRunCjs(contents) {
customDirname);
}

function embedderRequire(path) {
if (!Module.isBuiltin(path)) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(path);
function embedderRequire(id) {
const normalizedId = normalizeRequirableId(id);
if (!normalizedId) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
}

return require(path);
return require(normalizedId);
}

module.exports = { embedderRequire, embedderRunCjs };
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 c868aad

Please sign in to comment.