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
14 changes: 11 additions & 3 deletions reference-implementation/lib/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
hiroshige-g marked this conversation as resolved.
Show resolved Hide resolved
if (url === null) {
return null;
throw new TypeError(`Failed to resolve prefix-match relative URL for "${specifierKey}"`);
}
return url;
}
Expand Down
40 changes: 31 additions & 9 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,16 @@ 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 and to top-level "`imports`", from most-specific to least-specific prefixes.
For each entry, the result is one of the following:

- Successfully resolves a specifier to a [=URL=].
This makes the [=resolve a module specifier=] algorithm return immediately successfully with the [=URL=].
- Throws an error.
This makes the [=resolve a module specifier=] algorithm fail immediately, i.e. throw the error, without any further fallbacks.
</div>

<h3 id="new-resolve-algorithm">New "resolve a module specifier"</h3>

<div algorithm>
Expand Down Expand Up @@ -393,15 +408,22 @@ 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. If |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=/starts with=] |specifierKey|, then:
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. Return |url|.

1. For each |specifierKey| → |value| of |specifierMap|,
1. If |specifierKey| is |normalizedSpecifier|, then set |resolutionResult| to |value|.
1. Else if |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=/starts with=] |specifierKey|, then:
1. If |value| 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 |value|.
1. If |url| is failure, then set |resolutionResult| to null. Otherwise, set |resolutionResult| to |url|.
1. Otherwise, set |resolutionResult| to |value|.
1. Else, continue.
1. If |resolutionResult| is an [=URL=], then return |resolutionResult|.
1. Otherwise (i.e. |resolutionResult| is null), then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by an entry in an import map.
<p class="note">This terminates the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
1. Return null.
<p class="note">The caller will fallback to less specific scopes or to "`imports`", if any.</p>
</div>

<h3 id="resolving-updates">Updates to other algorithms</h3>
Expand Down