From 34dcb4a2caeaa1e20988b0a908d995b1b8f0a66e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 9 Feb 2021 12:27:32 +0100 Subject: [PATCH 1/3] module: disable package self resolution when name contains a colon There is a discrepancy between package self resolution in CJS and ESM contexts. This commit fixes this by forbidding self-resolution of packages whose names contain a colon. q --- doc/api/packages.md | 3 +- lib/internal/modules/cjs/loader.js | 2 + .../self_ref_module_with_colon/bin.js | 11 +++++ .../self_ref_module_with_colon/index.js | 4 ++ .../self_ref_module_with_colon/package.json | 4 ++ test/message/test-self-referential-package.js | 11 +++++ .../message/test-self-referential-package.out | 44 +++++++++++++++++++ 7 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/self_ref_module_with_colon/bin.js create mode 100644 test/fixtures/self_ref_module_with_colon/index.js create mode 100644 test/fixtures/self_ref_module_with_colon/package.json create mode 100644 test/message/test-self-referential-package.js create mode 100644 test/message/test-self-referential-package.out diff --git a/doc/api/packages.md b/doc/api/packages.md index 658e74af15551c..2055f54e387a1f 100644 --- a/doc/api/packages.md +++ b/doc/api/packages.md @@ -613,7 +613,8 @@ Then any module _in that package_ can reference an export in the package itself: import { something } from 'a-package'; // Imports "something" from ./main.mjs. ``` -Self-referencing is available only if `package.json` has [`"exports"`][], and +Self-referencing is available only if `package.json` has a [`"name"`][] that +does not contain the character `:`, and a [`"exports"`][] field, and will allow importing only what that [`"exports"`][] (in the `package.json`) allows. So the code below, given the previous package, will generate a runtime error: diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index e7298a2acd782e..89c1d71cd42e4c 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -54,6 +54,7 @@ const { StringPrototypeCharCodeAt, StringPrototypeEndsWith, StringPrototypeLastIndexOf, + StringPrototypeIncludes, StringPrototypeIndexOf, StringPrototypeMatch, StringPrototypeSlice, @@ -430,6 +431,7 @@ function trySelfParentPath(parent) { function trySelf(parentPath, request) { if (!parentPath) return false; + if (StringPrototypeIncludes(request, ':')) return false; const { data: pkg, path: pkgPath } = readPackageScope(parentPath) || {}; if (!pkg || pkg.exports === undefined) return false; diff --git a/test/fixtures/self_ref_module_with_colon/bin.js b/test/fixtures/self_ref_module_with_colon/bin.js new file mode 100644 index 00000000000000..80cbce6025ba3e --- /dev/null +++ b/test/fixtures/self_ref_module_with_colon/bin.js @@ -0,0 +1,11 @@ +#!/usr/bin/env node + +'use strict'; + +try { + console.log(require('file:package')); +} catch (e) { + console.log(e); +} +import('file:package').then(console.log, console.log); +import('file%3Apackage').then(console.log, console.log); diff --git a/test/fixtures/self_ref_module_with_colon/index.js b/test/fixtures/self_ref_module_with_colon/index.js new file mode 100644 index 00000000000000..7faa73693b54aa --- /dev/null +++ b/test/fixtures/self_ref_module_with_colon/index.js @@ -0,0 +1,4 @@ +'use strict' + +module.exports = 'Self resolution working'; + diff --git a/test/fixtures/self_ref_module_with_colon/package.json b/test/fixtures/self_ref_module_with_colon/package.json new file mode 100644 index 00000000000000..09d6bb8b347d89 --- /dev/null +++ b/test/fixtures/self_ref_module_with_colon/package.json @@ -0,0 +1,4 @@ +{ + "name": "file:package", + "exports": "./index.js" +} diff --git a/test/message/test-self-referential-package.js b/test/message/test-self-referential-package.js new file mode 100644 index 00000000000000..8a730b19492b80 --- /dev/null +++ b/test/message/test-self-referential-package.js @@ -0,0 +1,11 @@ +'use strict'; + +require('../common'); +const { spawn } = require('child_process'); + +const fixtures = require('../common/fixtures'); +const selfRefModule = fixtures.path('self_ref_module_with_colon'); + +spawn(process.argv0, [`${selfRefModule}/bin.js`], { + stdio: 'inherit' +}); diff --git a/test/message/test-self-referential-package.out b/test/message/test-self-referential-package.out new file mode 100644 index 00000000000000..7423de936668b1 --- /dev/null +++ b/test/message/test-self-referential-package.out @@ -0,0 +1,44 @@ +Error: Cannot find module 'file:package' +Require stack: +- */self_ref_module_with_colon/bin.js + at Function.Module._resolveFilename (node:internal/modules/cjs/loader:*) + at Function.Module._load (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) + at require (node:internal/modules/cjs/helpers:*) + at Object. (*/self_ref_module_with_colon/bin.js:*) + at Module._compile (node:internal/modules/cjs/loader:*) + at Object.Module._extensions..js (node:internal/modules/cjs/loader:*) + at Module.load (node:internal/modules/cjs/loader:*) + at Function.Module._load (node:internal/modules/cjs/loader:*) + at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*) { + code: 'MODULE_NOT_FOUND', + requireStack: [ + '*/self_ref_module_with_colon/bin.js' + ] +} +Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/package' imported from */self_ref_module_with_colon/bin.js + at new NodeError (node:internal/errors:*) + at finalizeResolution (node:internal/modules/esm/resolve:*) + at moduleResolve (node:internal/modules/esm/resolve:*) + at Loader.defaultResolve [as _resolve] (node:internal/modules/esm/resolve:*) + at Loader.resolve (node:internal/modules/esm/loader:*) + at Loader.getModuleJob (node:internal/modules/esm/loader:*) + at Loader.import (node:internal/modules/esm/loader:*) + at importModuleDynamically (node:internal/modules/cjs/loader:*) + at importModuleDynamicallyWrapper (node:internal/vm/module:*) + at importModuleDynamically (node:vm:*) { + code: 'ERR_MODULE_NOT_FOUND' +} +TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file%3Apackage" is not a valid package name imported from */self_ref_module_with_colon/bin.js + at new NodeError (node:internal/errors:*) + at parsePackageName (node:internal/modules/esm/resolve:*) + at packageResolve (node:internal/modules/esm/resolve:*) + at moduleResolve (node:internal/modules/esm/resolve:*) + at Loader.defaultResolve [as _resolve] (node:internal/modules/esm/resolve:*) + at Loader.resolve (node:internal/modules/esm/loader:*) + at Loader.getModuleJob (node:internal/modules/esm/loader:*) + at Loader.import (node:internal/modules/esm/loader:*) + at importModuleDynamically (node:internal/modules/cjs/loader:*) + at importModuleDynamicallyWrapper (node:internal/vm/module:*) { + code: 'ERR_INVALID_MODULE_SPECIFIER' +} From 42cddea0e3ee92978993a79d62fa16497f4a9ba2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 9 Feb 2021 15:07:44 +0100 Subject: [PATCH 2/3] fixup! module: disable package self resolution when name contains a colon --- lib/internal/modules/cjs/loader.js | 5 ++++- .../message/test-self-referential-package.out | 19 +++++++------------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 89c1d71cd42e4c..f6ebab9620b607 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -431,7 +431,6 @@ function trySelfParentPath(parent) { function trySelf(parentPath, request) { if (!parentPath) return false; - if (StringPrototypeIncludes(request, ':')) return false; const { data: pkg, path: pkgPath } = readPackageScope(parentPath) || {}; if (!pkg || pkg.exports === undefined) return false; @@ -901,6 +900,10 @@ Module._resolveFilename = function(request, parent, isMain, options) { const parentPath = trySelfParentPath(parent); const selfResolved = trySelf(parentPath, request); if (selfResolved) { + if (StringPrototypeIncludes(request, ':')) { + throw new ERR_INVALID_MODULE_SPECIFIER( + request, 'is not a valid package name', parentPath); + } const cacheKey = request + '\x00' + (paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00')); Module._pathCache[cacheKey] = selfResolved; diff --git a/test/message/test-self-referential-package.out b/test/message/test-self-referential-package.out index 7423de936668b1..b16e51bb7bb962 100644 --- a/test/message/test-self-referential-package.out +++ b/test/message/test-self-referential-package.out @@ -1,22 +1,17 @@ -Error: Cannot find module 'file:package' -Require stack: -- */self_ref_module_with_colon/bin.js +TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file:package" is not a valid package name imported from * + at new NodeError (node:internal/errors:*) at Function.Module._resolveFilename (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) at Module.require (node:internal/modules/cjs/loader:*) at require (node:internal/modules/cjs/helpers:*) - at Object. (*/self_ref_module_with_colon/bin.js:*) + at Object. (*) at Module._compile (node:internal/modules/cjs/loader:*) at Object.Module._extensions..js (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) - at Function.Module._load (node:internal/modules/cjs/loader:*) - at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*) { - code: 'MODULE_NOT_FOUND', - requireStack: [ - '*/self_ref_module_with_colon/bin.js' - ] + at Function.Module._load (node:internal/modules/cjs/loader:*) { + code: 'ERR_INVALID_MODULE_SPECIFIER' } -Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/package' imported from */self_ref_module_with_colon/bin.js +Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/package' imported from * at new NodeError (node:internal/errors:*) at finalizeResolution (node:internal/modules/esm/resolve:*) at moduleResolve (node:internal/modules/esm/resolve:*) @@ -29,7 +24,7 @@ Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/package' imported from */self at importModuleDynamically (node:vm:*) { code: 'ERR_MODULE_NOT_FOUND' } -TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file%3Apackage" is not a valid package name imported from */self_ref_module_with_colon/bin.js +TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file%%3Apackage" is not a valid package name imported from * at new NodeError (node:internal/errors:*) at parsePackageName (node:internal/modules/esm/resolve:*) at packageResolve (node:internal/modules/esm/resolve:*) From cdde0fcd0f8b987b74c5de987f59fa9a82e94e6e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 9 Feb 2021 16:03:50 +0100 Subject: [PATCH 3/3] fixup! module: disable package self resolution when name contains a colon --- lib/internal/modules/cjs/loader.js | 9 +++++---- test/message/test-self-referential-package.out | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index f6ebab9620b607..250e88f544fcfe 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -445,6 +445,11 @@ function trySelf(parentPath, request) { return false; } + if (StringPrototypeIncludes(pkg.name, ':')) { + throw new ERR_INVALID_MODULE_SPECIFIER( + request, 'is not a valid package name', parentPath); + } + try { return finalizeEsmResolution(packageExportsResolve( pathToFileURL(pkgPath + '/package.json'), expansion, pkg, @@ -900,10 +905,6 @@ Module._resolveFilename = function(request, parent, isMain, options) { const parentPath = trySelfParentPath(parent); const selfResolved = trySelf(parentPath, request); if (selfResolved) { - if (StringPrototypeIncludes(request, ':')) { - throw new ERR_INVALID_MODULE_SPECIFIER( - request, 'is not a valid package name', parentPath); - } const cacheKey = request + '\x00' + (paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00')); Module._pathCache[cacheKey] = selfResolved; diff --git a/test/message/test-self-referential-package.out b/test/message/test-self-referential-package.out index b16e51bb7bb962..b8a9dde4eeb6b7 100644 --- a/test/message/test-self-referential-package.out +++ b/test/message/test-self-referential-package.out @@ -1,5 +1,6 @@ TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file:package" is not a valid package name imported from * at new NodeError (node:internal/errors:*) + at trySelf (node:internal/modules/cjs/loader:*) at Function.Module._resolveFilename (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) at Module.require (node:internal/modules/cjs/loader:*) @@ -7,8 +8,7 @@ TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "file:package" is not a at Object. (*) at Module._compile (node:internal/modules/cjs/loader:*) at Object.Module._extensions..js (node:internal/modules/cjs/loader:*) - at Module.load (node:internal/modules/cjs/loader:*) - at Function.Module._load (node:internal/modules/cjs/loader:*) { + at Module.load (node:internal/modules/cjs/loader:*) { code: 'ERR_INVALID_MODULE_SPECIFIER' } Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/package' imported from *