WICG / import-maps

How to control the behavior of JavaScript imports
https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
Other
2.69k stars 70 forks source link

Double slash backtracking spec clarification #253

Closed guybedford closed 3 years ago

guybedford commented 3 years ago

Is this a chrome bug or spec bug?

<!doctype html>
<script type="importmap">
{
  "imports": {
    "x/": "/y/"
  }
}
</script>
<script type="module">
  import 'x//p';
</script>

Gives the error:

Uncaught TypeError: Failed to resolve module specifier "x//p". Import Map: "x//p" matches with "x/" but is blocked due to backtracking

Whereas x/y//p would be supported fine.

Reading the spec, I can't see why this would be specifically distinguished in this case since /y//p is a subpath of /y/.

domenic commented 3 years ago

What does the reference implementation do?

guybedford commented 3 years ago

The reference implementation throws -

Uncaught TypeError: The specifier "x//p" backtracks above its prefix "x/"
    at resolveImportsMatch (C:\Users\guybe\Projects\import-maps\reference-implem
entation\lib\resolver.js:70:15)
    at Object.exports.resolve (C:\Users\guybe\Projects\import-maps\reference-imp
lementation\lib\resolver.js:20:32)
> require('./lib/resolver.js').resolve('x//p', map, 'file:///base')
Uncaught TypeError: The specifier "x//p" backtracks above its prefix "x/"
    at resolveImportsMatch (C:\Users\guybe\Projects\import-maps\reference-implem
entation\lib\resolver.js:70:15)
    at Object.exports.resolve (C:\Users\guybe\Projects\import-maps\reference-imp
lementation\lib\resolver.js:20:32)
guybedford commented 3 years ago

Ah, it seems the issue is it is doing new URL('/p', '/y/') which returns file:///p which is below the base.

Interestingly this implies the following is valid:

<!doctype html>
<script type="importmap">
{
  "imports": {
    "x/": "/y/"
  }
}
</script>
<script type="module">
  import 'x//y/z';
</script>

And will resolve to /y/z.

domenic commented 3 years ago

Oh, you are doing this on a file: URL? Yeah those are wonky; in general all bets are off. (I'm surprised modules even work there...)

guybedford commented 3 years ago

The example above applies equally well to https URLs as it does file URLs. Basically because new URL('/y/z', '/y/') returns 'https://site.com/y/z' which passes the test of being contained in 'https://site.com/y/'.

(I'm surprised modules even work there...)

I hope you realise this is a major consumption point of this spec for the major server JS platforms. Deno relies on this,as does Node.js where users ship loaders supporting import maps and the only way that applies to local modules in both runtimes is via file urls.

guybedford commented 3 years ago

It seems this behaviour is a direct consequence of https://github.com/WICG/import-maps/issues/173 in using URL resolution over string concatenation to handle eg paths in querystrings.

On balance I agree this edge case is less confusing than the weird string concat edge case alternatives.