From 93c4dc5e5a90a51467d532f5659df271f08e3d23 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Thu, 14 Apr 2022 09:08:55 -0400 Subject: [PATCH] module: ensure 'node:'-only modules can access node_modules This commit allows require() and import to search the node_modules directories when importing a core module that must have the node: scheme. This prevents these core modules from shadowing userland modules with the same name but no prefix. PR-URL: https://github.com/nodejs/node/pull/42430 Reviewed-By: Darshan Sen Reviewed-By: Antoine du Hamel Reviewed-By: Beth Griggs Reviewed-By: Richard Lau --- lib/internal/modules/cjs/loader.js | 16 ++++---- lib/internal/modules/esm/loader.js | 3 +- lib/internal/modules/esm/resolve.js | 11 ++--- test/parallel/test-runner-import-no-scheme.js | 40 ++++++++++++++++++- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 3fd1aa3404f884..de919e7406b4e7 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -666,7 +666,8 @@ if (isWindows) { } Module._resolveLookupPaths = function(request, parent) { - if (NativeModule.canBeRequiredByUsers(request)) { + if (NativeModule.canBeRequiredByUsers(request) && + NativeModule.canBeRequiredWithoutScheme(request)) { debug('looking for %j in []', request); return null; } @@ -803,11 +804,8 @@ Module._load = function(request, parent, isMain) { } const mod = loadNativeModule(filename, request); - if (mod?.canBeRequiredByUsers) { - if (!NativeModule.canBeRequiredWithoutScheme(filename)) { - throw new ERR_UNKNOWN_BUILTIN_MODULE(filename); - } - + if (mod?.canBeRequiredByUsers && + NativeModule.canBeRequiredWithoutScheme(filename)) { return mod.exports; } @@ -854,7 +852,8 @@ Module._load = function(request, parent, isMain) { Module._resolveFilename = function(request, parent, isMain, options) { if (StringPrototypeStartsWith(request, 'node:') || - NativeModule.canBeRequiredByUsers(request)) { + (NativeModule.canBeRequiredByUsers(request) && + NativeModule.canBeRequiredWithoutScheme(request))) { return request; } @@ -1286,7 +1285,8 @@ Module._preloadModules = function(requests) { Module.syncBuiltinESMExports = function syncBuiltinESMExports() { for (const mod of NativeModule.map.values()) { - if (mod.canBeRequiredByUsers) { + if (mod.canBeRequiredByUsers && + NativeModule.canBeRequiredWithoutScheme(mod.id)) { mod.syncExports(); } } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 0dccfebbd4f37f..39f8ebca59e9df 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -534,7 +534,8 @@ class ESMLoader { globalThis, // Param getBuiltin (builtinName) => { - if (NativeModule.canBeRequiredByUsers(builtinName)) { + if (NativeModule.canBeRequiredByUsers(builtinName) && + NativeModule.canBeRequiredWithoutScheme(builtinName)) { return require(builtinName); } throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index e640947450c973..f1298e58dc445c 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -57,7 +57,6 @@ 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'); @@ -853,11 +852,8 @@ function parsePackageName(specifier, base) { * @returns {resolved: URL, format? : string} */ function packageResolve(specifier, base, conditions) { - if (NativeModule.canBeRequiredByUsers(specifier)) { - if (!NativeModule.canBeRequiredWithoutScheme(specifier)) { - throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); - } - + if (NativeModule.canBeRequiredByUsers(specifier) && + NativeModule.canBeRequiredWithoutScheme(specifier)) { return new URL('node:' + specifier); } @@ -1041,7 +1037,8 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) { return { url: parsed.href }; } - if (NativeModule.canBeRequiredByUsers(specifier)) { + if (NativeModule.canBeRequiredByUsers(specifier) && + NativeModule.canBeRequiredWithoutScheme(specifier)) { throw new ERR_NETWORK_IMPORT_DISALLOWED( specifier, parentURL, diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 4008d7494b6670..45dd83d0251988 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -1,15 +1,51 @@ 'use strict'; const common = require('../common'); +const tmpdir = require('../common/tmpdir'); const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fs = require('fs'); +const path = require('path'); +const { createRequire } = require('module'); assert.throws( () => require('test'), - common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }), + common.expectsError({ code: 'MODULE_NOT_FOUND' }), ); (async () => { await assert.rejects( async () => import('test'), - common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }), + common.expectsError({ code: 'ERR_MODULE_NOT_FOUND' }), ); })().then(common.mustCall()); + +assert.throws( + () => require.resolve('test'), + common.expectsError({ code: 'MODULE_NOT_FOUND' }), +); + +// Verify that files in node_modules can be resolved. +tmpdir.refresh(); + +const packageRoot = path.join(tmpdir.path, 'node_modules', 'test'); +const indexFile = path.join(packageRoot, 'index.js'); + +fs.mkdirSync(packageRoot, { recursive: true }); +fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };'); + +function test(argv) { + const child = spawnSync(process.execPath, argv, { cwd: tmpdir.path }); + assert.strictEqual(child.status, 0); + assert.strictEqual(child.stdout.toString().trim(), '{ marker: 1 }'); +} + +test(['-e', 'console.log(require("test"))']); +test(['-e', 'import("test").then(m=>console.log(m.default))']); +test(['--input-type=module', '-e', 'import test from "test";console.log(test)']); +test(['--input-type=module', '-e', 'console.log((await import("test")).default)']); + +{ + const dummyFile = path.join(tmpdir.path, 'file.js'); + const require = createRequire(dummyFile); + assert.strictEqual(require.resolve('test'), indexFile); +}