module: pkg exports validations and fallbacks

PR-URL: https://github.com/nodejs/node/pull/28949
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>

10 files changed, 418 insertions, 125 deletions

diff --git a/doc/api/esm.md b/doc/api/esm.md
index 79da763e8f..17d98ab208 100644
--- a/doc/api/esm.md
+++ b/doc/api/esm.md
@@ -242,13 +242,13 @@ throw when an attempt is made to import them:
 
 ```js
 import submodule from 'es-module-package/private-module.js';
-// Throws - Package exports error
+// Throws - Module not found
 ```
 
 > Note: this is not a strong encapsulation as any private modules can still be
 > loaded by absolute paths.
 
-Folders can also be mapped with package exports as well:
+Folders can also be mapped with package exports:
 
 <!-- eslint-skip -->
 ```js
@@ -268,8 +268,24 @@ import feature from 'es-module-package/features/x.js';
 If a package has no exports, setting `"exports": false` can be used instead of
 `"exports": {}` to indicate the package does not intend for submodules to be
 exposed.
-This is just a convention that works because `false`, just like `{}`, has no
-iterable own properties.
+
+Any invalid exports entries will be ignored. This includes exports not
+starting with `"./"` or a missing trailing `"/"` for directory exports.
+
+Array fallback support is provided for exports, similarly to import maps
+in order to be forward-compatible with fallback workflows in future:
+
+<!-- eslint-skip -->
+```js
+{
+  "exports": {
+    "./submodule": ["not:valid", "./submodule.js"]
+  }
+}
+```
+
+Since `"not:valid"` is not a supported target, `"./submodule.js"` is used
+instead as the fallback, as if it were the only target.
 
 ## <code>import</code> Specifiers
 
@@ -660,7 +676,7 @@ CommonJS loader. Additional formats such as _"addon"_ can be extended in future
 updates.
 
 In the following algorithms, all subroutine errors are propagated as errors
-of these top-level routines.
+of these top-level routines unless stated otherwise.
 
 _isMain_ is **true** when resolving the Node.js application entry point.
 
@@ -681,6 +697,9 @@ _isMain_ is **true** when resolving the Node.js application entry point.
 >    1. Note: _specifier_ is now a bare specifier.
 >    1. Set _resolvedURL_ the result of
 >       **PACKAGE_RESOLVE**(_specifier_, _parentURL_).
+> 1. If _resolvedURL_ contains any percent encodings of _"/"_ or _"\\"_ (_"%2f"_
+>    and _"%5C"_ respectively), then
+>    1. Throw an _Invalid Specifier_ error.
 > 1. If the file at _resolvedURL_ does not exist, then
 >    1. Throw a _Module Not Found_ error.
 > 1. Set _resolvedURL_ to the real path of _resolvedURL_.
@@ -737,7 +756,7 @@ _isMain_ is **true** when resolving the Node.js application entry point.
 > 1. If _pjson_ is **null**, then
 >    1. Throw a _Module Not Found_ error.
 > 1. If _pjson.main_ is a String, then
->    1. Let _resolvedMain_ be the concatenation of _packageURL_, "/", and
+>    1. Let _resolvedMain_ be the URL resolution of _packageURL_, "/", and
 >       _pjson.main_.
 >    1. If the file at _resolvedMain_ exists, then
 >       1. Return _resolvedMain_.
@@ -746,8 +765,6 @@ _isMain_ is **true** when resolving the Node.js application entry point.
 > 1. Let _legacyMainURL_ be the result applying the legacy
 >    **LOAD_AS_DIRECTORY** CommonJS resolver to _packageURL_, throwing a
 >    _Module Not Found_ error for no resolution.
-> 1. If _legacyMainURL_ does not end in _".js"_ then,
->    1. Throw an _Unsupported File Extension_ error.
 > 1. Return _legacyMainURL_.
 
 **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _packagePath_, _exports_)
@@ -755,19 +772,42 @@ _isMain_ is **true** when resolving the Node.js application entry point.
 >    1. Set _packagePath_ to _"./"_ concatenated with _packagePath_.
 >    1. If _packagePath_ is a key of _exports_, then
 >       1. Let _target_ be the value of _exports[packagePath]_.
->       1. If _target_ is not a String, continue the loop.
->       1. Return the URL resolution of the concatenation of _packageURL_ and
->          _target_.
+>       1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_,
+>          _""_).
 >    1. Let _directoryKeys_ be the list of keys of _exports_ ending in
 >       _"/"_, sorted by length descending.
 >    1. For each key _directory_ in _directoryKeys_, do
 >       1. If _packagePath_ starts with _directory_, then
 >          1. Let _target_ be the value of _exports[directory]_.
->          1. If _target_ is not a String, continue the loop.
 >          1. Let _subpath_ be the substring of _target_ starting at the index
 >             of the length of _directory_.
->          1. Return the URL resolution of the concatenation of _packageURL_,
->             _target_ and _subpath_.
+>          1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_,
+>             _subpath_).
+> 1. Throw a _Module Not Found_ error.
+
+**PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_)
+> 1. If _target_ is a String, then
+>    1. If _target_ does not start with _"./"_, throw a _Module Not Found_
+>       error.
+>    1. If _subpath_ has non-zero length and _target_ does not end with _"/"_,
+>       throw a _Module Not Found_ error.
+>    1. If _target_ or _subpath_ contain any _"node_modules"_ segments including
+>       _"node_modules"_ percent-encoding, throw a _Module Not Found_ error.
+>    1. Let _resolvedTarget_ be the URL resolution of the concatenation of
+>       _packageURL_ and _target_.
+>    1. If _resolvedTarget_ is contained in _packageURL_, then
+>       1. Let _resolved_ be the URL resolution of the concatenation of
+>          _subpath_ and _resolvedTarget_.
+>       1. If _resolved_ is contained in _resolvedTarget_, then
+>          1. Return _resolved_.
+> 1. Otherwise, if _target_ is an Array, then
+>    1. For each item _targetValue_ in _target_, do
+>       1. If _targetValue_ is not a String, continue the loop.
+>       1. Let _resolved_ be the result of
+>          **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _targetValue_,
+>          _subpath_), continuing the loop on abrupt completion.
+>       1. Assert: _resolved_ is a String.
+>       1. Return _resolved_.
 > 1. Throw a _Module Not Found_ error.
 
 **ESM_FORMAT**(_url_, _isMain_)
@@ -790,6 +830,7 @@ _isMain_ is **true** when resolving the Node.js application entry point.
 **READ_PACKAGE_SCOPE**(_url_)
 > 1. Let _scopeURL_ be _url_.
 > 1. While _scopeURL_ is not the file system root,
+>    1. If _scopeURL_ ends in a _"node_modules"_ path segment, return **null**.
 >    1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_scopeURL_).
 >    1. If _pjson_ is not **null**, then
 >       1. Return _pjson_.
diff --git a/doc/api/modules.md b/doc/api/modules.md
index bf8209965e..7197ef6ae2 100644
--- a/doc/api/modules.md
+++ b/doc/api/modules.md
@@ -202,11 +202,12 @@ NODE_MODULES_PATHS(START)
 5. return DIRS
 ```
 
-If `--experimental-exports` is enabled,
-node allows packages loaded via `LOAD_NODE_MODULES` to explicitly declare
-which filepaths to expose and how they should be interpreted.
-This expands on the control packages already had using the `main` field.
-With this feature enabled, the `LOAD_NODE_MODULES` changes as follows:
+If `--experimental-exports` is enabled, Node.js allows packages loaded via
+`LOAD_NODE_MODULES` to explicitly declare which file paths to expose and how
+they should be interpreted. This expands on the control packages already had
+using the `main` field.
+
+With this feature enabled, the `LOAD_NODE_MODULES` changes are:
 
 ```txt
 LOAD_NODE_MODULES(X, START)
@@ -224,10 +225,10 @@ RESOLVE_BARE_SPECIFIER(DIR, X)
    b. If "exports" is null or undefined, GOTO 3.
    c. Find the longest key in "exports" that the subpath starts with.
    d. If no such key can be found, throw "not found".
-   e. If the key matches the subpath entirely, return DIR/name/${exports[key]}.
-   f. If either the key or exports[key] do not end with a slash (`/`),
-      throw "not found".
-   g. Return DIR/name/${exports[key]}${subpath.slice(key.length)}.
+   e. let RESOLVED_URL =
+        PACKAGE_EXPORTS_TARGET_RESOLVE(pathToFileURL(DIR/name), exports[key],
+        subpath.slice(key.length)), as defined in the esm resolver.
+   f. return fileURLToPath(RESOLVED_URL)
 3. return DIR/X
 ```
 
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 9332c6c277..5d5767e0cf 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -24,6 +24,7 @@
 const {
   JSON,
   Object,
+  ObjectPrototype,
   Reflect,
   SafeMap,
   StringPrototype,
@@ -348,18 +349,18 @@ function resolveExports(nmPath, request, absoluteRequest) {
 
     const basePath = path.resolve(nmPath, name);
     const pkgExports = readExports(basePath);
+    const mappingKey = `.${expansion}`;
 
-    if (pkgExports != null) {
-      const mappingKey = `.${expansion}`;
-      const mapping = pkgExports[mappingKey];
-      if (typeof mapping === 'string') {
-        return fileURLToPath(new URL(mapping, `${pathToFileURL(basePath)}/`));
+    if (typeof pkgExports === 'object' && pkgExports !== null) {
+      if (ObjectPrototype.hasOwnProperty(pkgExports, mappingKey)) {
+        const mapping = pkgExports[mappingKey];
+        return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, '',
+                                    basePath, mappingKey);
       }
 
       let dirMatch = '';
-      for (const [candidateKey, candidateValue] of Object.entries(pkgExports)) {
+      for (const candidateKey of Object.keys(pkgExports)) {
         if (candidateKey[candidateKey.length - 1] !== '/') continue;
-        if (candidateValue[candidateValue.length - 1] !== '/') continue;
         if (candidateKey.length > dirMatch.length &&
             StringPrototype.startsWith(mappingKey, candidateKey)) {
           dirMatch = candidateKey;
@@ -367,15 +368,13 @@ function resolveExports(nmPath, request, absoluteRequest) {
       }
 
       if (dirMatch !== '') {
-        const dirMapping = pkgExports[dirMatch];
-        const remainder = StringPrototype.slice(mappingKey, dirMatch.length);
-        const expectedPrefix =
-          new URL(dirMapping, `${pathToFileURL(basePath)}/`);
-        const resolved = new URL(remainder, expectedPrefix).href;
-        if (StringPrototype.startsWith(resolved, expectedPrefix.href)) {
-          return fileURLToPath(resolved);
-        }
+        const mapping = pkgExports[dirMatch];
+        const subpath = StringPrototype.slice(mappingKey, dirMatch.length);
+        return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping,
+                                    subpath, basePath, mappingKey);
       }
+    }
+    if (pkgExports != null) {
       // eslint-disable-next-line no-restricted-syntax
       const e = new Error(`Package exports for '${basePath}' do not define ` +
           `a '${mappingKey}' subpath`);
@@ -387,6 +386,43 @@ function resolveExports(nmPath, request, absoluteRequest) {
   return path.resolve(nmPath, request);
 }
 
+function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
+  if (typeof target === 'string') {
+    if (target.startsWith('./') &&
+        (subpath.length === 0 || target.endsWith('/'))) {
+      const resolvedTarget = new URL(target, pkgPath);
+      const pkgPathPath = pkgPath.pathname;
+      const resolvedTargetPath = resolvedTarget.pathname;
+      if (StringPrototype.startsWith(resolvedTargetPath, pkgPathPath) &&
+          StringPrototype.indexOf(resolvedTargetPath, '/node_modules/',
+                                  pkgPathPath.length - 1) === -1) {
+        const resolved = new URL(subpath, resolvedTarget);
+        const resolvedPath = resolved.pathname;
+        if (StringPrototype.startsWith(resolvedPath, resolvedTargetPath) &&
+            StringPrototype.indexOf(resolvedPath, '/node_modules/',
+                                    pkgPathPath.length - 1) === -1) {
+          return fileURLToPath(resolved);
+        }
+      }
+    }
+  } else if (Array.isArray(target)) {
+    for (const targetValue of target) {
+      if (typeof targetValue !== 'string') continue;
+      try {
+        return resolveExportsTarget(pkgPath, targetValue, subpath, basePath,
+                                    mappingKey);
+      } catch (e) {
+        if (e.code !== 'MODULE_NOT_FOUND') throw e;
+      }
+    }
+  }
+  // eslint-disable-next-line no-restricted-syntax
+  const e = new Error(`Package exports for '${basePath}' do not define a ` +
+      `valid '${mappingKey}' target${subpath ? 'for ' + subpath : ''}`);
+  e.code = 'MODULE_NOT_FOUND';
+  throw e;
+}
+
 Module._findPath = function(request, paths, isMain) {
   const absoluteRequest = path.isAbsolute(request);
   if (absoluteRequest) {
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index b3d0c306c9..bae5e202dc 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -545,8 +545,8 @@ Maybe<const PackageConfig*> GetPackageConfig(Environment* env,
   if (existing != env->package_json_cache.end()) {
     const PackageConfig* pcfg = &existing->second;
     if (pcfg->is_valid == IsValid::No) {
-      std::string msg = "Invalid JSON in '" + path +
-        "' imported from " + base.ToFilePath();
+      std::string msg = "Invalid JSON in " + path +
+        " imported from " + base.ToFilePath();
       node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
       return Nothing<const PackageConfig*>();
     }
@@ -579,8 +579,8 @@ Maybe<const PackageConfig*> GetPackageConfig(Environment* env,
       env->package_json_cache.emplace(path,
           PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "",
                           PackageType::None, Global<Value>() });
-      std::string msg = "Invalid JSON in '" + path +
-          "' imported from " + base.ToFilePath();
+      std::string msg = "Invalid JSON in " + path +
+          " imported from " + base.ToFilePath();
       node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
       return Nothing<const PackageConfig*>();
     }
@@ -633,6 +633,12 @@ Maybe<const PackageConfig*> GetPackageScopeConfig(Environment* env,
                                                   const URL& base) {
   URL pjson_url("./package.json", &resolved);
   while (true) {
+    std::string pjson_url_path = pjson_url.path();
+    if (pjson_url_path.length() > 25 &&
+        pjson_url_path.substr(pjson_url_path.length() - 25, 25) ==
+        "node_modules/package.json") {
+      break;
+    }
     Maybe<const PackageConfig*> pkg_cfg =
         GetPackageConfig(env, pjson_url.ToFilePath(), base);
     if (pkg_cfg.IsNothing()) return pkg_cfg;
@@ -643,14 +649,13 @@ Maybe<const PackageConfig*> GetPackageScopeConfig(Environment* env,
 
     // Terminates at root where ../package.json equals ../../package.json
     // (can't just check "/package.json" for Windows support).
-    if (pjson_url.path() == last_pjson_url.path()) {
-      auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(),
-          PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "",
-                          PackageType::None, Global<Value>() });
-      const PackageConfig* pcfg = &entry.first->second;
-      return Just(pcfg);
-    }
+    if (pjson_url.path() == last_pjson_url.path()) break;
   }
+  auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(),
+  PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "",
+                  PackageType::None, Global<Value>() });
+  const PackageConfig* pcfg = &entry.first->second;
+  return Just(pcfg);
 }
 
 /*
@@ -750,16 +755,17 @@ Maybe<URL> FinalizeResolution(Environment* env,
     if (!file.IsNothing()) {
       return file;
     }
-    std::string msg = "Cannot find module '" + resolved.path() +
-        "' imported from " + base.ToFilePath();
+    std::string msg = "Cannot find module " + resolved.path() +
+        " imported from " + base.ToFilePath();
     node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
     return Nothing<URL>();
   }
 
   const std::string& path = resolved.ToFilePath();
   if (CheckDescriptorAtPath(path) != FILE) {
-    std::string msg = "Cannot find module '" + path +
-        "' imported from " + base.ToFilePath();
+    std::string msg = "Cannot find module " +
+        (path.length() != 0 ? path : resolved.path()) +
+        " imported from " + base.ToFilePath();
     node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
     return Nothing<URL>();
   }
@@ -793,13 +799,94 @@ Maybe<URL> PackageMainResolve(Environment* env,
       }
     }
   }
-  std::string msg = "Cannot find main entry point for '" +
-      URL(".", pjson_url).ToFilePath() + "' imported from " +
+  std::string msg = "Cannot find main entry point for " +
+      URL(".", pjson_url).ToFilePath() + " imported from " +
       base.ToFilePath();
   node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
   return Nothing<URL>();
 }
 
+void ThrowExportsNotFound(Environment* env,
+                          const std::string& subpath,
+                          const URL& pjson_url,
+                          const URL& base) {
+  const std::string msg = "Package exports for " +
+      pjson_url.ToFilePath() + " do not define a '" + subpath +
+      "' subpath, imported from " + base.ToFilePath();
+  node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
+}
+
+void ThrowExportsInvalid(Environment* env,
+                         const std::string& subpath,
+                         const std::string& target,
+                         const URL& pjson_url,
+                         const URL& base) {
+  const std::string msg = "Cannot resolve package exports target '" + target +
+      "' matched for '" + subpath + "' in " + pjson_url.ToFilePath() +
+      ", imported from " + base.ToFilePath();
+  node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
+}
+
+void ThrowExportsInvalid(Environment* env,
+                         const std::string& subpath,
+                         Local<Value> target,
+                         const URL& pjson_url,
+                         const URL& base) {
+  Local<String> target_string;
+  if (target->ToString(env->context()).ToLocal(&target_string)) {
+    Utf8Value target_utf8(env->isolate(), target_string);
+    std::string target_str(*target_utf8, target_utf8.length());
+    if (target->IsArray()) {
+      target_str = '[' + target_str + ']';
+    }
+    ThrowExportsInvalid(env, subpath, target_str, pjson_url, base);
+  }
+}
+
+Maybe<URL> ResolveExportsTarget(Environment* env,
+                                const std::string& target,
+                                const std::string& subpath,
+                                const std::string& match,
+                                const URL& pjson_url,
+                                const URL& base,
+                                bool throw_invalid = true) {
+  if (target.substr(0, 2) != "./") {
+    if (throw_invalid) {
+      ThrowExportsInvalid(env, match, target, pjson_url, base);
+    }
+    return Nothing<URL>();
+  }
+  if (subpath.length() > 0 && target.back() != '/') {
+    if (throw_invalid) {
+      ThrowExportsInvalid(env, match, target, pjson_url, base);
+    }
+    return Nothing<URL>();
+  }
+  URL resolved(target, pjson_url);
+  std::string resolved_path = resolved.path();
+  std::string pkg_path = URL(".", pjson_url).path();
+  if (resolved_path.find(pkg_path) != 0 ||
+      resolved_path.find("/node_modules/", pkg_path.length() - 1) !=
+      std::string::npos) {
+    if (throw_invalid) {
+      ThrowExportsInvalid(env, match, target, pjson_url, base);
+    }
+    return Nothing<URL>();
+  }
+  if (subpath.length() == 0) return Just(resolved);
+  URL subpath_resolved(subpath, resolved);
+  std::string subpath_resolved_path = subpath_resolved.path();
+  if (subpath_resolved_path.find(resolved_path) != 0 ||
+      subpath_resolved_path.find("/node_modules/", pkg_path.length() - 1)
+      != std::string::npos) {
+    if (throw_invalid) {
+      ThrowExportsInvalid(env, match, target + subpath, pjson_url, base);
+    }
+    return Nothing<URL>();
+  }
+  return Just(subpath_resolved);
+}
+
 Maybe<URL> PackageExportsResolve(Environment* env,
                                  const URL& pjson_url,
                                  const std::string& pkg_subpath,
@@ -809,57 +896,126 @@ Maybe<URL> PackageExportsResolve(Environment* env,
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
   Local<Value> exports = pcfg.exports.Get(isolate);
-  if (exports->IsObject()) {
-    Local<Object> exports_obj = exports.As<Object>();
-    Local<String> subpath = String::NewFromUtf8(isolate,
-        pkg_subpath.c_str(), v8::NewStringType::kNormal).ToLocalChecked();
+  if (!exports->IsObject()) {
+    ThrowExportsNotFound(env, pkg_subpath, pjson_url, base);
+    return Nothing<URL>();
+  }
+  Local<Object> exports_obj = exports.As<Object>();
+  Local<String> subpath = String::NewFromUtf8(isolate,
+      pkg_subpath.c_str(), v8::NewStringType::kNormal).ToLocalChecked();
 
-    auto target = exports_obj->Get(context, subpath).ToLocalChecked();
+  if (exports_obj->HasOwnProperty(context, subpath).FromJust()) {
+    Local<Value> target = exports_obj->Get(context, subpath).ToLocalChecked();
     if (target->IsString()) {
       Utf8Value target_utf8(isolate, target.As<v8::String>());
-      std::string target(*target_utf8, target_utf8.length());
-      if (target.substr(0, 2) == "./") {
-        URL target_url(target, pjson_url);
-        return FinalizeResolution(env, target_url, base);
+      std::string target_str(*target_utf8, target_utf8.length());
+      Maybe<URL> resolved = ResolveExportsTarget(env, target_str, "",
+          pkg_subpath, pjson_url, base);
+      if (resolved.IsNothing()) {
+        ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+        return Nothing<URL>();
       }
+      return FinalizeResolution(env, resolved.FromJust(), base);
+    } else if (target->IsArray()) {
+      Local<Array> target_arr = target.As<Array>();
+      const uint32_t length = target_arr->Length();
+      if (length == 0) {
+        ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+        return Nothing<URL>();
+      }
+      for (uint32_t i = 0; i < length; i++) {
+        auto target_item = target_arr->Get(context, i).ToLocalChecked();
+        if (target_item->IsString()) {
+          Utf8Value target_utf8(isolate, target_item.As<v8::String>());
+          std::string target(*target_utf8, target_utf8.length());
+          Maybe<URL> resolved = ResolveExportsTarget(env, target, "",
+              pkg_subpath, pjson_url, base, false);
+          if (resolved.IsNothing()) continue;
+          return FinalizeResolution(env, resolved.FromJust(), base);
+        }
+      }
+      auto invalid = target_arr->Get(context, length - 1).ToLocalChecked();
+      if (!invalid->IsString()) {
+        ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base);
+        return Nothing<URL>();
+      }
+      Utf8Value invalid_utf8(isolate, invalid.As<v8::String>());
+      std::string invalid_str(*invalid_utf8, invalid_utf8.length());
+      Maybe<URL> resolved = ResolveExportsTarget(env, invalid_str, "",
+                                                 pkg_subpath, pjson_url, base);
+      CHECK(resolved.IsNothing());
+      return Nothing<URL>();
+    } else {
+      ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+      return Nothing<URL>();
     }
+  }
 
-    Local<String> best_match;
-    std::string best_match_str = "";
-    Local<Array> keys =
-        exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
-    for (uint32_t i = 0; i < keys->Length(); ++i) {
-      Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
-      Utf8Value key_utf8(isolate, key);
-      std::string key_str(*key_utf8, key_utf8.length());
-      if (key_str.back() != '/') continue;
-      if (pkg_subpath.substr(0, key_str.length()) == key_str &&
-          key_str.length() > best_match_str.length()) {
-        best_match = key;
-        best_match_str = key_str;
-      }
+  Local<String> best_match;
+  std::string best_match_str = "";
+  Local<Array> keys =
+      exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
+  for (uint32_t i = 0; i < keys->Length(); ++i) {
+    Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
+    Utf8Value key_utf8(isolate, key);
+    std::string key_str(*key_utf8, key_utf8.length());
+    if (key_str.back() != '/') continue;
+    if (pkg_subpath.substr(0, key_str.length()) == key_str &&
+        key_str.length() > best_match_str.length()) {
+      best_match = key;
+      best_match_str = key_str;
     }
+  }
 
-    if (best_match_str.length() > 0) {
-      auto target = exports_obj->Get(context, best_match).ToLocalChecked();
-      if (target->IsString()) {
-        Utf8Value target_utf8(isolate, target.As<v8::String>());
-        std::string target(*target_utf8, target_utf8.length());
-        if (target.back() == '/' && target.substr(0, 2) == "./") {
-          std::string subpath = pkg_subpath.substr(best_match_str.length());
-          URL url_prefix(target, pjson_url);
-          URL target_url(subpath, url_prefix);
-          if (target_url.path().find(url_prefix.path()) == 0) {
-            return FinalizeResolution(env, target_url, base);
-          }
+  if (best_match_str.length() > 0) {
+    auto target = exports_obj->Get(context, best_match).ToLocalChecked();
+    std::string subpath = pkg_subpath.substr(best_match_str.length());
+    if (target->IsString()) {
+      Utf8Value target_utf8(isolate, target.As<v8::String>());
+      std::string target(*target_utf8, target_utf8.length());
+      Maybe<URL> resolved = ResolveExportsTarget(env, target, subpath,
+          pkg_subpath, pjson_url, base);
+      if (resolved.IsNothing()) {
+        ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+        return Nothing<URL>();
+      }
+      return FinalizeResolution(env, URL(subpath, resolved.FromJust()), base);
+    } else if (target->IsArray()) {
+      Local<Array> target_arr = target.As<Array>();
+      const uint32_t length = target_arr->Length();
+      if (length == 0) {
+        ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+        return Nothing<URL>();
+      }
+      for (uint32_t i = 0; i < length; i++) {
+        auto target_item = target_arr->Get(context, i).ToLocalChecked();
+        if (target_item->IsString()) {
+          Utf8Value target_utf8(isolate, target_item.As<v8::String>());
+          std::string target_str(*target_utf8, target_utf8.length());
+          Maybe<URL> resolved = ResolveExportsTarget(env, target_str, subpath,
+              pkg_subpath, pjson_url, base, false);
+          if (resolved.IsNothing()) continue;
+          return FinalizeResolution(env, resolved.FromJust(), base);
         }
       }
+      auto invalid = target_arr->Get(context, length - 1).ToLocalChecked();
+      if (!invalid->IsString()) {
+        ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base);
+        return Nothing<URL>();
+      }
+      Utf8Value invalid_utf8(isolate, invalid.As<v8::String>());
+      std::string invalid_str(*invalid_utf8, invalid_utf8.length());
+      Maybe<URL> resolved = ResolveExportsTarget(env, invalid_str, subpath,
+                                                 pkg_subpath, pjson_url, base);
+      CHECK(resolved.IsNothing());
+      return Nothing<URL>();
+    } else {
+      ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base);
+      return Nothing<URL>();
     }
   }
-  std::string msg = "Package exports for '" +
-      URL(".", pjson_url).ToFilePath() + "' do not define a '" + pkg_subpath +
-      "' subpath, imported from " + base.ToFilePath();
-  node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
+
+  ThrowExportsNotFound(env, pkg_subpath, pjson_url, base);
   return Nothing<URL>();
 }
 
diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs
index 6c42d3b64e..4f77252158 100644
--- a/test/es-module/test-esm-exports.mjs
+++ b/test/es-module/test-esm-exports.mjs
@@ -17,6 +17,9 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
     ['pkgexports/space', { default: 'encoded path' }],
     // Verifying that normal packages still work with exports turned on.
     isRequire ? ['baz/index', { default: 'eye catcher' }] : [null],
+    // Fallbacks
+    ['pkgexports/fallbackdir/asdf.js', { default: 'asdf' }],
+    ['pkgexports/fallbackfile', { default: 'asdf' }],
   ]);
   for (const [validSpecifier, expected] of validSpecifiers) {
     if (validSpecifier === null) continue;
@@ -27,20 +30,56 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
       }));
   }
 
-  // There's no such export - so there's nothing to do.
-  loadFixture('pkgexports/missing').catch(mustCall((err) => {
-    strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
-    assertStartsWith(err.message, 'Package exports');
-    assertIncludes(err.message, 'do not define a \'./missing\' subpath');
-  }));
+  const undefinedExports = new Map([
+    // There's no such export - so there's nothing to do.
+    ['pkgexports/missing', './missing'],
+    // The file exists but isn't exported. The exports is a number which counts
+    // as a non-null value without any properties, just like `{}`.
+    ['pkgexports-number/hidden.js', './hidden.js'],
+  ]);
 
-  // The file exists but isn't exported. The exports is a number which counts
-  // as a non-null value without any properties, just like `{}`.
-  loadFixture('pkgexports-number/hidden.js').catch(mustCall((err) => {
-    strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
-    assertStartsWith(err.message, 'Package exports');
-    assertIncludes(err.message, 'do not define a \'./hidden.js\' subpath');
-  }));
+  const invalidExports = new Map([
+    // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like"
+    // variants do not to prevent confusion and accidental loopholes.
+    ['pkgexports/sub/./../asdf.js', './sub/./../asdf.js'],
+    // This path steps back inside the package but goes through an exports
+    // target that escapes the package, so we still catch that as invalid
+    ['pkgexports/belowdir/pkgexports/asdf.js', './belowdir/pkgexports/asdf.js'],
+    // This target file steps below the package
+    ['pkgexports/belowfile', './belowfile'],
+    // Directory mappings require a trailing / to work
+    ['pkgexports/missingtrailer/x', './missingtrailer/x'],
+    // Invalid target handling
+    ['pkgexports/null', './null'],
+    ['pkgexports/invalid1', './invalid1'],
+    ['pkgexports/invalid2', './invalid2'],
+    ['pkgexports/invalid3', './invalid3'],
+    ['pkgexports/invalid4', './invalid4'],
+    // Missing / invalid fallbacks
+    ['pkgexports/nofallback1', './nofallback1'],
+    ['pkgexports/nofallback2', './nofallback2'],
+    // Reaching into nested node_modules
+    ['pkgexports/nodemodules', './nodemodules'],
+  ]);
+
+  for (const [specifier, subpath] of undefinedExports) {
+    loadFixture(specifier).catch(mustCall((err) => {
+      strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
+      assertStartsWith(err.message, 'Package exports');
+      assertIncludes(err.message, `do not define a '${subpath}' subpath`);
+    }));
+  }
+
+  for (const [specifier, subpath] of invalidExports) {
+    loadFixture(specifier).catch(mustCall((err) => {
+      strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
+      assertStartsWith(err.message, (isRequire ? 'Package exports' :
+        'Cannot resolve'));
+      assertIncludes(err.message, isRequire ?
+        `do not define a valid '${subpath}' subpath` :
+        `matched for '${subpath}'`);
+    }));
+  }
 
   // There's no main field so we won't find anything when importing the name.
   // The fact that "." is mapped is ignored, it's not a valid main config.
@@ -54,26 +93,19 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
     }
   }));
 
-  // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" variants
-  // do not to prevent confusion and accidental loopholes.
-  loadFixture('pkgexports/sub/./../asdf.js').catch(mustCall((err) => {
+  // Covering out bases - not a file is still not a file after dir mapping.
+  loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => {
     strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');
-    assertStartsWith(err.message, 'Package exports');
-    assertIncludes(err.message,
-                   'do not define a \'./sub/./../asdf.js\' subpath');
+    // ESM returns a full file path
+    assertStartsWith(err.message, isRequire ?
+      'Cannot find module \'pkgexports/sub/not-a-file.js\'' :
+      'Cannot find module');
   }));
 
-  // Covering out bases - not a file is still not a file after dir mapping.
-  loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => {
-    if (isRequire) {
-      strictEqual(err.code, 'MODULE_NOT_FOUND');
-      assertStartsWith(err.message,
-                       'Cannot find module \'pkgexports/sub/not-a-file.js\'');
-    } else {
-      strictEqual(err.code, 'ERR_MODULE_NOT_FOUND');
-      // ESM currently returns a full file path
-      assertStartsWith(err.message, 'Cannot find module');
-    }
+  // THe use of %2F escapes in paths fails loading
+  loadFixture('pkgexports/sub/..%2F..%2Fbar.js').catch(mustCall((err) => {
+    strictEqual(err.code, isRequire ? 'ERR_INVALID_FILE_URL_PATH' :
+      'ERR_MODULE_NOT_FOUND');
   }));
 });
 
diff --git a/test/es-module/test-esm-scope-node-modules.mjs b/test/es-module/test-esm-scope-node-modules.mjs
new file mode 100644
index 0000000000..8358da5c76
--- /dev/null
+++ b/test/es-module/test-esm-scope-node-modules.mjs
@@ -0,0 +1,10 @@
+// Flags: --experimental-modules
+import '../common/index.mjs';
+import cjs from '../fixtures/baz.js';
+import { message } from '../fixtures/es-modules/message.mjs';
+import assert from 'assert';
+
+// Assert we loaded esm dependency as ".js" in this mode
+assert.strictEqual(message, 'A message');
+// Assert we loaded CommonJS dependency
+assert.strictEqual(cjs, 'perhaps I work');
diff --git a/test/fixtures/es-modules/package-type-module/index.js b/test/fixtures/es-modules/package-type-module/index.js
index 12aba970ef..e8f4db3e16 100644
--- a/test/fixtures/es-modules/package-type-module/index.js
+++ b/test/fixtures/es-modules/package-type-module/index.js
@@ -1,3 +1,4 @@
+import 'dep/dep.js';
 const identifier = 'package-type-module';
 console.log(identifier);
 export default identifier;
diff --git a/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js b/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js
new file mode 100644
index 0000000000..7b04e8b380
--- /dev/null
+++ b/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js
@@ -0,0 +1,2 @@
+// No package.json -> should still be CommonJS as it is in node_modules
+module.exports = 42;
diff --git a/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js b/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js
new file mode 100644
index 0000000000..888cae37af
--- /dev/null
+++ b/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js
@@ -0,0 +1 @@
+module.exports = 42;
diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json
index 97f07da85e..2b190521e5 100644
--- a/test/fixtures/node_modules/pkgexports/package.json
+++ b/test/fixtures/node_modules/pkgexports/package.json
@@ -3,6 +3,19 @@
     ".": "./asdf.js",
     "./space": "./sp%20ce.js",
     "./valid-cjs": "./asdf.js",
-    "./sub/": "./"
+    "./sub/": "./",
+    "./belowdir/": "../belowdir/",
+    "./belowfile": "../belowfile",
+    "./missingtrailer/": ".",
+    "./null": null,
+    "./invalid1": {},
+    "./invalid2": 1234,
+    "./invalid3": "",
+    "./invalid4": {},
+    "./fallbackdir/": [[], null, {}, "builtin:x/", "./fallbackfile", "./"],
+    "./fallbackfile": [[], null, {}, "builtin:x", "./asdf.js"],
+    "./nofallback1": [],
+    "./nofallback2": [null, {}, "builtin:x"],
+    "./nodemodules": "./node_modules/internalpkg/x.js"
   }
 }