Make parsing and resolution errors cause resolution to throw

After this change, all errors, found either at parsing or resolution time, result in the resolution processing throwing errors.

Previously, errors found at parsing time, namely:

* Non-string values, including null, [], etc.
* Invalid URLs
* Entries with keys with trailing "/" and values without trailing "/"

were ignored, allowing fallback to the next candidate at resolution time. Thus, they could not be used as a definitive way to block particular resolution. After this change, such entries are kept in the parsed specifier maps as entries with null value, and cause resolution to fail (without further fallback) by throwing errors.

Also, before this change, relative URL resolution failures at prefix matching in "resolve a module specifier" didn't allow fallback to less-specific prefixes, but did allow fallback to less-specific scopes. This PR makes errors which would have caused such fallbacks always throw errors, so fallback is not allowed in either case.

With this change, all resolution failures are surfaced via thrown errors, and resolution is definitively blocked. Fallback to less-specific scopes only occurs when there are no matching entries in a scope; errored entries no longer cause fallbacks.

Fixes #184.

reference-implementation/__tests__/json/parsing-addresses-absolute.json

@@ -51,6 +51,8 @@
       "importMapBaseURL": "https://base.example/path1/path2/path3",
       "expectedParsedImportMap": {
         "imports": {
+          "unparseable2": null,
+          "unparseable3": null,
           "invalidButParseable1": "https://example.org/",
           "invalidButParseable2": "https://example.com///",
           "prettyNormal": "https://example.net/",

reference-implementation/__tests__/json/parsing-addresses-invalid.json

@@ -13,7 +13,13 @@
       },
       "importMapBaseURL": "https://base.example/path1/path2/path3",
       "expectedParsedImportMap": {
-        "imports": {},
+        "imports": {
+          "foo1": null,
+          "foo2": null,
+          "foo3": null,
+          "foo4": null,
+          "foo5": null
+        },
         "scopes": {}
       }
     }

reference-implementation/__tests__/json/parsing-addresses.json

@@ -29,7 +29,11 @@
       },
       "importMapBaseURL": "data:text/html,test",
       "expectedParsedImportMap": {
-        "imports": {},
+        "imports": {
+          "dotSlash": null,
+          "dotDotSlash": null,
+          "slash": null
+        },
         "scopes": {}
       }
     },
@@ -65,7 +69,15 @@
       },
       "importMapBaseURL": "https://base.example/path1/path2/path3",
       "expectedParsedImportMap": {
-        "imports": {},
+        "imports": {
+          "dotSlash1": null,
+          "dotDotSlash1": null,
+          "dotSlash2": null,
+          "dotDotSlash2": null,
+          "slash2": null,
+          "dotSlash3": null,
+          "dotDotSlash3": null
+        },
         "scopes": {}
       }
     }

reference-implementation/__tests__/json/parsing-schema-specifier-map.json

@@ -18,6 +18,12 @@
       },
       "expectedParsedImportMap": {
         "imports": {
+          "null": null,
+          "boolean": null,
+          "number": null,
+          "object": null,
+          "array": null,
+          "array2": null,
           "string": "https://example.com/"
         },
         "scopes": {}

reference-implementation/__tests__/json/parsing-trailing-slashes.json

@@ -7,7 +7,9 @@
   },
   "importMapBaseURL": "https://base.example/path1/path2/path3",
   "expectedParsedImportMap": {
-    "imports": {},
+    "imports": {
+      "trailer/": null
+    },
     "scopes": {}
   }
 }

reference-implementation/__tests__/json/resolving-null.json

@@ -0,0 +1,82 @@
+{
+  "importMapBaseURL": "https://example.com/app/index.html",
+  "baseURL": "https://example.com/js/app.mjs",
+  "name": "Entries with errors shouldn't allow fallback",
+  "tests": {
+    "No fallback to less-specific prefixes": {
+      "importMap": {
+        "imports": {
+          "null/": "/1/",
+          "null/b/": null,
+          "null/b/c/": "/1/2/",
+          "invalid-url/": "/1/",
+          "invalid-url/b/": "https://:invalid-url:/",
+          "invalid-url/b/c/": "/1/2/",
+          "without-trailing-slashes/": "/1/",
+          "without-trailing-slashes/b/": "/x",
+          "without-trailing-slashes/b/c/": "/1/2/",
+          "prefix-resolution-error/": "/1/",
+          "prefix-resolution-error/b/": "data:text/javascript,/",
+          "prefix-resolution-error/b/c/": "/1/2/"
+        }
+      },
+      "expectedResults": {
+        "null/x": "https://example.com/1/x",
+        "null/b/x": null,
+        "null/b/c/x": "https://example.com/1/2/x",
+        "invalid-url/x": "https://example.com/1/x",
+        "invalid-url/b/x": null,
+        "invalid-url/b/c/x": "https://example.com/1/2/x",
+        "without-trailing-slashes/x": "https://example.com/1/x",
+        "without-trailing-slashes/b/x": null,
+        "without-trailing-slashes/b/c/x": "https://example.com/1/2/x",
+        "prefix-resolution-error/x": "https://example.com/1/x",
+        "prefix-resolution-error/b/x": null,
+        "prefix-resolution-error/b/c/x": "https://example.com/1/2/x"
+      }
+    },
+    "No fallback to less-specific scopes": {
+      "importMap": {
+        "imports": {
+          "null": "https://example.com/a",
+          "invalid-url": "https://example.com/b",
+          "without-trailing-slashes/": "https://example.com/c/",
+          "prefix-resolution-error/": "https://example.com/d/"
+        },
+        "scopes": {
+          "/js/": {
+            "null": null,
+            "invalid-url": "https://:invalid-url:/",
+            "without-trailing-slashes/": "/x",
+            "prefix-resolution-error/": "data:text/javascript,/"
+          }
+        }
+      },
+      "expectedResults": {
+        "null": null,
+        "invalid-url": null,
+        "without-trailing-slashes/x": null,
+        "prefix-resolution-error/x": null
+      }
+    },
+    "No fallback to absolute URL parsing": {
+      "importMap": {
+        "imports": {},
+        "scopes": {
+          "/js/": {
+            "https://example.com/null": null,
+            "https://example.com/invalid-url": "https://:invalid-url:/",
+            "https://example.com/without-trailing-slashes/": "/x",
+            "https://example.com/prefix-resolution-error/": "data:text/javascript,/"
+          }
+        }
+      },
+      "expectedResults": {
+        "https://example.com/null": null,
+        "https://example.com/invalid-url": null,
+        "https://example.com/without-trailing-slashes/x": null,
+        "https://example.com/prefix-resolution-error/x": null
+      }
+    }
+  }
+}

reference-implementation/__tests__/test.js

@@ -16,6 +16,7 @@ for (const jsonFile of [
   'parsing-scope-keys.json',
   'parsing-specifier-keys.json',
   'parsing-trailing-slashes.json',
+  'resolving-null.json',
   'scopes-exact-vs-prefix.json',
   'scopes.json',
   'tricky-specifiers.json',

reference-implementation/lib/parser.js

@@ -52,18 +52,21 @@ function sortAndNormalizeSpecifierMap(obj, baseURL) {
     if (typeof value !== 'string') {
       console.warn(`Invalid address ${JSON.stringify(value)} for the specifier key "${specifierKey}". ` +
           `Addresses must be strings.`);
+      normalized[normalizedSpecifierKey] = null;
       continue;
     }
 
     const addressURL = tryURLLikeSpecifierParse(value, baseURL);
     if (addressURL === null) {
       console.warn(`Invalid address "${value}" for the specifier key "${specifierKey}".`);
+      normalized[normalizedSpecifierKey] = null;
       continue;
     }
 
     if (specifierKey.endsWith('/') && !addressURL.href.endsWith('/')) {
       console.warn(`Invalid address "${addressURL.href}" for package specifier key "${specifierKey}". ` +
           `Package addresses must end with "/".`);
+      normalized[normalizedSpecifierKey] = null;
       continue;
     }
 

reference-implementation/lib/resolver.js

@@ -31,25 +31,39 @@ exports.resolve = (specifier, parsedImportMap, scriptURL) => {
 };
 
 function resolveImportsMatch(normalizedSpecifier, specifierMap) {
-  for (const [specifierKey, address] of Object.entries(specifierMap)) {
+  for (const [specifierKey, resolutionResult] of Object.entries(specifierMap)) {
     // Exact-match case
     if (specifierKey === normalizedSpecifier) {
-      return address;
+      if (resolutionResult === null) {
+        throw new TypeError(`Blocked by a null entry for "${specifierKey}"`);
+      }
+
+      assert(resolutionResult instanceof URL);
+
+      return resolutionResult;
     }
 
     // Package prefix-match case
     if (specifierKey.endsWith('/') && normalizedSpecifier.startsWith(specifierKey)) {
+      if (resolutionResult === null) {
+        throw new TypeError(`Blocked by a null entry for "${specifierKey}"`);
+      }
+
+      assert(resolutionResult instanceof URL);
+
       const afterPrefix = normalizedSpecifier.substring(specifierKey.length);
 
       // Enforced by parsing
-      assert(address.href.endsWith('/'));
+      assert(resolutionResult.href.endsWith('/'));
 
-      const url = tryURLParse(afterPrefix, address);
+      const url = tryURLParse(afterPrefix, resolutionResult);
 
-      // This code looks stupid but it follows the spec more exactly and also gives code coverage a chance to shine.
       if (url === null) {
-        return null;
+        throw new TypeError(`Failed to resolve prefix-match relative URL for "${specifierKey}"`);
       }
+
+      assert(url instanceof URL);
+
       return url;
     }
   }

spec.bs

@@ -53,7 +53,9 @@ summary {
 
 <h2 id="definitions">Definitions</h2>
 
-A <dfn>specifier map</dfn> is an [=ordered map=] from [=strings=] to [=URLs=].
+A <dfn>resolution result</dfn> is either a [=URL=] or null.
+
+A <dfn>specifier map</dfn> is an [=ordered map=] from [=strings=] to [=resolution results=].
 
 A <dfn>import map</dfn> is a [=struct=] with two [=struct/items=]:
 
@@ -310,13 +312,16 @@ To <dfn>register an import map</dfn> given an {{HTMLScriptElement}} |element|:
     1. If |normalizedSpecifierKey| is null, then [=continue=].
     1. If |value| is not a [=string=], then:
       1. [=Report a warning to the console=] that addresses must be strings.
+      1. Set |normalized|[|specifierKey|] to null.
       1. [=Continue=].
     1. Let |addressURL| be the result of [=parsing a URL-like import specifier=] given |value| and |baseURL|.
     1. If |addressURL| is null, then:
       1. [=Report a warning to the console=] that the address was invalid.
+      1. Set |normalized|[|specifierKey|] to null.
       1. [=Continue=].
     1. If |specifierKey| ends with U+002F (/), and the [=URL serializer|serialization=] of |addressURL| does not end with U+002F (/), then:
       1. [=Report a warning to the console=] that an invalid address was given for the specifier key |specifierKey|; since |specifierKey| ended in a slash, so must the address.
+      1. Set |normalized|[|specifierKey|] to null.
       1. [=Continue=].
     1. Set |normalized|[|specifierKey|] to |addressURL|.
   1. Return the result of [=map/sorting=] |normalized|, with an entry |a| being less than an entry |b| if |b|'s [=map/key=] is [=code unit less than=] |a|'s [=map/key=].
@@ -365,6 +370,14 @@ To <dfn>register an import map</dfn> given an {{HTMLScriptElement}} |element|:
 
 <h2 id="resolving">Resolving module specifiers</h2>
 
+<div class="note">
+  During [=resolve a module specifier|resolving a module specifier=], the following algorithms check candidate entries of [=specifier maps=], from most-specific to least-specific scopes (falling back to top-level "`imports`"), and from most-specific to least-specific prefixes. For each candidate, the result is one of the following:
+
+  - Successfully resolves a specifier to a [=URL=]. This makes the [=resolve a module specifier=] algorithm immediately return that [=URL=].
+  - Throws an error. This makes the [=resolve a module specifier=] algorithm rethrow the error, without any further fallbacks.
+  - Fails to resolve, without an error. In this case the algorithm moves on to the next candidate.
+</div>
+
 <h3 id="new-resolve-algorithm">New "resolve a module specifier"</h3>
 
 <div algorithm>
@@ -393,15 +406,25 @@ To <dfn>register an import map</dfn> given an {{HTMLScriptElement}} |element|:
 <div algorithm>
   To <dfn lt="resolve an imports match|resolving an imports match">resolve an imports match</dfn>, given a [=string=] |normalizedSpecifier| and a [=specifier map=] |specifierMap|:
 
-  1. For each |specifierKey| → |address| of |specifierMap|,
-    1. If |specifierKey| is |normalizedSpecifier|, then return |address|.
+  1. For each |specifierKey| → |resolutionResult| of |specifierMap|,
+    1. If |specifierKey| is |normalizedSpecifier|, then:
+      1. If |resolutionResult| is null, then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by a null entry.
+        <p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
+      1. Assert: |resolutionResult| is a [=URL=].
+      1. Return |resolutionResult|.
     1. If |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=/starts with=] |specifierKey|, then:
+      1. If |resolutionResult| is null, then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by a null entry.
+        <p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
+      1. Assert: |resolutionResult| is a [=URL=].
       1. Let |afterPrefix| be the portion of |normalizedSpecifier| after the initial |specifierKey| prefix.
-      1. Assert: |afterPrefix| ends with "`/`", as enforced during [=parse an import map string|parsing=].
-      1. Let |url| be the result of [=URL parser|parsing=] |afterPrefix| relative to the base URL |address|.
-      1. If |url| is failure, then return null.
+      1. Assert: |resolutionResult|, [=URL serializer|serialized=], ends with "`/`", as enforced during [=parse an import map string|parsing=].
+      1. Let |url| be the result of [=URL parser|parsing=] |afterPrefix| relative to the base URL |resolutionResult|.
+      1. If |url| is failure, then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked due to a URL parse failure.
+        <p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
+      1. Assert: |url| is a [=URL=].
       1. Return |url|.
   1. Return null.
+     <p class="note">The [=resolve a module specifier=] algorithm will fallback to a less specific scope or to "`imports`", if possible.</p>
 </div>
 
 <h3 id="resolving-updates">Updates to other algorithms</h3>