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

Use a single-string URLPattern for dictionaries #2689

Merged
merged 5 commits into from
Dec 11, 2023
Merged

Conversation

pmeenan
Copy link
Contributor

@pmeenan pmeenan commented Nov 27, 2023

This changes the URLPattern match to use a single match pattern to be consistent with other uses of URLPattern.

@pmeenan
Copy link
Contributor Author

pmeenan commented Nov 27, 2023

@jeremyroman @domenic this is an attempt at a change to have it use a single string for pattern construction. I don't feel great about the same-origin check part of the validation but I don't see why it wouldn't work.

It still allows for people to waste bytes by not using path-relative patterns so I may have to re-open the issue around that but should provide the consistency you were asking for.

This would be instead of PR #2688.

Copy link

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for working on this!

draft-ietf-httpbis-compression-dictionary.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-compression-dictionary.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-compression-dictionary.md Show resolved Hide resolved
Copy link

@jeremyroman jeremyroman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple nitpicks while passing by

draft-ietf-httpbis-compression-dictionary.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-compression-dictionary.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-compression-dictionary.md Outdated Show resolved Hide resolved
@pmeenan pmeenan merged commit 2f7267f into httpwg:main Dec 11, 2023
1 check passed
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 18, 2024
Currently the logic of constructor string parsing of URL Pattern [1] is
implemented in `blink::url_pattern::Parser` which is not accessible from
the network service.

We are planing to use this logic for parsing `match` value of
`Use-As-Dictionary` HTTP response header in the network service for
the CompressionDictionaryTransport feature. This behavior was introduced
in the spec by [2]. And we are planing to use this logic when
features::CompressionDictionaryTransportBackendVersion::kV2 is enabled.

So this CL move the logic in the `blink::url_pattern::Parser` class to
the new `liburlpattern::ConstructorStringParser` class in
third_party/liburlpattern/.

The differences from `blink::url_pattern::Parser` to
`liburlpattern::ConstructorStringParser` are following:

- Use new `liburlpattern::ConstructorStringParser::Result` structure
  instead of `blink::URLPatternInit`.
- Use new `liburlpattern::ConstructorStringParser::StringParserOptions`
  structure instead of `blink::URLPatternOptions`.
- Use new `liburlpattern::ConstructorStringParser::ComponentType` enum
  class instead of `blink::url_pattern::Component::Type`.
- The `Parse()` method receives a callback which handles the logic
  of "compute protocol matches a special scheme flag" [3] instead of
  compiling the protocol component inside the
  Parser::ComputeShouldTreatAsStandardURL() method.

[1]: https://urlpattern.spec.whatwg.org/#constructor-string-parsing
[2]: httpwg/http-extensions#2689
[3]: https://urlpattern.spec.whatwg.org/#compute-protocol-matches-a-special-scheme-flag


Bug: 1413922
Change-Id: I0c5db20302e77820efe6219285b6e9fff5db985e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5199751
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Patrick Meenan <pmeenan@chromium.org>
Reviewed-by: Shunya Shishido <sisidovski@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1248494}
@pmeenan pmeenan deleted the match branch January 31, 2024 15:11
aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 5, 2024
The spec of Compression Dictionary Transport has been changed by [1] to
use URLPattern [2] for the matching algorithm of dictionaries. Also [3]
changed the Use-As-Dictionary header’s "match" value to be parsed as a
constructor string of URLPattern.

This CL changes the behavior of Chromium to follow those spec changes
when V2 backend is enabled.

[1]: httpwg/http-extensions#2646
[2]: https://urlpattern.spec.whatwg.org/
[3]: httpwg/http-extensions#2689

Fuchsia-Binary-Size: Size increase is unavoidable.
Bug: 1413922
Change-Id: I6ffba1c8c016145822643a18bc4f18bb7f0ac35f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5232339
Reviewed-by: Patrick Meenan <pmeenan@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1256029}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants