From bca33e26cb7a0a96ad9c5cf22f40bcffd157c6d7 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 21 Mar 2022 22:16:52 -0400 Subject: [PATCH 1/7] 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. --- 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 | 70 ++++++++++++++++++- 4 files changed, 82 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 16b832527f7c64..f38f1d8801bdf1 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -510,7 +510,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 53085f327469b3..4e8e3ddb9dd9d9 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'); @@ -861,11 +860,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); } @@ -1049,7 +1045,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..9e03024c6c1789 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -1,15 +1,81 @@ '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'); 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'); +const cjsFile = path.join(tmpdir.path, 'cjs.js'); +const esmFile = path.join(tmpdir.path, 'esm.mjs'); + +fs.mkdirSync(packageRoot, { recursive: true }); +fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };'); +fs.writeFileSync(cjsFile, ` +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const test = require('test'); + +assert.deepStrictEqual(test, { marker: 1 }); +assert.strictEqual(require.resolve('test'), '${indexFile}'); + +(async function() { + const dynamicImportTest = await import('test'); + + assert.deepStrictEqual(dynamicImportTest.default, { marker: 1 }); +})().then(common.mustCall()); +`); +fs.writeFileSync(esmFile, ` +import { mustCall } from '../common/index.mjs'; +import assert from 'assert'; +import { createRequire } from 'module'; +import test from 'test'; + +assert.deepStrictEqual(test, { marker: 1 }); + +const dynamicImportTest = await import('test'); +assert.deepStrictEqual(dynamicImportTest.default, { marker: 1 }); + +const require = createRequire(import.meta.url); +const requireTest = require('test'); + +assert.deepStrictEqual(requireTest, { marker: 1 }); +assert.strictEqual(require.resolve('test'), '${indexFile}'); +`); + +let child = spawnSync(process.execPath, [cjsFile], { cwd: tmpdir.path }); + +assert.strictEqual(child.stdout.toString().trim(), ''); +assert.strictEqual(child.stderr.toString().trim(), ''); +assert.strictEqual(child.status, 0); +assert.strictEqual(child.signal, null); + +child = spawnSync(process.execPath, [esmFile], { cwd: tmpdir.path }); + +assert.strictEqual(child.stdout.toString().trim(), ''); +assert.strictEqual(child.stderr.toString().trim(), ''); +assert.strictEqual(child.status, 0); +assert.strictEqual(child.signal, null); From 2fc6f2a550097114dd9483bbdacd6ad888bc50db Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:02:31 -0400 Subject: [PATCH 2/7] Update test/parallel/test-runner-import-no-scheme.js Co-authored-by: Antoine du Hamel --- test/parallel/test-runner-import-no-scheme.js | 63 +++++-------------- 1 file changed, 16 insertions(+), 47 deletions(-) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 9e03024c6c1789..2ae798f1d15273 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -28,54 +28,23 @@ tmpdir.refresh(); const packageRoot = path.join(tmpdir.path, 'node_modules', 'test'); const indexFile = path.join(packageRoot, 'index.js'); -const cjsFile = path.join(tmpdir.path, 'cjs.js'); -const esmFile = path.join(tmpdir.path, 'esm.mjs'); +const { createRequire } = require('module'); fs.mkdirSync(packageRoot, { recursive: true }); fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };'); -fs.writeFileSync(cjsFile, ` -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const test = require('test'); - -assert.deepStrictEqual(test, { marker: 1 }); -assert.strictEqual(require.resolve('test'), '${indexFile}'); - -(async function() { - const dynamicImportTest = await import('test'); - - assert.deepStrictEqual(dynamicImportTest.default, { marker: 1 }); -})().then(common.mustCall()); -`); -fs.writeFileSync(esmFile, ` -import { mustCall } from '../common/index.mjs'; -import assert from 'assert'; -import { createRequire } from 'module'; -import test from 'test'; - -assert.deepStrictEqual(test, { marker: 1 }); - -const dynamicImportTest = await import('test'); -assert.deepStrictEqual(dynamicImportTest.default, { marker: 1 }); - -const require = createRequire(import.meta.url); -const requireTest = require('test'); - -assert.deepStrictEqual(requireTest, { marker: 1 }); -assert.strictEqual(require.resolve('test'), '${indexFile}'); -`); - -let child = spawnSync(process.execPath, [cjsFile], { cwd: tmpdir.path }); - -assert.strictEqual(child.stdout.toString().trim(), ''); -assert.strictEqual(child.stderr.toString().trim(), ''); -assert.strictEqual(child.status, 0); -assert.strictEqual(child.signal, null); - -child = spawnSync(process.execPath, [esmFile], { cwd: tmpdir.path }); -assert.strictEqual(child.stdout.toString().trim(), ''); -assert.strictEqual(child.stderr.toString().trim(), ''); -assert.strictEqual(child.status, 0); -assert.strictEqual(child.signal, null); +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(console.log)']) +test(['--input-type=module', '-e', 'import test from test;console.log(test)']); +test(['--input-type=module', '-e', 'console.log(await import("test"))']); + +{ + const require = createRequire(tmpdir.path); + assert.strictEqual(require.resolve('test'), indexFile); +} From 3b626e51bca0221d29649d67d22c0213fefc562c Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:37:05 -0400 Subject: [PATCH 3/7] Update test/parallel/test-runner-import-no-scheme.js Co-authored-by: Antoine du Hamel --- test/parallel/test-runner-import-no-scheme.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 2ae798f1d15273..803ba638d20be1 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -40,11 +40,12 @@ function test(argv) { } test(['-e', 'console.log(require("test"))']) -test(['-e', 'import("test").then(console.log)']) -test(['--input-type=module', '-e', 'import test from test;console.log(test)']); -test(['--input-type=module', '-e', 'console.log(await import("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 require = createRequire(tmpdir.path); + const dummyFile = path.join(tmpdir.path, 'file.js'); + const require = createRequire(dummyFile); assert.strictEqual(require.resolve('test'), indexFile); } From bcc91f8a3edc402efd637d33d7bb66e10df9c09d Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:37:23 -0400 Subject: [PATCH 4/7] Update test/parallel/test-runner-import-no-scheme.js Co-authored-by: Antoine du Hamel --- test/parallel/test-runner-import-no-scheme.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 803ba638d20be1..81a1d2a1de8ace 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -28,7 +28,6 @@ tmpdir.refresh(); const packageRoot = path.join(tmpdir.path, 'node_modules', 'test'); const indexFile = path.join(packageRoot, 'index.js'); -const { createRequire } = require('module'); fs.mkdirSync(packageRoot, { recursive: true }); fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };'); From 8c52cea52faf24005c7cd035ac09f5c2aafc1d86 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:37:30 -0400 Subject: [PATCH 5/7] Update test/parallel/test-runner-import-no-scheme.js Co-authored-by: Antoine du Hamel --- test/parallel/test-runner-import-no-scheme.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 81a1d2a1de8ace..5b424e2b7a1eda 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -5,6 +5,7 @@ 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'), From 712d9f32c99ef7d098ff323ed62ac1946d8f423e Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:43:46 -0400 Subject: [PATCH 6/7] Update test/parallel/test-runner-import-no-scheme.js --- test/parallel/test-runner-import-no-scheme.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index 5b424e2b7a1eda..ed15188599da82 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -39,7 +39,7 @@ function test(argv) { assert.strictEqual(child.stdout.toString().trim(), '{ marker: 1 }'); } -test(['-e', 'console.log(require("test"))']) +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)']); From aa7445a186c54138bd6b530b865ca4c5b3d4e18a Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 23 Mar 2022 18:44:21 -0400 Subject: [PATCH 7/7] Update test/parallel/test-runner-import-no-scheme.js --- test/parallel/test-runner-import-no-scheme.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js index ed15188599da82..45dd83d0251988 100644 --- a/test/parallel/test-runner-import-no-scheme.js +++ b/test/parallel/test-runner-import-no-scheme.js @@ -40,7 +40,7 @@ function test(argv) { } test(['-e', 'console.log(require("test"))']); -test(['-e', 'import("test").then(m=>console.log(m.default))']) +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)']);