From bf2e8104214607b3d5ca9f1ae06f5bd29c36cb80 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sat, 19 Mar 2022 23:27:07 -0400 Subject: [PATCH] module,repl: support 'node:'-only core modules This commit makes it possible to add new core modules that can only be require()'ed and imported when the 'node:' scheme is used. The 'test' module is the first such module. These 'node:'-only modules are not included in the list returned by module.builtinModules. PR-URL: https://github.com/nodejs/node/pull/42325 Refs: https://github.com/nodejs/node/issues/40954 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel --- lib/internal/bootstrap/loaders.js | 16 ++++++++++++++++ lib/internal/modules/cjs/loader.js | 11 +++++++++-- lib/internal/modules/esm/resolve.js | 8 +++++++- lib/repl.js | 5 +++++ test/parallel/test-code-cache.js | 2 +- test/parallel/test-repl-tab-complete-import.js | 16 ++++++++++------ test/parallel/test-repl-tab-complete.js | 16 ++++++++++------ test/parallel/test-runner-import-no-scheme.js | 15 +++++++++++++++ 8 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 test/parallel/test-runner-import-no-scheme.js diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index f5ea60a6ab6228..41162fabcbc589 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -44,6 +44,7 @@ /* global process, getLinkedBinding, getInternalBinding, primordials */ const { + ArrayFrom, ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSlice, @@ -120,6 +121,11 @@ const legacyWrapperList = new SafeSet([ 'util', ]); +// Modules that can only be imported via the node: scheme. +const schemelessBlockList = new SafeSet([ + 'test', +]); + // Set up process.binding() and process._linkedBinding(). { const bindingObj = ObjectCreate(null); @@ -243,6 +249,16 @@ class NativeModule { return mod && mod.canBeRequiredByUsers; } + // Determine if a core module can be loaded without the node: prefix. This + // function does not validate if the module actually exists. + static canBeRequiredWithoutScheme(id) { + return !schemelessBlockList.has(id); + } + + static getSchemeOnlyModuleNames() { + return ArrayFrom(schemelessBlockList); + } + // Used by user-land module loaders to compile and load builtins. compileForPublicLoader() { if (!this.canBeRequiredByUsers) { diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 24d283ebbe036c..740985a381dbaa 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -182,7 +182,8 @@ function Module(id = '', parent) { const builtinModules = []; for (const { 0: id, 1: mod } of NativeModule.map) { - if (mod.canBeRequiredByUsers) { + if (mod.canBeRequiredByUsers && + NativeModule.canBeRequiredWithoutScheme(id)) { ArrayPrototypePush(builtinModules, id); } } @@ -805,7 +806,13 @@ Module._load = function(request, parent, isMain) { } const mod = loadNativeModule(filename, request); - if (mod?.canBeRequiredByUsers) return mod.exports; + if (mod?.canBeRequiredByUsers) { + if (!NativeModule.canBeRequiredWithoutScheme(filename)) { + throw new ERR_UNKNOWN_BUILTIN_MODULE(filename); + } + + return mod.exports; + } // Don't call updateChildren(), Module constructor already does. const module = cachedModule || new Module(filename, parent); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index f927c74e718641..7b6537ff8ba187 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -57,6 +57,7 @@ const { ERR_PACKAGE_PATH_NOT_EXPORTED, ERR_UNSUPPORTED_DIR_IMPORT, ERR_NETWORK_IMPORT_DISALLOWED, + ERR_UNKNOWN_BUILTIN_MODULE, ERR_UNSUPPORTED_ESM_URL_SCHEME, } = require('internal/errors').codes; const { Module: CJSModule } = require('internal/modules/cjs/loader'); @@ -887,8 +888,13 @@ function parsePackageName(specifier, base) { * @returns {URL} */ function packageResolve(specifier, base, conditions) { - if (NativeModule.canBeRequiredByUsers(specifier)) + if (NativeModule.canBeRequiredByUsers(specifier)) { + if (!NativeModule.canBeRequiredWithoutScheme(specifier)) { + throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); + } + return new URL('node:' + specifier); + } const { packageName, packageSubpath, isScoped } = parsePackageName(specifier, base); diff --git a/lib/repl.js b/lib/repl.js index d84c1922d9e947..eeb8594eff4802 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -101,6 +101,7 @@ const { globalThis, } = primordials; +const { NativeModule } = require('internal/bootstrap/loaders'); const { makeRequireFunction, addBuiltinLibsToObject @@ -130,6 +131,10 @@ let _builtinLibs = ArrayPrototypeFilter( ); const nodeSchemeBuiltinLibs = ArrayPrototypeMap( _builtinLibs, (lib) => `node:${lib}`); +ArrayPrototypeForEach( + NativeModule.getSchemeOnlyModuleNames(), + (lib) => ArrayPrototypePush(nodeSchemeBuiltinLibs, `node:${lib}`), +); const domain = require('domain'); let debug = require('internal/util/debuglog').debuglog('repl', (fn) => { debug = fn; diff --git a/test/parallel/test-code-cache.js b/test/parallel/test-code-cache.js index 1b151e269dcfaf..f61ed9f5c54077 100644 --- a/test/parallel/test-code-cache.js +++ b/test/parallel/test-code-cache.js @@ -16,7 +16,7 @@ const { } = internalBinding('native_module'); for (const key of canBeRequired) { - require(key); + require(`node:${key}`); } // The computation has to be delayed until we have done loading modules diff --git a/test/parallel/test-repl-tab-complete-import.js b/test/parallel/test-repl-tab-complete-import.js index 1968caa5accf54..e328d95db5986c 100644 --- a/test/parallel/test-repl-tab-complete-import.js +++ b/test/parallel/test-repl-tab-complete-import.js @@ -53,14 +53,18 @@ testMe.complete("import\t( 'n", common.mustCall((error, data) => { assert.strictEqual(data[1], 'n'); const completions = data[0]; // import(...) completions include `node:` URL modules: - publicModules.forEach((lib, index) => - assert.strictEqual(completions[index], `node:${lib}`)); - assert.strictEqual(completions[publicModules.length], ''); + let lastIndex = -1; + + publicModules.forEach((lib, index) => { + lastIndex = completions.indexOf(`node:${lib}`); + assert.notStrictEqual(lastIndex, -1); + }); + assert.strictEqual(completions[lastIndex + 1], ''); // There is only one Node.js module that starts with n: - assert.strictEqual(completions[publicModules.length + 1], 'net'); - assert.strictEqual(completions[publicModules.length + 2], ''); + assert.strictEqual(completions[lastIndex + 2], 'net'); + assert.strictEqual(completions[lastIndex + 3], ''); // It's possible to pick up non-core modules too - completions.slice(publicModules.length + 3).forEach((completion) => { + completions.slice(lastIndex + 4).forEach((completion) => { assert.match(completion, /^n/); }); })); diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 4c6386816ddf2f..cc211d6da8aaef 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -293,14 +293,18 @@ testMe.complete("require\t( 'n", common.mustCall(function(error, data) { assert.strictEqual(data.length, 2); assert.strictEqual(data[1], 'n'); // require(...) completions include `node:`-prefixed modules: - publicModules.forEach((lib, index) => - assert.strictEqual(data[0][index], `node:${lib}`)); - assert.strictEqual(data[0][publicModules.length], ''); + let lastIndex = -1; + + publicModules.forEach((lib, index) => { + lastIndex = data[0].indexOf(`node:${lib}`); + assert.notStrictEqual(lastIndex, -1); + }); + assert.strictEqual(data[0][lastIndex + 1], ''); // There is only one Node.js module that starts with n: - assert.strictEqual(data[0][publicModules.length + 1], 'net'); - assert.strictEqual(data[0][publicModules.length + 2], ''); + assert.strictEqual(data[0][lastIndex + 2], 'net'); + assert.strictEqual(data[0][lastIndex + 3], ''); // It's possible to pick up non-core modules too - data[0].slice(publicModules.length + 3).forEach((completion) => { + data[0].slice(lastIndex + 4).forEach((completion) => { assert.match(completion, /^n/); }); })); diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js new file mode 100644 index 00000000000000..4008d7494b6670 --- /dev/null +++ b/test/parallel/test-runner-import-no-scheme.js @@ -0,0 +1,15 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +assert.throws( + () => require('test'), + common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }), +); + +(async () => { + await assert.rejects( + async () => import('test'), + common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }), + ); +})().then(common.mustCall());