Bug 157153

This is quite interesting, because in a slightly different context, percent escaping curly braces caused a serious issue (bug 116887).

Took me ten minutes, and a little angst that browsers would differ per place where you can put a URL, but the difference here is that bug 116887 is about the query string, which per https://url.spec.whatwg.org/#query-state should indeed not have { and } escaped. However, per https://url.spec.whatwg.org/#path-state the path uses https://url.spec.whatwg.org/#default-encode-set which does include { and }.

Thank you Anne! Seems pretty clear that we should fix this then.

I think of a URL as a string that needs parsing before obtaining its values. Both the encoded and the raw curly brace would unambiguously reduce to the raw curly brace after decoding the URL. I understand the advantage of the approach "accept liberally, output conservatively" in terms of the browser presenting DOM elements to Javascript, so the suggested correction makes sense to me. I see that RFC 3986, while formally calling for encoding any character outside the following characters, ALPHA / DIGIT / "-" / "." / "_" / "~" / "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "=" / ":" / "@" suggests that "it is sometimes better for usability to avoid percent-encoding" special characters. These two parts of the RFC look self-contradictory. As for bug 116887, I obtained the same response from Washington Post regardless of the encoding, and I interpret this as an early conservative output. Perhaps, the server did produce different results a few years back, and the conservative output of the browser on sending the request could work that around.

This difference is also evident from a much simpler test case: alert(new URL("https://webkit.org/{path}/file.html"))

Created attachment 279846 [details] Patch

This first patch is extremely under-tested, probably breaks something, and isn't optimized. I'm expecting an r-, but I'm interested if you think I'm going in the right direction. Our URL parser is a mess :(

Attachment 279846 [details] did not pass style-queue: ERROR: Source/WebCore/platform/URL.cpp:300: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/URL.cpp:302: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 279846 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279846&action=review > LayoutTests/fast/dom/DOMURL/parsing.html:61 > +shouldBe("breakDownURL('https://webkit.org/{path}/file.html?{query}')", "'protocol=https:, host=webkit.org, pathname=/%7Bpath%7D/file.html, search=?{query}, origin=https://webkit.org, toString=https://webkit.org/%7Bpath%7D/file.html?{query}'"); Can this also be tested with an end to end test (loading a resource with curly braces in the path)?

Comment on attachment 279846 [details] Patch Attachment 279846 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1383190 New failing tests: http/tests/misc/script-defer-after-slow-stylesheet.html fast/url/url-credentials-escaping.html fast/css/uri-token-parsing.html http/tests/security/no-popup-from-sandbox-top.html fast/loader/url-strip-cr-lf-tab.html fast/url/path.html

Created attachment 279849 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5

Comment on attachment 279846 [details] Patch Attachment 279846 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1383192 New failing tests: http/tests/misc/script-defer-after-slow-stylesheet.html fast/url/url-credentials-escaping.html fast/css/uri-token-parsing.html http/tests/security/no-popup-from-sandbox-top.html fast/loader/url-strip-cr-lf-tab.html fast/url/path.html

Created attachment 279850 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5

Comment on attachment 279846 [details] Patch Attachment 279846 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1383204 New failing tests: http/tests/misc/script-defer-after-slow-stylesheet.html fast/url/url-credentials-escaping.html fast/css/uri-token-parsing.html http/tests/security/no-popup-from-sandbox-top.html fast/loader/url-strip-cr-lf-tab.html fast/url/path.html

Created attachment 279856 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5

Comment on attachment 279846 [details] Patch Attachment 279846 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1383209 New failing tests: http/tests/misc/script-defer-after-slow-stylesheet.html fast/url/url-credentials-escaping.html fast/css/uri-token-parsing.html mathml/opentype/horizontal-munderover.html http/tests/security/no-popup-from-sandbox-top.html fast/loader/url-strip-cr-lf-tab.html fast/url/path.html

Created attachment 279857 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4

Per https://jsdom.github.io/whatwg-url/#url=aHR0cHM6Ly93ZWJraXQub3JnL3twYXRofS9maWxlLmh0bWw=&base=YWJvdXQ6Ymxhbms= we are now correct for this input.