module: exports & imports map invalid slash deprecation

PR-URL: https://github.com/nodejs/node/pull/44477
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
This commit is contained in:
Guy Bedford 2022-09-11 03:01:37 -07:00 committed by GitHub
parent 8bf7754538
commit 53633c033d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 176 additions and 65 deletions

View File

@ -3197,6 +3197,23 @@ Type: Documentation-only
The [`--trace-atomics-wait`][] flag is deprecated.
### DEP0166: Double slashes in imports and exports targets
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/44477
description: Documentation-only deprecation
with `--pending-deprecation` support.
-->
Type: Documentation-only (supports [`--pending-deprecation`][])
Package imports and exports targets mapping into paths including a double slash
(of _"/"_ or _"\\"_) are deprecated and will fail with a resolution validation
error in a future release. This same deprecation also applies to pattern matches
starting or ending in a slash.
[Legacy URL API]: url.md#legacy-url-api
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3

View File

@ -1357,8 +1357,7 @@ The resolver can throw the following errors:
> 1. Set _mainExport_ to _exports_\[_"."_].
> 4. If _mainExport_ is not **undefined**, then
> 1. Let _resolved_ be the result of **PACKAGE\_TARGET\_RESOLVE**(
> _packageURL_, _mainExport_, _""_, **false**, **false**,
> _conditions_).
> _packageURL_, _mainExport_, **null**, **false**, _conditions_).
> 2. If _resolved_ is not **null** or **undefined**, return _resolved_.
> 3. Otherwise, if _exports_ is an Object and all keys of _exports_ start with
> _"."_, then
@ -1389,7 +1388,7 @@ _isImports_, _conditions_)
> 1. If _matchKey_ is a key of _matchObj_ and does not contain _"\*"_, then
> 1. Let _target_ be the value of _matchObj_\[_matchKey_].
> 2. Return the result of **PACKAGE\_TARGET\_RESOLVE**(_packageURL_,
> _target_, _""_, **false**, _isImports_, _conditions_).
> _target_, **null**, _isImports_, _conditions_).
> 2. Let _expansionKeys_ be the list of keys of _matchObj_ containing only a
> single _"\*"_, sorted by the sorting function **PATTERN\_KEY\_COMPARE**
> which orders in descending order of specificity.
@ -1403,11 +1402,11 @@ _isImports_, _conditions_)
> _patternTrailer_ and the length of _matchKey_ is greater than or
> equal to the length of _expansionKey_, then
> 1. Let _target_ be the value of _matchObj_\[_expansionKey_].
> 2. Let _subpath_ be the substring of _matchKey_ starting at the
> 2. Let _patternMatch_ be the substring of _matchKey_ starting at the
> index of the length of _patternBase_ up to the length of
> _matchKey_ minus the length of _patternTrailer_.
> 3. Return the result of **PACKAGE\_TARGET\_RESOLVE**(_packageURL_,
> _target_, _subpath_, **true**, _isImports_, _conditions_).
> _target_, _patternMatch_, _isImports_, _conditions_).
> 4. Return **null**.
**PATTERN\_KEY\_COMPARE**(_keyA_, _keyB_)
@ -1426,37 +1425,32 @@ _isImports_, _conditions_)
> 10. If the length of _keyB_ is greater than the length of _keyA_, return 1.
> 11. Return 0.
**PACKAGE\_TARGET\_RESOLVE**(_packageURL_, _target_, _subpath_, _pattern_,
_internal_, _conditions_)
**PACKAGE\_TARGET\_RESOLVE**(_packageURL_, _target_, _patternMatch_,
_isImports_, _conditions_)
> 1. If _target_ is a String, then
> 1. If _pattern_ is **false**, _subpath_ has non-zero length and _target_
> does not end with _"/"_, throw an _Invalid Module Specifier_ error.
> 2. If _target_ does not start with _"./"_, then
> 1. If _internal_ is **true** and _target_ does not start with _"../"_ or
> _"/"_ and is not a valid URL, then
> 1. If _pattern_ is **true**, then
> 1. Return **PACKAGE\_RESOLVE**(_target_ with every instance of
> _"\*"_ replaced by _subpath_, _packageURL_ + _"/"_).
> 2. Return **PACKAGE\_RESOLVE**(_target_ + _subpath_,
> _packageURL_ + _"/"_).
> 2. Otherwise, throw an _Invalid Package Target_ error.
> 3. If _target_ split on _"/"_ or _"\\"_ contains any _"."_, _".."_, or
> _"node\_modules"_ segments after the first segment, case insensitive and
> including percent encoded variants, throw an _Invalid Package Target_
> error.
> 4. Let _resolvedTarget_ be the URL resolution of the concatenation of
> 1. If _target_ does not start with _"./"_, then
> 1. If _isImports_ is **false**, or if _target_ starts with _"../"_ or
> _"/"_, or if _target_ is a valid URL, then
> 1. Throw an _Invalid Package Target_ error.
> 2. If _patternMatch_ is a String, then
> 1. Return **PACKAGE\_RESOLVE**(_target_ with every instance of _"\*"_
> replaced by _patternMatch_, _packageURL_ + _"/"_).
> 3. Return **PACKAGE\_RESOLVE**(_target_, _packageURL_ + _"/"_).
> 2. If _target_ split on _"/"_ or _"\\"_ contains any _""_, _"."_, _".."_,
> or _"node\_modules"_ segments after the first _"."_ segment, case
> insensitive and including percent encoded variants, throw an _Invalid
> Package Target_ error.
> 3. Let _resolvedTarget_ be the URL resolution of the concatenation of
> _packageURL_ and _target_.
> 5. Assert: _resolvedTarget_ is contained in _packageURL_.
> 6. If _subpath_ split on _"/"_ or _"\\"_ contains any _"."_, _".."_, or
> _"node\_modules"_ segments, case insensitive and including percent
> encoded variants, throw an _Invalid Module Specifier_ error.
> 7. If _pattern_ is **true**, then
> 1. Return the URL resolution of _resolvedTarget_ with every instance of
> _"\*"_ replaced with _subpath_.
> 8. Otherwise,
> 1. Return the URL resolution of the concatenation of _subpath_ and
> _resolvedTarget_.
> 4. Assert: _resolvedTarget_ is contained in _packageURL_.
> 5. If _patternMatch_ is **null**, then
> 1. Return _resolvedTarget_.
> 6. If _patternMatch_ split on _"/"_ or _"\\"_ contains any _""_, _"."_,
> _".."_, or _"node\_modules"_ segments, case insensitive and including
> percent encoded variants, throw an _Invalid Module Specifier_ error.
> 7. Return the URL resolution of _resolvedTarget_ with every instance of
> _"\*"_ replaced with _patternMatch_.
> 2. Otherwise, if _target_ is a non-null Object, then
> 1. If _exports_ contains any index property keys, as defined in ECMA-262
> [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error.
@ -1465,7 +1459,7 @@ _internal_, _conditions_)
> then
> 1. Let _targetValue_ be the value of the _p_ property in _target_.
> 2. Let _resolved_ be the result of **PACKAGE\_TARGET\_RESOLVE**(
> _packageURL_, _targetValue_, _subpath_, _pattern_, _internal_,
> _packageURL_, _targetValue_, _patternMatch_, _isImports_,
> _conditions_).
> 3. If _resolved_ is equal to **undefined**, continue the loop.
> 4. Return _resolved_.
@ -1474,7 +1468,7 @@ _internal_, _conditions_)
> 1. If \_target.length is zero, return **null**.
> 2. For each item _targetValue_ in _target_, do
> 1. Let _resolved_ be the result of **PACKAGE\_TARGET\_RESOLVE**(
> _packageURL_, _targetValue_, _subpath_, _pattern_, _internal_,
> _packageURL_, _targetValue_, _patternMatch_, _isImports_,
> _conditions_), continuing the loop on any _Invalid Package Target_
> error.
> 2. If _resolved_ is **undefined**, continue the loop.

View File

@ -33,6 +33,7 @@ const {
Stats,
} = require('fs');
const { getOptionValue } = require('internal/options');
const pendingDeprecation = getOptionValue('--pending-deprecation');
// Do not eagerly grab .manifest, it may be in TDZ
const policy = getOptionValue('--experimental-policy') ?
require('internal/process/policy') :
@ -98,6 +99,23 @@ function emitTrailingSlashPatternDeprecation(match, pjsonUrl, base) {
);
}
const doubleSlashRegEx = /[/\\][/\\]/;
function emitInvalidSegmentDeprecation(target, request, match, pjsonUrl, base) {
if (!pendingDeprecation) { return; }
const pjsonPath = fileURLToPath(pjsonUrl);
const double = RegExpPrototypeExec(doubleSlashRegEx, target) !== null;
process.emitWarning(
`Use of deprecated ${double ? 'double slash' :
'leading or trailing slash matching'} resolving "${target}" for module ` +
`request "${request}" ${request !== match ? `matched to "${match}" ` : ''
}in the "exports" field module resolution of the package at ${pjsonPath}${
base ? ` imported from ${fileURLToPath(base)}` : ''}.`,
'DeprecationWarning',
'DEP0166'
);
}
/**
* @param {URL} url
* @param {URL} packageJSONUrl
@ -344,15 +362,17 @@ function throwExportsNotFound(subpath, packageJSONUrl, base) {
/**
*
* @param {string | URL} subpath
* @param {string} request
* @param {string} match
* @param {URL} packageJSONUrl
* @param {boolean} internal
* @param {string | URL | undefined} base
*/
function throwInvalidSubpath(subpath, packageJSONUrl, internal, base) {
const reason = `request is not a valid subpath for the "${internal ?
'imports' : 'exports'}" resolution of ${fileURLToPath(packageJSONUrl)}`;
throw new ERR_INVALID_MODULE_SPECIFIER(subpath, reason,
function throwInvalidSubpath(request, match, packageJSONUrl, internal, base) {
const reason = `request is not a valid match in pattern "${match}" for the "${
internal ? 'imports' : 'exports'}" resolution of ${
fileURLToPath(packageJSONUrl)}`;
throw new ERR_INVALID_MODULE_SPECIFIER(request, reason,
base && fileURLToPath(base));
}
@ -368,12 +388,22 @@ function throwInvalidPackageTarget(
internal, base && fileURLToPath(base));
}
const invalidSegmentRegEx = /(^|\\|\/)((\.|%2e)(\.|%2e)?|(n|%6e|%4e)(o|%6f|%4f)(d|%64|%44)(e|%65|%45)(_|%5f)(m|%6d|%4d)(o|%6f|%4f)(d|%64|%44)(u|%75|%55)(l|%6c|%4c)(e|%65|%45)(s|%73|%53))(\\|\/|$)/i;
const invalidSegmentRegEx = /(^|\\|\/)((\.|%2e)(\.|%2e)?|(n|%6e|%4e)(o|%6f|%4f)(d|%64|%44)(e|%65|%45)(_|%5f)(m|%6d|%4d)(o|%6f|%4f)(d|%64|%44)(u|%75|%55)(l|%6c|%4c)(e|%65|%45)(s|%73|%53))?(\\|\/|$)/i;
const deprecatedInvalidSegmentRegEx = /(^|\\|\/)((\.|%2e)(\.|%2e)?|(n|%6e|%4e)(o|%6f|%4f)(d|%64|%44)(e|%65|%45)(_|%5f)(m|%6d|%4d)(o|%6f|%4f)(d|%64|%44)(u|%75|%55)(l|%6c|%4c)(e|%65|%45)(s|%73|%53))(\\|\/|$)/i;
const invalidPackageNameRegEx = /^\.|%|\\/;
const patternRegEx = /\*/g;
function resolvePackageTargetString(
target, subpath, match, packageJSONUrl, base, pattern, internal, conditions) {
target,
subpath,
match,
packageJSONUrl,
base,
pattern,
internal,
isPathMap,
conditions,
) {
if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
@ -399,8 +429,21 @@ function resolvePackageTargetString(
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
}
if (RegExpPrototypeExec(invalidSegmentRegEx, StringPrototypeSlice(target, 2)) !== null)
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
if (RegExpPrototypeExec(invalidSegmentRegEx, StringPrototypeSlice(target, 2)) !== null) {
if (RegExpPrototypeExec(deprecatedInvalidSegmentRegEx, StringPrototypeSlice(target, 2)) === null) {
if (!isPathMap) {
const request = pattern ?
StringPrototypeReplace(match, '*', () => subpath) :
match + subpath;
const resolvedTarget = pattern ?
RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
target;
emitInvalidSegmentDeprecation(resolvedTarget, request, match, packageJSONUrl, base);
}
} else {
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
}
}
const resolved = new URL(target, packageJSONUrl);
const resolvedPath = resolved.pathname;
@ -412,18 +455,22 @@ function resolvePackageTargetString(
if (subpath === '') return resolved;
if (RegExpPrototypeExec(invalidSegmentRegEx, subpath) !== null) {
const request = pattern ?
StringPrototypeReplace(match, '*', () => subpath) : match + subpath;
throwInvalidSubpath(request, packageJSONUrl, internal, base);
const request = pattern ? StringPrototypeReplace(match, '*', () => subpath) : match + subpath;
if (RegExpPrototypeExec(deprecatedInvalidSegmentRegEx, subpath) === null) {
if (!isPathMap) {
const resolvedTarget = pattern ?
RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
target;
emitInvalidSegmentDeprecation(resolvedTarget, request, match, packageJSONUrl, base);
}
} else {
throwInvalidSubpath(request, match, packageJSONUrl, internal, base);
}
}
if (pattern) {
return new URL(
RegExpPrototypeSymbolReplace(
patternRegEx,
resolved.href,
() => subpath
)
RegExpPrototypeSymbolReplace(patternRegEx, resolved.href, () => subpath)
);
}
@ -441,11 +488,11 @@ function isArrayIndex(key) {
}
function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
base, pattern, internal, conditions) {
base, pattern, internal, isPathMap, conditions) {
if (typeof target === 'string') {
return resolvePackageTargetString(
target, subpath, packageSubpath, packageJSONUrl, base, pattern, internal,
conditions);
isPathMap, conditions);
} else if (ArrayIsArray(target)) {
if (target.length === 0) {
return null;
@ -458,7 +505,7 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
try {
resolveResult = resolvePackageTarget(
packageJSONUrl, targetItem, subpath, packageSubpath, base, pattern,
internal, conditions);
internal, isPathMap, conditions);
} catch (e) {
lastException = e;
if (e.code === 'ERR_INVALID_PACKAGE_TARGET') {
@ -494,7 +541,7 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
const conditionalTarget = target[key];
const resolveResult = resolvePackageTarget(
packageJSONUrl, conditionalTarget, subpath, packageSubpath, base,
pattern, internal, conditions);
pattern, internal, isPathMap, conditions);
if (resolveResult === undefined)
continue;
return resolveResult;
@ -557,7 +604,8 @@ function packageExportsResolve(
!StringPrototypeEndsWith(packageSubpath, '/')) {
const target = exports[packageSubpath];
const resolveResult = resolvePackageTarget(
packageJSONUrl, target, '', packageSubpath, base, false, false, conditions
packageJSONUrl, target, '', packageSubpath, base, false, false, false,
conditions
);
if (resolveResult == null) {
@ -608,6 +656,7 @@ function packageExportsResolve(
base,
true,
false,
StringPrototypeEndsWith(packageSubpath, '/'),
conditions);
if (resolveResult == null) {
@ -654,7 +703,8 @@ function packageImportsResolve(name, base, conditions) {
if (ObjectPrototypeHasOwnProperty(imports, name) &&
!StringPrototypeIncludes(name, '*')) {
const resolveResult = resolvePackageTarget(
packageJSONUrl, imports[name], '', name, base, false, true, conditions
packageJSONUrl, imports[name], '', name, base, false, true, false,
conditions
);
if (resolveResult != null) {
return resolveResult;
@ -687,7 +737,7 @@ function packageImportsResolve(name, base, conditions) {
const resolveResult = resolvePackageTarget(packageJSONUrl, target,
bestMatchSubpath,
bestMatch, base, true,
true, conditions);
true, false, conditions);
if (resolveResult != null) {
return resolveResult;
}

View File

@ -1,9 +1,22 @@
// Flags: --pending-deprecation
import { mustCall } from '../common/index.mjs';
import assert from 'assert';
let curWarning = 0;
const expectedWarnings = [
'Use of deprecated leading or trailing slash',
'Use of deprecated double slash',
'.//asdf.js',
'".//internal/test.js"',
'".//internal//test.js"',
'"./////internal/////test.js"',
'"./trailing-pattern-slash/"',
'"./subpath/dir1/dir1.js"',
'"./subpath//dir1/dir1.js"',
'.//asdf.js',
'".//internal/test.js"',
'".//internal//test.js"',
'"./////internal/////test.js"',
'no_exports',
'default_index',
];

View File

@ -41,6 +41,15 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
['pkgexports/a/b/dir1/dir1', { default: 'main' }],
// Deprecated:
// Double slashes:
['pkgexports/a//dir1/dir1', { default: 'main' }],
// double slash target
['pkgexports/doubleslash', { default: 'asdf' }],
// Null target with several slashes
['pkgexports/sub//internal/test.js', { default: 'internal only' }],
['pkgexports/sub//internal//test.js', { default: 'internal only' }],
['pkgexports/sub/////internal/////test.js', { default: 'internal only' }],
// trailing slash
['pkgexports/trailing-pattern-slash/',
{ default: 'trailing-pattern-slash' }],
]);
@ -74,7 +83,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
['pkgexports/invalid1', './invalid1'],
['pkgexports/invalid4', './invalid4'],
// Null mapping
['pkgexports/sub/internal/test.js', './sub/internal/test.js'],
['pkgexports/sub/internal//test.js', './sub/internal//test.js'],
['pkgexports/null', './null'],
['pkgexports//null', './/null'],
['pkgexports/////null', './////null'],
['pkgexports/null/subpath', './null/subpath'],
// Empty fallback
['pkgexports/nofallback1', './nofallback1'],
@ -133,7 +146,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
loadFixture(specifier).catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
assertStartsWith(err.message, 'Invalid module ');
assertIncludes(err.message, 'is not a valid subpath');
assertIncludes(err.message, 'is not a valid match in pattern');
assertIncludes(err.message, subpath);
}));
}

View File

@ -51,7 +51,7 @@ const { requireImport, importImport } = importer;
const invalidImportSpecifiers = new Map([
// Backtracking below the package base
['#subpath/sub/../../../belowbase', 'request is not a valid subpath'],
['#subpath/sub/../../../belowbase', 'request is not a valid match in pattern'],
// Percent-encoded slash errors
['#external/subpath/x%2Fy', 'must not include encoded "/" or "\\"'],
['#external/subpath/x%5Cy', 'must not include encoded "/" or "\\"'],
@ -79,10 +79,14 @@ const { requireImport, importImport } = importer;
'#missing',
// Explicit null import
'#null',
'#subpath/null',
// No condition match import
'#nullcondition',
// Null subpath shadowing
'#subpath/nullshadow/x',
// Null pattern
'#subpath/internal/test',
'#subpath/internal//test',
]);
for (const specifier of undefinedImports) {
@ -94,10 +98,20 @@ const { requireImport, importImport } = importer;
}
// Handle not found for the defined imports target not existing
loadFixture('#notfound').catch(mustCall((err) => {
strictEqual(err.code,
isRequire ? 'MODULE_NOT_FOUND' : 'ERR_MODULE_NOT_FOUND');
}));
const nonDefinedImports = new Set([
'#notfound',
'#subpath//null',
'#subpath/////null',
'#subpath//internal/test',
'#subpath//internal//test',
'#subpath/////internal/////test',
]);
for (const specifier of nonDefinedImports) {
loadFixture(specifier).catch(mustCall((err) => {
strictEqual(err.code,
isRequire ? 'MODULE_NOT_FOUND' : 'ERR_MODULE_NOT_FOUND');
}));
}
});
// CJS resolver must still support #package packages in node_modules

View File

@ -6,6 +6,8 @@
"require": "./requirebranch.js"
},
"#subpath/*": "./sub/*",
"#subpath/internal/*": null,
"#subpath/null": null,
"#subpath/*.asdf": "./test.js",
"#external": "pkgexports/valid-cjs",
"#external/subpath/*": "pkgexports/sub/*",

View File

@ -0,0 +1,3 @@
'use strict';
module.exports = 'internal only';

View File

@ -0,0 +1,3 @@
'use strict';
module.exports = 'internal only';

View File

@ -5,6 +5,7 @@
"./space": "./sp%20ce.js",
"./valid-cjs": "./asdf.js",
"./sub/*": "./*",
"./sub/internal/*": null,
"./belowdir/*": "../belowdir/*",
"./belowfile": "../belowfile",
"./null": null,
@ -19,6 +20,7 @@
"./nofallback1": [],
"./nofallback2": [null, {}, "builtin:x"],
"./nodemodules": "./node_modules/internalpkg/x.js",
"./doubleslash": ".//asdf.js",
"./no-addons": {
"node-addons": "./addons-entry.js",
"default": "./no-addons-entry.js"