Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter #685

karwa · 2022-01-25T05:59:38Z

Attempt at resolving #627 and #319

At least two implementers are interested (and none opposed):
- Chrome (Presumably - See linked issue)
- WebKit (Presumably - See linked issue)
Tests are written and can be reviewed and commented upon at:
- [URL] Split forbidden host/domain code points and add more tests for each web-platform-tests/wpt#32524
Implementation bugs are filed:

(See WHATWG Working Mode: Changes for more details.)

…U+007F to the latter

…de point

annevk

This looks good to me. I don't think the conformance requirements end up needing adjusting (and it seems like this fixes a small problem for "valid opaque-host string"), although for "valid domain string" it's a bit unclear due to the imperative nature of its definition.

I'd appreciate a second review.

domenic · 2022-01-25T17:57:22Z

Chrome (Presumably - See linked issue)

Which part of which linked issue are you referring to?

karwa · 2022-01-25T18:07:10Z

Chrome and Go forbid C0 controls and \u007F, whether encoded or not

@domenic This is Chrome’s existing behaviour for C0 controls and U+007F in domains. This PR makes no changes to opaque hosts compared to the status quo.

Apologies for not making that clearer.

domenic · 2022-01-25T18:13:20Z

Hmm. But https://wpt.fyi/results/url?diff&filter=ADC&run_id=5656521351364608&run_id=5648068402741248 shows many new failing tests in Chrome. Are all of those tests that would already fail with the current spec? Maybe it'd be best to separate out the tests PR into one that tests the current spec, and one that tests the change here, so that we can actually see if the change here causes any deltas from the current spec?

karwa · 2022-01-25T18:29:09Z

There are more failures because there are more tests, but for the part which is changed by this spec change - domains containing (raw or encoded) code points in the range \u0000-\u001F or \u007F - Chrome passes them all.

In the WPT.fyi viewer, these are the ones like:

Parsing: <http://ho%01st/> against <about:blank>

As well as the ones just above them which all seem to look the same (because of the raw C0 controls):

Parsing: <http://ab/> against <about:blank>

If I changed the tests to the current spec, there would be even more failures for all browsers. None of them implement the current spec. WebKit was the last one, but @achristensen07 recently changed made the change to reject those code-points (for all hosts, not just domains, which is why WebKit still has one failure, but the consensus from the issue seemed to be to leave those alone for now):

I think there is a consensus that the forbidden host code points should include all C0 controls and U+007F, and that now matches the behavior of all browsers. Let's make that change to the specification and then make a separate issue about what we want to do with opaque hosts, where there is less consensus and less spec compliance.

#627 (comment)

domenic · 2022-01-25T19:35:12Z

If I changed the tests to the current spec, there would be even more failures for all browsers

Right. That would be helpful. Then we could double-check that the new tests, specific to the changes in this PR, are all passing in Chromium.

karwa · 2022-01-25T20:34:30Z

OK, I've put up web-platform-tests/wpt#32539

Same tests as linked for this change, but with expected values that match the current spec.

These give much more full coverage for host code points. The expectations will be changed in #32524 to follow the spec change proposed in whatwg/url#685; this initial PR gives us a baseline so we can be clear about what the changes are.

domenic · 2022-01-25T21:46:37Z

Editorially LGTM, let's just rebase web-platform-tests/wpt#32524 on WPT master, confirm that the result is more in alignment with browsers, and then we should be good to merge. Thanks @karwa for doing the work, including the testing against whatwg-url.

annevk · 2022-01-26T08:42:25Z

@karwa do you want to file the implementation bugs? https://github.com/whatwg/meta/blob/master/MAINTAINERS.md has pointers.

[Spec change]: whatwg/url#685

karwa · 2022-01-27T14:20:48Z

@annevk bugs have been filed.

annevk · 2022-01-28T09:05:20Z

Thanks a lot @karwa for your work on tackling this as well as providing all those great tests!

See whatwg/url#685 for context.

add all C0 controls and add U+007F to is_invalid_domain_char

Follows whatwg/url#685.

…ode points, a=testonly Automatic update from web-platform-tests URL: More tests of (currently) non-forbidden host code points These give much more full coverage for host code points. The expectations will be changed in #32524 to follow the spec change proposed in whatwg/url#685; this initial PR gives us a baseline so we can be clear about what the changes are. -- wpt-commits: 53a00b35ce2808bbbef9699ac4b6f0d5da46009a wpt-pr: 32539

See whatwg/url#685 for context.

…oints and add more tests for each, a=testonly Automatic update from web-platform-tests URL: forbid more code points in non-opaque domains See whatwg/url#685 for context. -- wpt-commits: 2a64dae4641fbd61bd4257df460e188f425b492e wpt-pr: 32524

See whatwg/url#685 for context.

…oints and add more tests for each, a=testonly Automatic update from web-platform-tests URL: forbid more code points in non-opaque domains See whatwg/url#685 for context. -- wpt-commits: 2a64dae4641fbd61bd4257df460e188f425b492e wpt-pr: 32524

whatwg/url#685

karwa added 2 commits January 25, 2022 06:53

Split forbidden host/domain code points, and add all C0 controls and …

adfe8f8

…U+007F to the latter

Move U+0025 (%) from forbidden host code point to forbidden domain co…

3229929

…de point

karwa mentioned this pull request Jan 25, 2022

Split forbidden host/domain code-points jsdom/whatwg-url#227

Merged

annevk requested a review from achristensen07 January 25, 2022 15:50

annevk approved these changes Jan 25, 2022

View reviewed changes

annevk mentioned this pull request Jan 25, 2022

Disallow all C0 control and DEL codepoints in hosts #673

Closed

3 tasks

domenic mentioned this pull request Jan 25, 2022

[URL] Split forbidden host/domain code points and add more tests for each web-platform-tests/wpt#32524

Merged

alwinb added a commit to alwinb/spec-url that referenced this pull request Jan 27, 2022

[Spec change] forbid C0-controls and DEL in domains

2d7359e

[Spec change]: whatwg/url#685

This was referenced Jan 27, 2022

[URL] Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter denoland/deno#13509

Closed

[URL] Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter nodejs/node#41717

Closed

crowlKats mentioned this pull request Jan 28, 2022

Specification alignment tracking issue servo/rust-url#742

Open

9 tasks

annevk merged commit 35e195a into whatwg:main Jan 28, 2022

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 28, 2022

URL: forbid more code points in non-opaque domains

2a64dae

See whatwg/url#685 for context.

crowlKats mentioned this pull request Jan 28, 2022

add all C0 controls and add U+007F to is_invalid_domain_char servo/rust-url#747

Merged

valenting added a commit to servo/rust-url that referenced this pull request Jan 28, 2022

Merge pull request #747 from crowlKats/whatwg/url#685

fc76310

add all C0 controls and add U+007F to is_invalid_domain_char

domenic pushed a commit to jsdom/whatwg-url that referenced this pull request Jan 28, 2022

Split forbidden host/domain code-points

a4cb133

Follows whatwg/url#685.

mattwoodrow pushed a commit to mattwoodrow/wpt that referenced this pull request Feb 15, 2022

URL: forbid more code points in non-opaque domains

c2a5c52

See whatwg/url#685 for context.

DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this pull request Feb 28, 2022

URL: forbid more code points in non-opaque domains

308ba62

See whatwg/url#685 for context.

karwa added a commit to karwa/swift-url that referenced this pull request Mar 19, 2022

[Standard update] Forbid C0 controls and U+007F in domains

f7452ff

whatwg/url#685

karwa mentioned this pull request Mar 19, 2022

[Standard update] Forbid C0 controls and U+007F in domains karwa/swift-url#137

Merged

karwa added a commit to karwa/swift-url that referenced this pull request Mar 19, 2022

[Standard update] Forbid C0 controls and U+007F in domains

764b09e

whatwg/url#685

F3n67u mentioned this pull request Jun 3, 2022

test, url: update WHATWG URL parser to align with latest spec nodejs/node#43190

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter #685

Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter #685

karwa commented Jan 25, 2022 •

edited by annevk

Loading

annevk left a comment

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022 •

edited

Loading

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022 •

edited

Loading

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022

domenic commented Jan 25, 2022

annevk commented Jan 26, 2022

karwa commented Jan 27, 2022

annevk commented Jan 28, 2022

Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter #685

Split forbidden host/domain code points, and add all C0 controls and add U+007F to the latter #685

Conversation

karwa commented Jan 25, 2022 • edited by annevk Loading

annevk left a comment

Choose a reason for hiding this comment

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022 • edited Loading

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022 • edited Loading

domenic commented Jan 25, 2022

karwa commented Jan 25, 2022

domenic commented Jan 25, 2022

annevk commented Jan 26, 2022

karwa commented Jan 27, 2022

annevk commented Jan 28, 2022

karwa commented Jan 25, 2022 •

edited by annevk

Loading

karwa commented Jan 25, 2022 •

edited

Loading

karwa commented Jan 25, 2022 •

edited

Loading