diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index b0203d552503c7..bb0549d7ab4cea 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3182,6 +3182,23 @@ Type: Documentation-only The [`--trace-atomics-wait`][] flag is deprecated. +### DEP0166: Double slashes in imports and exports targets + + + +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 diff --git a/doc/api/esm.md b/doc/api/esm.md index 3a02f456a789d2..8360c6f821b25c 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -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 @@ -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. @@ -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_) @@ -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. @@ -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_. @@ -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. diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index a923508f2fd866..bfee280212fc4d 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -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; } diff --git a/test/es-module/test-esm-exports-deprecations.mjs b/test/es-module/test-esm-exports-deprecations.mjs index 5523af286bccfc..d4d8b8e3e379c0 100644 --- a/test/es-module/test-esm-exports-deprecations.mjs +++ b/test/es-module/test-esm-exports-deprecations.mjs @@ -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', ]; diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index c08fc9f8d9a54a..01432d92227e10 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -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); })); } diff --git a/test/es-module/test-esm-imports.mjs b/test/es-module/test-esm-imports.mjs index 77726c06f101b2..a42738646fbaa8 100644 --- a/test/es-module/test-esm-imports.mjs +++ b/test/es-module/test-esm-imports.mjs @@ -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 diff --git a/test/fixtures/es-modules/pkgimports/package.json b/test/fixtures/es-modules/pkgimports/package.json index 5b2f2a2ac5dd5f..6971e8b6fbcdea 100644 --- a/test/fixtures/es-modules/pkgimports/package.json +++ b/test/fixtures/es-modules/pkgimports/package.json @@ -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/*", diff --git a/test/fixtures/es-modules/pkgimports/sub/internal/test.js b/test/fixtures/es-modules/pkgimports/sub/internal/test.js new file mode 100644 index 00000000000000..bdf53dcd043e0d --- /dev/null +++ b/test/fixtures/es-modules/pkgimports/sub/internal/test.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'internal only'; diff --git a/test/fixtures/node_modules/pkgexports/internal/test.js b/test/fixtures/node_modules/pkgexports/internal/test.js new file mode 100644 index 00000000000000..bdf53dcd043e0d --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/internal/test.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'internal only'; diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index b686257875bc42..f68f1ac812f507 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -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"