Bug 232034 - URLParser should reject hosts with C0 control characters or U+007F
Summary: URLParser should reject hosts with C0 control characters or U+007F
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-20 12:35 PDT by Alex Christensen
Modified: 2021-10-20 17:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (25.37 KB, patch)
2021-10-20 12:36 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (28.46 KB, patch)
2021-10-20 15:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-10-20 12:35:10 PDT
URLParser should reject hosts with C0 control characters or U+007F
Comment 1 Alex Christensen 2021-10-20 12:36:06 PDT
Created attachment 441916 [details]
Patch
Comment 2 EWS Watchlist 2021-10-20 12:37:22 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Timothy Gu 2021-10-20 13:51:51 PDT
Comment on attachment 441916 [details]
Patch

Thanks for the speedy fix!

>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
[...]
>+        This matches Chrome and Firefox and was proposed to the standard at
>+        https://github.com/whatwg/url/issues/627

I was incorrect in my initial testing: Firefox does not throw for \u007F apparently, but it does throw for %7F. So it almost matches Firefox, and completely matches Chrome.

[...]

>diff --git a/LayoutTests/imported/w3c/web-platform-tests/url/url-character-sets.any.js b/LayoutTests/imported/w3c/web-platform-tests/url/url-character-sets.any.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..70beab1a92fcd03faca00dcaa4bbe0c93195fe24
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/url/url-character-sets.any.js
>@@ -0,0 +1,12 @@
>+for (let cp = 0; cp <= 0x7f; cp++) {
>+  test(() => {
>+    let caught = false
>+    try {
>+      new URL("https://" + String.fromCodePoint(cp));
>+      new URL("not-special://" + String.fromCodePoint(cp));

Could we split the two cases into two separate test()s? It'd make my life a lot easier as Chrome currently doesn't support hosts in non-special URLs.
Comment 4 Alex Christensen 2021-10-20 15:16:52 PDT
Created attachment 441947 [details]
Patch
Comment 5 Alex Christensen 2021-10-20 15:18:17 PDT
The non-special URL host parsing actually has different rules.  There is yet some standards work to be done there that is not in the scope of this change, so I omitted it.  I did, however, add a percent-encoded test that shows Firefox's current behavior with %7f
Comment 6 EWS 2021-10-20 17:03:38 PDT
Committed r284588 (243321@main): <https://commits.webkit.org/243321@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441947 [details].
Comment 7 Radar WebKit Bug Importer 2021-10-20 17:04:27 PDT
<rdar://problem/84485556>