pathname . collapsing triggering too much

[crowlKats]

test code:

const p = new URLPattern({ pathname: '/:foo.' });
console.log(p.test({pathname: "/bar."}));
console.log(p.test({pathname: "/bar"}));

Chrome:

true
false

urlpattern-polyfill:

true
true

deno currently panics, but with a quick fix for the panic, i reach same result as urlpattern-polyfill. Is something missing in the spec?

[wanderview]

Yes, this is a bug in the spec. The algorithm here:

https://wicg.github.io/urlpattern/#canonicalize-a-pathname

Creates a fakes URL and assigns the character to encode to the end of the pathname. A . after the implicit leading slash gets ellided by the URL normalization algorithm. We should special case this.

[wanderview]

Hmm, I patched the immediate issue, but I'm not sure its adequate now.

Consider new URLPattern({ pathname: ':foo./' }). Per the spec and the polyfill this gets converted to just :foo which seems wrong. Chromium preserves it as :foo./ instead.

On the flip side, though, what would we expect for new URLPattern({ pathname: './foo' }). Note there is no base URL here. Chromium keeps it as ./foo, but the spec and polyfill collapse the ./ to just foo. I think the chromium behavior is more correct here, but not as sure. (Note, if there is a base URL with a pathname ending in a slash, then chromium does collapse correctly.)

I need to think about what the correct fix is here.

[wanderview]

I think maybe the solution could be pre-pending a prefix like "/-" before we pass a value to the URL pathname parser if the value does not already start with a slash. Then we would strip it back off at the end.

[domenic]

I think this could benefit from some discussion with users, if possible. It kind of depends how much people expect these sort of strings to be "path-like" (where ./foo is equivalent to foo) or "regexp-like", and how inserting :bar in various locations changes their intuition. I don't have much of a handle on that...

[wanderview]

I'm not sure users will have a strong informed opinion here. Its a bit esoteric.

Also, there is a rationale for the chromium behavior. We only perform . and .. collapsing when we are certain there are slashes on both sides.

The issue with the non-chromium behavior is an artifact of how we have to use the URL parser algorithm that implicitly inserts a leading / because its resolving full URLs. We basically don't want that behavior here.

I also think keeping new URLPattern({ pathname: "./foo" }) as ./foo is principled because it has not been resolved against a base URL. Its an unresolved path and so the leading ./ does not meet the criteria for collapsing.

Having thought this through more I feel pretty strongly that we should stick with the chromium behavior.

[wanderview]

FWIW, prepending /- and then later removing it fixes all the issues in the polyfill.

[wanderview]

Another issue I thought of is new URLPattern({ pathname: '/.*' }) like you are trying to match dot files. Unfortunately the URL parser ends up seeing /. in this situation and collapses the whole thing to /*. This one affects chromium, the spec, and the polyfill.

This one would be a bit more complex to fix. We would have to do something like check for additional content after a trailing /. and then bypass the encoder for the /. in that case.

We could also just leave this as a quirk of the API, but I worry that the "match dotfiles" case might actually be something people will hit. There is a work around with /(\\.)* I guess.

[wanderview]

I decided to move #156 (comment) off into its own issue at #159.