Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: exports & imports invalid slash deprecation #44477

Merged
merged 10 commits into from
Sep 11, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3194,6 +3194,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 @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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_)
Expand All @@ -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.
Expand All @@ -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_.
Expand All @@ -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.
Expand Down
99 changes: 77 additions & 22 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
guybedford marked this conversation as resolved.
Show resolved Hide resolved
) {

if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
Expand All @@ -399,8 +429,24 @@ 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 @@ -414,16 +460,22 @@ function resolvePackageTargetString(
if (RegExpPrototypeExec(invalidSegmentRegEx, subpath) !== null) {
const request = pattern ?
StringPrototypeReplace(match, '*', () => subpath) : match + subpath;
throwInvalidSubpath(request, packageJSONUrl, internal, base);
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);
}
guybedford marked this conversation as resolved.
Show resolved Hide resolved
}

if (pattern) {
return new URL(
RegExpPrototypeSymbolReplace(
patternRegEx,
resolved.href,
() => subpath
)
RegExpPrototypeSymbolReplace(patternRegEx, resolved.href, () => subpath)
);
}

Expand All @@ -441,11 +493,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 +510,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 +546,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 +609,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 +661,7 @@ function packageExportsResolve(
base,
true,
false,
StringPrototypeEndsWith(packageSubpath, '/'),
conditions);

if (resolveResult == null) {
Expand Down Expand Up @@ -654,7 +708,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 +742,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
Loading