module: exports & imports map invalid slash deprecation

PR-URL: #44477 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
nodejs · Sep 26, 2022 · b63315d · b63315d
1 parent 26f25c9
commit b63315d
Show file tree

Hide file tree

Showing 10 changed files with 176 additions and 65 deletions.
diff --git 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
+
+<!-- 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

diff --git 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
@@ -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
@@ -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
@@ -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);
     }));
   }