Update reference impelementation and add/update tests.

WICG · Jan 29, 2020 · 26d9748 · 26d9748
1 parent e522f3d
commit 26d9748
Show file tree

Hide file tree

Showing 9 changed files with 129 additions and 7 deletions.
diff --git a/reference-implementation/__tests__/json/parsing-addresses-absolute.json b/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/",

diff --git a/reference-implementation/__tests__/json/parsing-addresses-invalid.json b/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": {}
       }
     }

diff --git a/reference-implementation/__tests__/json/parsing-addresses.json b/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": {}
       }
     }

diff --git a/reference-implementation/__tests__/json/parsing-schema-specifier-map.json b/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": {}

diff --git a/reference-implementation/__tests__/json/parsing-trailing-slashes.json b/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": {}
   }
 }
diff --git a/reference-implementation/__tests__/json/resolving-null.json b/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
+      }
+    }
+  }
+}
diff --git a/reference-implementation/__tests__/test.js b/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',

diff --git a/reference-implementation/lib/parser.js b/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;
     }
 

diff --git a/reference-implementation/lib/resolver.js b/reference-implementation/lib/resolver.js
@@ -34,11 +34,19 @@ function resolveImportsMatch(normalizedSpecifier, specifierMap) {
   for (const [specifierKey, address] of Object.entries(specifierMap)) {
     // Exact-match case
     if (specifierKey === normalizedSpecifier) {
-      return address;
+      if (address === null) {
+        throw new TypeError(`Blocked by a null entry for "${specifierKey}"`);
+      } else {
+        return address;
+      }
     }
 
     // Package prefix-match case
-    if (specifierKey.endsWith('/') && normalizedSpecifier.startsWith(specifierKey)) {
+    else if (specifierKey.endsWith('/') && normalizedSpecifier.startsWith(specifierKey)) {
+      if (address === null) {
+        throw new TypeError(`Blocked by a null entry for "${specifierKey}"`);
+      }
+
       const afterPrefix = normalizedSpecifier.substring(specifierKey.length);
 
       // Enforced by parsing
@@ -48,7 +56,7 @@ function resolveImportsMatch(normalizedSpecifier, specifierMap) {
 
       // 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}"`);
       }
       return url;
     }