Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Double slash backtracking spec clarification #253

Closed
guybedford opened this issue Aug 2, 2021 · 6 comments
Closed

Double slash backtracking spec clarification #253

guybedford opened this issue Aug 2, 2021 · 6 comments

Comments

@guybedford
Copy link
Collaborator

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
Copy link
Collaborator

domenic commented Aug 3, 2021

What does the reference implementation do?

@guybedford
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

domenic commented Aug 4, 2021

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
Copy link
Collaborator Author

guybedford commented Aug 6, 2021 •
edited
Loading

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
Copy link
Collaborator Author

It seems this behaviour is a direct consequence of #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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@guybedford @domenic