From 614323fed5538a95758c8dbf6493d696eb7b8043 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 29 Apr 2023 11:30:47 +0530 Subject: [PATCH 1/2] 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 --- lib/internal/util/embedding.js | 8 +++++++- test/fixtures/sea.js | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/internal/util/embedding.js b/lib/internal/util/embedding.js index 139d4c7a25fb7c..ac2ae946dedede 100644 --- a/lib/internal/util/embedding.js +++ b/lib/internal/util/embedding.js @@ -36,12 +36,18 @@ function embedderRunCjs(contents) { customDirname); } +let embedderRequireInternal; + function embedderRequire(path) { if (!Module.isBuiltin(path)) { throw new ERR_UNKNOWN_BUILTIN_MODULE(path); } - return require(path); + if (!embedderRequireInternal) { + embedderRequireInternal = Module.createRequire(process.execPath); + } + + return embedderRequireInternal(path); } module.exports = { embedderRequire, embedderRunCjs }; diff --git a/test/fixtures/sea.js b/test/fixtures/sea.js index efdc32708b9898..2ce544cd909cfe 100644 --- a/test/fixtures/sea.js +++ b/test/fixtures/sea.js @@ -10,7 +10,7 @@ expectWarning('ExperimentalWarning', 'might change at any time'); const { deepStrictEqual, strictEqual, throws } = require('assert'); -const { dirname } = require('path'); +const { dirname } = require('node:path'); deepStrictEqual(process.argv, [process.execPath, process.execPath, '-a', '--b=c', 'd']); From 2d9e3c5836bfc0476abbd72373f7b34378005ca0 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Wed, 3 May 2023 12:53:02 +0530 Subject: [PATCH 2/2] bootstrap: fix bug in the snapshot require function and share the code with the SEA require Previously, this code: ```sh node --snapshot-blob snapshot.blob --build-snapshot <(echo "require('node:test')")` ``` was failing with a "Error: Cannot find module 'node:test'." error, so this change fixes that and uses the same code path for normalizing ids for both the snapshot require function as well as the SEA require function. However, building a snapshot when `node:test` is required now fails with: ```sh ./node --snapshot-blob snapshot.blob --build-snapshot <(echo "require('node:test')") (node:5362) Warning: built-in module node:test is not yet supported in user snapshots (Use `node --trace-warnings ...` to show where the warning was created) Unknown external reference 0x10201dd70. [1] 5362 trace trap ./node --snapshot-blob snapshot.blob --build-snapshot ``` which could be solved in a separate PR. Refs: https://github.com/nodejs/node/pull/47779#discussion_r1181671294 Signed-off-by: Darshan Sen --- lib/internal/bootstrap/realm.js | 14 ++++++++++++++ lib/internal/main/mksnapshot.js | 13 +++---------- lib/internal/util/embedding.js | 17 ++++++----------- test/fixtures/sea.js | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js index c8cdaeff7d369b..9dc0aee75125e3 100644 --- a/lib/internal/bootstrap/realm.js +++ b/lib/internal/bootstrap/realm.js @@ -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' && diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js index 376639ef91996e..2a9b5d9851a2b1 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 { getEmbedderEntryFunction, compileSerializeMain, @@ -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}'. `, diff --git a/lib/internal/util/embedding.js b/lib/internal/util/embedding.js index ac2ae946dedede..e2e67202477bc7 100644 --- a/lib/internal/util/embedding.js +++ b/lib/internal/util/embedding.js @@ -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: @@ -36,18 +37,12 @@ function embedderRunCjs(contents) { customDirname); } -let embedderRequireInternal; - -function embedderRequire(path) { - if (!Module.isBuiltin(path)) { - throw new ERR_UNKNOWN_BUILTIN_MODULE(path); - } - - if (!embedderRequireInternal) { - embedderRequireInternal = Module.createRequire(process.execPath); +function embedderRequire(id) { + const normalizedId = normalizeRequirableId(id); + if (!normalizedId) { + throw new ERR_UNKNOWN_BUILTIN_MODULE(id); } - - return embedderRequireInternal(path); + return require(normalizedId); } module.exports = { embedderRequire, embedderRunCjs }; diff --git a/test/fixtures/sea.js b/test/fixtures/sea.js index 2ce544cd909cfe..2cd82c709ea157 100644 --- a/test/fixtures/sea.js +++ b/test/fixtures/sea.js @@ -9,9 +9,24 @@ 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('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']); strictEqual(require.cache, undefined);