diff --git a/reference-implementation/__tests__/parsing-addresses.js b/reference-implementation/__tests__/parsing-addresses.js index a5ed24f..0f5fc73 100644 --- a/reference-implementation/__tests__/parsing-addresses.js +++ b/reference-implementation/__tests__/parsing-addresses.js @@ -116,23 +116,19 @@ describe('Built-in module addresses', () => { ); }); - it('should ignore and warn on built-in module URLs that contain "/"', () => { + it('should allow built-in module URLs that contain "/" or "\\"', () => { expectSpecifierMap( `{ - "bad1": "${BUILT_IN_MODULE_SCHEME}:foo/", - "bad2": "${BUILT_IN_MODULE_SCHEME}:foo/bar", - "good": "${BUILT_IN_MODULE_SCHEME}:foo\\\\baz" + "slashEnd": "${BUILT_IN_MODULE_SCHEME}:foo/", + "slashMiddle": "${BUILT_IN_MODULE_SCHEME}:foo/bar", + "backslash": "${BUILT_IN_MODULE_SCHEME}:foo\\\\baz" }`, 'https://base.example/path1/path2/path3', { - bad1: [], - bad2: [], - good: [expect.toMatchURL(`${BUILT_IN_MODULE_SCHEME}:foo\\baz`)] - }, - [ - `Invalid address "${BUILT_IN_MODULE_SCHEME}:foo/". Built-in module URLs must not contain "/".`, - `Invalid address "${BUILT_IN_MODULE_SCHEME}:foo/bar". Built-in module URLs must not contain "/".` - ] + slashEnd: [expect.toMatchURL(`${BUILT_IN_MODULE_SCHEME}:foo/`)], + slashMiddle: [expect.toMatchURL(`${BUILT_IN_MODULE_SCHEME}:foo/bar`)], + backslash: [expect.toMatchURL(`${BUILT_IN_MODULE_SCHEME}:foo\\baz`)] + } ); }); }); @@ -285,39 +281,54 @@ describe('Failing addresses: mismatched trailing slashes', () => { it('should warn for the simple case', () => { expectSpecifierMap( `{ - "trailer/": "/notrailer" + "trailer/": "/notrailer", + "${BUILT_IN_MODULE_SCHEME}:trailer/": "/bim-notrailer" }`, 'https://base.example/path1/path2/path3', { - 'trailer/': [] + 'trailer/': [], + [`${BUILT_IN_MODULE_SCHEME}:trailer/`]: [] }, - [`Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`] + [ + `Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`, + `Invalid address "https://base.example/bim-notrailer" for package specifier key "${BUILT_IN_MODULE_SCHEME}:trailer/". Package addresses must end with "/".` + ] ); }); it('should warn for a mismatch alone in an array', () => { expectSpecifierMap( `{ - "trailer/": ["/notrailer"] + "trailer/": ["/notrailer"], + "${BUILT_IN_MODULE_SCHEME}:trailer/": ["/bim-notrailer"] }`, 'https://base.example/path1/path2/path3', { - 'trailer/': [] + 'trailer/': [], + [`${BUILT_IN_MODULE_SCHEME}:trailer/`]: [] }, - [`Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`] + [ + `Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`, + `Invalid address "https://base.example/bim-notrailer" for package specifier key "${BUILT_IN_MODULE_SCHEME}:trailer/". Package addresses must end with "/".` + ] ); }); it('should warn for a mismatch alongside non-mismatches in an array', () => { expectSpecifierMap( `{ - "trailer/": ["/atrailer/", "/notrailer"] + "trailer/": ["/atrailer/", "/notrailer"], + "${BUILT_IN_MODULE_SCHEME}:trailer/": ["/bim-atrailer/", "/bim-notrailer"] }`, 'https://base.example/path1/path2/path3', { - 'trailer/': [expect.toMatchURL('https://base.example/atrailer/')] + 'trailer/': [expect.toMatchURL('https://base.example/atrailer/')], + [`${BUILT_IN_MODULE_SCHEME}:trailer/`]: [expect.toMatchURL('https://base.example/bim-atrailer/')] }, - [`Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`] + [ + `Invalid address "https://base.example/notrailer" for package specifier key "trailer/". Package addresses must end with "/".`, + `Invalid address "https://base.example/bim-notrailer" for package specifier key "${BUILT_IN_MODULE_SCHEME}:trailer/". Package addresses must end with "/".` + ] ); }); }); diff --git a/reference-implementation/__tests__/parsing-specifier-keys.js b/reference-implementation/__tests__/parsing-specifier-keys.js index 0858266..9eb423a 100644 --- a/reference-implementation/__tests__/parsing-specifier-keys.js +++ b/reference-implementation/__tests__/parsing-specifier-keys.js @@ -139,7 +139,7 @@ describe('Absolute URL specifier keys', () => { ); }); - it('should only parse built-in module specifier keys without a /', () => { + it('should parse built-in module specifier keys, including with a "/"', () => { expectSpecifierMap( `{ "${BLANK}": "/blank", @@ -150,12 +150,10 @@ describe('Absolute URL specifier keys', () => { 'https://base.example/path1/path2/path3', { [BLANK]: [expect.toMatchURL('https://base.example/blank')], + [`${BLANK}/`]: [expect.toMatchURL('https://base.example/blank/')], + [`${BLANK}/foo`]: [expect.toMatchURL('https://base.example/blank/foo')], [`${BLANK}\\foo`]: [expect.toMatchURL('https://base.example/blank/backslashfoo')] - }, - [ - `Invalid specifier key "${BLANK}/". Built-in module specifiers must not contain "/".`, - `Invalid specifier key "${BLANK}/foo". Built-in module specifiers must not contain "/".` - ] + } ); }); }); diff --git a/reference-implementation/__tests__/resolving-builtins.js b/reference-implementation/__tests__/resolving-builtins.js index c378aad..a9383df 100644 --- a/reference-implementation/__tests__/resolving-builtins.js +++ b/reference-implementation/__tests__/resolving-builtins.js @@ -40,6 +40,24 @@ describe('Remapping built-in module specifiers', () => { expect(resolveUnderTest(NONE)).toMatchURL('https://example.com/app/none.mjs'); }); + it('should remap built-in modules with slashes', () => { + const resolveUnderTest = makeResolveUnderTest(`{ + "imports": { + "${BLANK}/": "./blank-slash/", + "${BLANK}/foo": "./blank-foo.mjs", + "${NONE}/": "./none-slash/", + "${NONE}/foo": "./none-foo.mjs" + } + }`); + + expect(resolveUnderTest(`${BLANK}/`)).toMatchURL('https://example.com/app/blank-slash/'); + expect(resolveUnderTest(`${BLANK}/foo`)).toMatchURL('https://example.com/app/blank-foo.mjs'); + expect(resolveUnderTest(`${BLANK}/bar`)).toMatchURL('https://example.com/app/blank-slash/bar'); + expect(resolveUnderTest(`${NONE}/`)).toMatchURL('https://example.com/app/none-slash/'); + expect(resolveUnderTest(`${NONE}/foo`)).toMatchURL('https://example.com/app/none-foo.mjs'); + expect(resolveUnderTest(`${NONE}/bar`)).toMatchURL('https://example.com/app/none-slash/bar'); + }); + it('should remap built-in modules with fallbacks', () => { const resolveUnderTest = makeResolveUnderTest(`{ "imports": { @@ -51,6 +69,31 @@ describe('Remapping built-in module specifiers', () => { expect(resolveUnderTest(BLANK)).toMatchURL(BLANK); expect(resolveUnderTest(NONE)).toMatchURL('https://example.com/app/none.mjs'); }); + + it('should remap built-in modules with slashes and fallbacks', () => { + // NOTE: `${BLANK}/for-testing` is not per spec, just for these tests. + // See resolver.js. + const resolveUnderTest = makeResolveUnderTest(`{ + "imports": { + "${BLANK}/": ["${BLANK}/", "./blank/"], + "${BLANK}/for-testing": ["${BLANK}/for-testing", "./blank-for-testing-special"], + "${NONE}/": ["${NONE}/", "./none/"], + "${NONE}/foo": ["${NONE}/foo", "./none-foo-special"] + } + }`); + + // Built-in modules only resolve for exact matches, so this will trigger the fallback. + expect(resolveUnderTest(`${BLANK}/`)).toMatchURL('https://example.com/app/blank/'); + expect(resolveUnderTest(`${BLANK}/foo`)).toMatchURL('https://example.com/app/blank/foo'); + + // This would fall back in a real implementation; it's only because we've gone against + // spec in the reference implementation (to make this testable) that this maps. + expect(resolveUnderTest(`${BLANK}/for-testing`)).toMatchURL(`${BLANK}/for-testing`); + + expect(resolveUnderTest(`${NONE}/`)).toMatchURL('https://example.com/app/none/'); + expect(resolveUnderTest(`${NONE}/bar`)).toMatchURL('https://example.com/app/none/bar'); + expect(resolveUnderTest(`${NONE}/foo`)).toMatchURL('https://example.com/app/none-foo-special'); + }); }); describe('Remapping to built-in modules', () => { @@ -58,6 +101,8 @@ describe('Remapping to built-in modules', () => { "imports": { "blank": "${BLANK}", "/blank": "${BLANK}", + "/blank/": "${BLANK}/", + "/blank-for-testing": "${BLANK}/for-testing", "none": "${NONE}", "/none": "${NONE}" } @@ -68,6 +113,15 @@ describe('Remapping to built-in modules', () => { expect(resolveUnderTest('/blank')).toMatchURL(BLANK); }); + it(`should fail when remapping to "${BLANK}/"`, () => { + expect(() => resolveUnderTest('/blank/')).toThrow(TypeError); + }); + + it(`should remap to "${BLANK}/for-testing"`, () => { + expect(resolveUnderTest('/blank/for-testing')).toMatchURL(`${BLANK}/for-testing`); + expect(resolveUnderTest('/blank-for-testing')).toMatchURL(`${BLANK}/for-testing`); + }); + it(`should remap to "${BLANK}" for URL-like specifiers`, () => { expect(resolveUnderTest('/blank')).toMatchURL(BLANK); expect(resolveUnderTest('https://example.com/blank')).toMatchURL(BLANK); diff --git a/reference-implementation/lib/parser.js b/reference-implementation/lib/parser.js index 4cc04d6..3a2e2ac 100644 --- a/reference-implementation/lib/parser.js +++ b/reference-implementation/lib/parser.js @@ -1,6 +1,6 @@ 'use strict'; const assert = require('assert'); -const { tryURLParse, hasFetchScheme, tryURLLikeSpecifierParse, BUILT_IN_MODULE_PROTOCOL } = require('./utils.js'); +const { tryURLParse, hasFetchScheme, tryURLLikeSpecifierParse } = require('./utils.js'); exports.parseFromString = (input, baseURL) => { const parsed = JSON.parse(input); @@ -86,11 +86,6 @@ function sortAndNormalizeSpecifierMap(obj, baseURL) { continue; } - if (addressURL.protocol === BUILT_IN_MODULE_PROTOCOL && addressURL.href.includes('/')) { - console.warn(`Invalid address "${potentialAddress}". Built-in module URLs must not contain "/".`); - continue; - } - validNormalizedAddresses.push(addressURL); } normalized[specifierKey] = validNormalizedAddresses; @@ -145,12 +140,7 @@ function normalizeSpecifierKey(specifierKey, baseURL) { const url = tryURLLikeSpecifierParse(specifierKey, baseURL); if (url !== null) { - const urlString = url.href; - if (url.protocol === BUILT_IN_MODULE_PROTOCOL && urlString.includes('/')) { - console.warn(`Invalid specifier key "${urlString}". Built-in module specifiers must not contain "/".`); - return null; - } - return urlString; + return url.href; } return specifierKey; diff --git a/reference-implementation/lib/resolver.js b/reference-implementation/lib/resolver.js index 9fe87c9..19bb030 100644 --- a/reference-implementation/lib/resolver.js +++ b/reference-implementation/lib/resolver.js @@ -1,8 +1,13 @@ 'use strict'; const { URL } = require('url'); +const assert = require('assert'); const { tryURLLikeSpecifierParse, BUILT_IN_MODULE_SCHEME, BUILT_IN_MODULE_PROTOCOL } = require('./utils.js'); -const supportedBuiltInModules = new Set([`${BUILT_IN_MODULE_SCHEME}:blank`]); +// TODO: clean up by allowing caller (and thus tests) to choose the list of built-ins? +const supportedBuiltInModules = new Set([ + `${BUILT_IN_MODULE_SCHEME}:blank`, + `${BUILT_IN_MODULE_SCHEME}:blank/for-testing` // NOTE: not in the spec. +]); exports.resolve = (specifier, parsedImportMap, scriptURL) => { const asURL = tryURLLikeSpecifierParse(specifier, scriptURL); @@ -64,7 +69,27 @@ function resolveImportsMatch(normalizedSpecifier, specifierMap) { `(via prefix specifier key "${specifierKey}").`); } else if (addresses.length === 1) { const afterPrefix = normalizedSpecifier.substring(specifierKey.length); - return new URL(afterPrefix, addresses[0]); + + // Enforced by parsing + assert(addresses[0].href.endsWith('/')); + + // Cannot use URL resolution directly. E.g. "switch" relative to "std:elements/" is a + // parsing failure. + return new URL(addresses[0] + afterPrefix); + } else if (addresses.length === 2 && + addresses[0].protocol === BUILT_IN_MODULE_PROTOCOL && + addresses[1].protocol !== BUILT_IN_MODULE_PROTOCOL) { + const afterPrefix = normalizedSpecifier.substring(specifierKey.length); + + // Enforced by parsing + assert(addresses[0].href.endsWith('/')); + assert(addresses[1].href.endsWith('/')); + + // Cannot use URL resolution directly. E.g. "switch" relative to "std:elements/" is a + // parsing failure. + const url0 = new URL(addresses[0] + afterPrefix); + const url1 = new URL(addresses[1] + afterPrefix); + return supportedBuiltInModules.has(url0.href) ? url0 : url1; } else { throw new Error('The reference implementation for multi-address fallbacks that are not ' + '[built-in module, fetch-scheme URL] is not yet implemented.'); diff --git a/spec.bs b/spec.bs index 9c14ee2..bba0eac 100644 --- a/spec.bs +++ b/spec.bs @@ -398,9 +398,6 @@ To register an import map given an {{HTMLScriptElement}} |element|: 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. [=Continue=]. - 1. If |specifierKey|'s [=url/scheme=] is "`std`" and the [=URL serializer|serialization=] of |addressURL| contains U+002F (/), then: - 1. [=Report a warning to the console=] that built-in module URLs must not contain slashes. - 1. [=Continue=]. 1. [=list/Append=] |addressURL| to |validNormalizedAddresses|. 1. Set |normalized|[|specifierKey|] to |validNormalizedAddresses|. 1. Return the result of [=map/sorting=] |normalized|, with an entry |a| being less than an entry |b| if |a|'s [=map/key=] is [=longer or code unit less than=] |b|'s [=map/key=]. @@ -431,12 +428,7 @@ To register an import map given an {{HTMLScriptElement}} |element|: 1. [=Report a warning to the console=] that specifier keys cannot be the empty string. 1. Return null. 1. Let |url| be the result of [=parsing a URL-like import specifier=], given |specifierKey| and |baseURL|. - 1. If |url| is not null, then: - 1. Let |urlString| be the [=URL serializer|serialization=] of |url|. - 1. If |url|'s [=url/scheme=] is "`std`" and |urlString| contains U+002F (/), then: - 1. [=Report a warning to the console=] that built-in module specifiers must not contain slashes. - 1. Return null. - 1. Return |urlString|. + 1. If |url| is not null, then return the [=URL serializer|serialization=] of |url|. 1. Return |specifierKey|. @@ -511,9 +503,18 @@ To register an import map given an {{HTMLScriptElement}} |element|: 1. If |addresses|'s [=list/size=] is 0, then throw a {{TypeError}} indicating that |normalizedSpecifier| was mapped to no addresses. 1. If |addresses|'s [=list/size=] is 1, then: 1. Let |afterPrefix| be the portion of |normalizedSpecifier| after the initial |specifierKey| prefix. - 1. Let |url| be the result of [=URL parser|parsing=] |afterPrefix| relative to |addresses|[0]. - 1. If |url| is failure, throw a {{TypeError}}, implicating |normalizedSpecifier| (and in particular the |afterPrefix| portion). + 1. Assert: |afterPrefix| ends with "`/`", as enforced during [=parse an import map string|parsing=]. + 1. Let |url| be the result of [=URL parser|parsing=] the concatenation of the [=URL serializer|serialization=] of |addresses|[0] with |afterPrefix|. +

We [=URL parser|parse=] the concatenation, instead of parsing |afterPrefix| relative to |addresses|[0], due to cases such as an |afterPrefix| of "`switch`" and an |addresses|[0] of "`std:elements/`". + 1. Assert: |url| is not failure, since |addresses|[0] was a URL, and appending after the trailing "`/`" will not make it unparseable. 1. Return |url|. + 1. If |addresses|'s [=list/size=] is 2, and |addresses|[0]'s [=url/scheme=] is "`std`", and |addresses|[1]'s [=url/scheme=] is not "`std`", 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 |url0| be the result of [=URL parser|parsing=] the concatenation of the [=URL serializer|serialization=] of |addresses|[0] with |afterPrefix|; similarly, let |url1| be the result of [=URL parser|parsing=] the concatenation of the [=URL serializer|serialization=] of |addresses|[1] with |afterPrefix|. +

As above, we parse the concatenation to deal with built-in module cases. + 1. Assert: neither |url0| nor |url1| are failure, since |addresses|[0] and |addresses|[1] were URLs, and appending after their trailing "`/`" will not make them unparseable. + 1. Return |url0|, if |moduleMap|[|url0|] [=map/exists=]; otherwise, return |url1|. 1. Otherwise, we have no specification for more complicated fallbacks yet; throw a {{TypeError}} indicating this is not yet supported. 1. Return null. @@ -522,7 +523,6 @@ To register an import map given an {{HTMLScriptElement}} |element|: To validate a module script URL, given a [=URL=] |url|, an [=environment settings object=] |settings object|, and a [=URL=] |base URL|: 1. If |url|'s [=url/scheme=] is "`std`", then: - 1. If the [=URL serializer|serialization=] of |url| contains U+002F (/), then throw a {{TypeError}} indicating that |url| is a malformed built-in URL. 1. Let |moduleMap| be |settings object|'s [=environment settings object/module map=]. 1. If |moduleMap|[|url|] does not [=map/exist=], then throw a {{TypeError}} indicating that the requested built-in module is not implemented.

This condition is added to ensure that |moduleMap|[|url|] does not [=map/exist=] for unimplemented built-ins. Without this condition, fetch a single module script might be called and |moduleMap|[|url|] can be set to null, which might complicates the spec around built-ins.