Skip to content

Commit 93c4dc5

Browse files
authoredApr 14, 2022
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: #42430 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Richard Lau <[email protected]>
1 parent a058cef commit 93c4dc5

File tree

4 files changed

+52
-18
lines changed

4 files changed

+52
-18
lines changed
 

‎lib/internal/modules/cjs/loader.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,8 @@ if (isWindows) {
666666
}
667667

668668
Module._resolveLookupPaths = function(request, parent) {
669-
if (NativeModule.canBeRequiredByUsers(request)) {
669+
if (NativeModule.canBeRequiredByUsers(request) &&
670+
NativeModule.canBeRequiredWithoutScheme(request)) {
670671
debug('looking for %j in []', request);
671672
return null;
672673
}
@@ -803,11 +804,8 @@ Module._load = function(request, parent, isMain) {
803804
}
804805

805806
const mod = loadNativeModule(filename, request);
806-
if (mod?.canBeRequiredByUsers) {
807-
if (!NativeModule.canBeRequiredWithoutScheme(filename)) {
808-
throw new ERR_UNKNOWN_BUILTIN_MODULE(filename);
809-
}
810-
807+
if (mod?.canBeRequiredByUsers &&
808+
NativeModule.canBeRequiredWithoutScheme(filename)) {
811809
return mod.exports;
812810
}
813811

@@ -854,7 +852,8 @@ Module._load = function(request, parent, isMain) {
854852

855853
Module._resolveFilename = function(request, parent, isMain, options) {
856854
if (StringPrototypeStartsWith(request, 'node:') ||
857-
NativeModule.canBeRequiredByUsers(request)) {
855+
(NativeModule.canBeRequiredByUsers(request) &&
856+
NativeModule.canBeRequiredWithoutScheme(request))) {
858857
return request;
859858
}
860859

@@ -1286,7 +1285,8 @@ Module._preloadModules = function(requests) {
12861285

12871286
Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
12881287
for (const mod of NativeModule.map.values()) {
1289-
if (mod.canBeRequiredByUsers) {
1288+
if (mod.canBeRequiredByUsers &&
1289+
NativeModule.canBeRequiredWithoutScheme(mod.id)) {
12901290
mod.syncExports();
12911291
}
12921292
}

‎lib/internal/modules/esm/loader.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,8 @@ class ESMLoader {
534534
globalThis,
535535
// Param getBuiltin
536536
(builtinName) => {
537-
if (NativeModule.canBeRequiredByUsers(builtinName)) {
537+
if (NativeModule.canBeRequiredByUsers(builtinName) &&
538+
NativeModule.canBeRequiredWithoutScheme(builtinName)) {
538539
return require(builtinName);
539540
}
540541
throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName);

‎lib/internal/modules/esm/resolve.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ const {
5757
ERR_PACKAGE_PATH_NOT_EXPORTED,
5858
ERR_UNSUPPORTED_DIR_IMPORT,
5959
ERR_NETWORK_IMPORT_DISALLOWED,
60-
ERR_UNKNOWN_BUILTIN_MODULE,
6160
ERR_UNSUPPORTED_ESM_URL_SCHEME,
6261
} = require('internal/errors').codes;
6362
const { Module: CJSModule } = require('internal/modules/cjs/loader');
@@ -853,11 +852,8 @@ function parsePackageName(specifier, base) {
853852
* @returns {resolved: URL, format? : string}
854853
*/
855854
function packageResolve(specifier, base, conditions) {
856-
if (NativeModule.canBeRequiredByUsers(specifier)) {
857-
if (!NativeModule.canBeRequiredWithoutScheme(specifier)) {
858-
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
859-
}
860-
855+
if (NativeModule.canBeRequiredByUsers(specifier) &&
856+
NativeModule.canBeRequiredWithoutScheme(specifier)) {
861857
return new URL('node:' + specifier);
862858
}
863859

@@ -1041,7 +1037,8 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
10411037

10421038
return { url: parsed.href };
10431039
}
1044-
if (NativeModule.canBeRequiredByUsers(specifier)) {
1040+
if (NativeModule.canBeRequiredByUsers(specifier) &&
1041+
NativeModule.canBeRequiredWithoutScheme(specifier)) {
10451042
throw new ERR_NETWORK_IMPORT_DISALLOWED(
10461043
specifier,
10471044
parentURL,
+38-2
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,51 @@
11
'use strict';
22
const common = require('../common');
3+
const tmpdir = require('../common/tmpdir');
34
const assert = require('assert');
5+
const { spawnSync } = require('child_process');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const { createRequire } = require('module');
49

510
assert.throws(
611
() => require('test'),
7-
common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
12+
common.expectsError({ code: 'MODULE_NOT_FOUND' }),
813
);
914

1015
(async () => {
1116
await assert.rejects(
1217
async () => import('test'),
13-
common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
18+
common.expectsError({ code: 'ERR_MODULE_NOT_FOUND' }),
1419
);
1520
})().then(common.mustCall());
21+
22+
assert.throws(
23+
() => require.resolve('test'),
24+
common.expectsError({ code: 'MODULE_NOT_FOUND' }),
25+
);
26+
27+
// Verify that files in node_modules can be resolved.
28+
tmpdir.refresh();
29+
30+
const packageRoot = path.join(tmpdir.path, 'node_modules', 'test');
31+
const indexFile = path.join(packageRoot, 'index.js');
32+
33+
fs.mkdirSync(packageRoot, { recursive: true });
34+
fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };');
35+
36+
function test(argv) {
37+
const child = spawnSync(process.execPath, argv, { cwd: tmpdir.path });
38+
assert.strictEqual(child.status, 0);
39+
assert.strictEqual(child.stdout.toString().trim(), '{ marker: 1 }');
40+
}
41+
42+
test(['-e', 'console.log(require("test"))']);
43+
test(['-e', 'import("test").then(m=>console.log(m.default))']);
44+
test(['--input-type=module', '-e', 'import test from "test";console.log(test)']);
45+
test(['--input-type=module', '-e', 'console.log((await import("test")).default)']);
46+
47+
{
48+
const dummyFile = path.join(tmpdir.path, 'file.js');
49+
const require = createRequire(dummyFile);
50+
assert.strictEqual(require.resolve('test'), indexFile);
51+
}

0 commit comments

Comments
 (0)
Please sign in to comment.