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

URLPattern #566

Closed
wanderview opened this issue Aug 13, 2021 · 13 comments · Fixed by #893
Closed

URLPattern #566

wanderview opened this issue Aug 13, 2021 · 13 comments · Fixed by #893
Labels
position: positive venue: Proposal Proposal not associated with a community group or standards org venue: WHATWG Specifications in a WHATWG Workstream

Comments

@wanderview
Copy link

Request for Mozilla Position on an Emerging Web Specification

Other information

Previously discussed as part of service worker scope pattern matching at:

We also have some web developer feedback that may be helpful.

Thanks.

@wanderview wanderview changed the title URLPattern Request for Position: URLPattern Aug 31, 2021
@wanderview
Copy link
Author

FYI, there is now an intent-to-ship for this feature in chromium.

@annevk
Copy link
Contributor

annevk commented Sep 6, 2021

Is there a fully conforming implementation of this specification? I worry a bit about Chromium's internals leaking due to it not having a compliant URL parser.

I also left some minor comments directly on the repository and asked for feedback in whatwg/url.

@valenting @asutherland did you have a chance to look at this already?

@wanderview
Copy link
Author

wanderview commented Sep 7, 2021

Is there a fully conforming implementation of this specification? I worry a bit about Chromium's internals leaking due to it not having a compliant URL parser.

The polyfill passes all WPT tests in both chromium and non-chromium environments. It uses new URL() under the hood for canonicalization wherever it can (although there are some cases that are made difficult by URL API).

The deno project is also working on a rust implementation. As second implementer they are catching a lot of the inevitable mistakes in the spec (which we are fixing as reported). I'm not sure how close they are to completion of their implementation.

@asutherland
Copy link
Member

@annevk I have not been following the evolution of the spec text proper but have been following the issues and discussion since the initial explainer.

URLPattern has a clear use case and it was appropriate for it to be split out into its own spec independent of the ServiceWorkers spec proper to be used as a future primitive for ServiceWorkers to build on.

It makes sense to me to mark the spec as worth prototyping.

@lucacasonato
Copy link

@annevk Our (Deno) implementation is now complete. There were a few "chromiumisms" in the spec, but they have been all ironed out now. We pass all WPT test cases now.

We managed to implement the entire thing on-top of rust-url's public API (which is more compliant than Chromium's URL parser), so it is implementable on top of other URL parsers.

The implementation is written in Rust, so a bunch of the code could just be reused for Gecko, by pulling in the crate. I'd be happy to collaborate on this front if there is interest - just reach out.

@annevk
Copy link
Contributor

annevk commented Sep 8, 2021

I looked through the tests and I didn't immediately see any for the problematic cases I know about, such as non-special schemes and hosts or relative paths, input such as %6A not getting decoded, etc. I guess that's why the polyfill manages to pass all the tests? I think we should add those those tests. If the polyfill is on top of new URL() a more realistic outcome would be that only Safari passes all tests at this point. And perhaps the Deno implementation. (I'm not sure how far along the rust-url implementation is these days.)

Overall though I agree that it looks pretty solid.

@valenting
Copy link

Thanks for the info @lucacasonato - I'll take a look at the possibilities for using the crate in Gecko.
I agree with @asutherland that this is worth prototyping

@annevk annevk added venue: Proposal Proposal not associated with a community group or standards org venue: WHATWG Specifications in a WHATWG Workstream labels Sep 8, 2021
@wanderview
Copy link
Author

wanderview commented Sep 8, 2021

I looked through the tests and I didn't immediately see any for the problematic cases

I think we should add those those tests

I'm not opposed to adding more tests hitting places where chrome gets URL encoding wrong, etc. I do think fixing URL interop is a separate problem than URLPattern itself, though. I believe @TimothyGu has recently done a bunch of work to move the ball forward on chrome URL interop.

Edit: Also, I want to note that URLPattern purposely does not specify URL encoding and canonicalization itself, but calls out to the URL spec. And its expected that implementations should use the platform's URL encoding as well (whether its right or wrong). This is so code like this works:

let p = new URLPattern({ hostname: 'café.example.com' });
p.test(new URL('https://café.example.com/foo'));

@annevk
Copy link
Contributor

annevk commented Sep 9, 2021

Yeah, I think it's great how it delegates. I just want to make sure we exercise some of the known problems to ensure it holds up.

@asutherland @valenting would either of you want to create a PR for this?

@wanderview
Copy link
Author

wanderview commented Sep 9, 2021

FWIW, I think I can at least close one interop issue by making chromium's URLPattern throw for more illegal hostname code points. That should at least avoid making it harder to fix that URL interop issue in the future.

Edit: And I'll add WPT for these code points as part of doing this.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2021
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 10, 2021
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 10, 2021
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}
pull bot pushed a commit to ZSX-JOJO/chromium that referenced this issue Sep 10, 2021
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 27, 2021
…me code points., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on more illegal hostname code points.

As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}

--

wpt-commits: 50f3e3a19f6d819268d01143fa4d0146dbdecb59
wpt-pr: 30471
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Sep 29, 2021
…me code points., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on more illegal hostname code points.

As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}

--

wpt-commits: 50f3e3a19f6d819268d01143fa4d0146dbdecb59
wpt-pr: 30471
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 29, 2021
…me code points., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on more illegal hostname code points.

As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}

--

wpt-commits: 50f3e3a19f6d819268d01143fa4d0146dbdecb59
wpt-pr: 30471
aosmond pushed a commit to aosmond/gecko that referenced this issue Sep 30, 2021
…me code points., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on more illegal hostname code points.

As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}

--

wpt-commits: 50f3e3a19f6d819268d01143fa4d0146dbdecb59
wpt-pr: 30471
Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this issue Nov 18, 2021
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}
@fregante
Copy link

Pasting the bugzilla tracking URL:

https://bugzilla.mozilla.org/show_bug.cgi?id=1731418

@karlcow
Copy link

karlcow commented Sep 26, 2022

Paper trail: WebKit standard Position

@wanderview
Copy link
Author

FYI, we are making a small enhancement to URLPattern to add the option to perform case-insensitive matching. This was a web developer community request. Please see:

https://groups.google.com/a/chromium.org/g/blink-dev/c/hNUoDg94-_8/m/-Cnv2qyMAAAJ
whatwg/urlpattern#148

Note, since this is a small change I thought it might be better to comment here instead of creating a new issue. If you would prefer a new issue, please let me know.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
As discussed in the mozilla standards-position issue on URLPattern:

mozilla/standards-positions#566

There are a number of URL encoding interop issues between browsers.
Since URLPattern delegates to the URL parser for canonicalization it
is also exposed to these interop issues.

While fixing all of URL's interop issues is orthogonal to shipping
URLPattern, we can at least prevent clearly illegal code points from
being used in URLPattern.  This will avoid increasing usage of these
code points which will in turn make it slightly easier to fix the issue
in the URL parser in the future.

Bug: 1248061
Change-Id: I58a55e4206004a6cba727f19d537c5929ce16d43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3152036
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#920223}
NOKEYCHECK=True
GitOrigin-RevId: 70a691d8a4840ab2ac989031d207b42834a80482
asutherland added a commit to asutherland/standards-positions that referenced this issue Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
position: positive venue: Proposal Proposal not associated with a community group or standards org venue: WHATWG Specifications in a WHATWG Workstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants