Skip to content

Commit

Permalink
module: exports & imports map invalid slash deprecation
Browse files Browse the repository at this point in the history
PR-URL: #44477
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
  • Loading branch information
guybedford authored and RafaelGSS committed Sep 26, 2022
1 parent 993bd9b commit f8ec946
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 65 deletions.
17 changes: 17 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3182,6 +3182,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
Expand Down
64 changes: 29 additions & 35 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,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
Expand Down Expand Up @@ -1381,7 +1380,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.
Expand All @@ -1395,11 +1394,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_)
Expand All @@ -1418,37 +1417,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.
Expand All @@ -1457,7 +1451,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_.
Expand All @@ -1466,7 +1460,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.
Expand Down
98 changes: 74 additions & 24 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') :
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
}

Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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)
);
}

Expand All @@ -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;
Expand All @@ -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') {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -608,6 +656,7 @@ function packageExportsResolve(
base,
true,
false,
StringPrototypeEndsWith(packageSubpath, '/'),
conditions);

if (resolveResult == null) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions test/es-module/test-esm-exports-deprecations.mjs
Original file line number Diff line number Diff line change
@@ -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',
];
Expand Down
15 changes: 14 additions & 1 deletion test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
]);
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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);
}));
}
Expand Down
Loading

0 comments on commit f8ec946

Please sign in to comment.