Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make any errors (invalid URLs etc) throw errors in resolution #205

Merged
merged 9 commits into from
Feb 7, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {}
}
}
Expand Down
16 changes: 14 additions & 2 deletions reference-implementation/__tests__/json/parsing-addresses.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@
},
"importMapBaseURL": "data:text/html,test",
"expectedParsedImportMap": {
"imports": {},
"imports": {
"dotSlash": null,
"dotDotSlash": null,
"slash": null
},
"scopes": {}
}
},
Expand Down Expand Up @@ -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": {}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
},
"expectedParsedImportMap": {
"imports": {
"null": null,
"boolean": null,
"number": null,
"object": null,
"array": null,
"array2": null,
"string": "https://example.com/"
},
"scopes": {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
},
"importMapBaseURL": "https://base.example/path1/path2/path3",
"expectedParsedImportMap": {
"imports": {},
"imports": {
"trailer/": null
},
"scopes": {}
}
}
82 changes: 82 additions & 0 deletions reference-implementation/__tests__/json/resolving-null.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
1 change: 1 addition & 0 deletions reference-implementation/__tests__/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
3 changes: 3 additions & 0 deletions reference-implementation/lib/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
26 changes: 20 additions & 6 deletions reference-implementation/lib/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
35 changes: 29 additions & 6 deletions spec.bs
Original file line number Diff line number Diff line change
Expand Up @@ -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=]:

Expand Down Expand Up @@ -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=].
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down