Bug 174051 – REGRESSION(r215096) Queries of URLs with non-special schemes should not percent-encode single quotes

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

REGRESSION(r215096) Queries of URLs with non-special schemes should not percent-encode single quotes

[Alex Christensen]

Created attachment 314310 [details]
Patch

[Tim Horton]

Comment on attachment 314310 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314310&action=review

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=174051

RADAR

> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:1
> -/*
> +/*

Best revert this change

[Alex Christensen]

http://trac.webkit.org/r219024

[Alex Christensen]

http://trac.webkit.org/r219030

[Matt Lewis]

It looks like this also caused these test to fail:
http/tests/security/no-popup-from-sandbox-top.html
imported/w3c/web-platform-tests/url/url-setters.html

Would these be a rebaseline like fast/events/popup-blocked-from-unique-frame-via-window-open-named-sibling-frame.html ?


Build
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r219025%20(2667)/results.html

http/tests/security/no-popup-from-sandbox-top.html diff:
--- /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/http/tests/security/no-popup-from-sandbox-top-expected.txt
+++ /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/http/tests/security/no-popup-from-sandbox-top-actual.txt
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/no-popup-from-sandbox-top.html' from frame with URL 'data:text/html,       <script>       var win = window.open('about:blank', '_top');       alert(win ?%20%27FAIL%27%20:%20%27PASS%27);%20%20%20%20%20%20%20%3C/script%3E'. The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.
+CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/no-popup-from-sandbox-top.html' from frame with URL 'data:text/html,       <script>       var win = window.open('about:blank', '_top');       alert(win ?%20'FAIL'%20:%20'PASS');%20%20%20%20%20%20%20%3C/script%3E'. The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.
 
 ALERT: PASS
 To run this test outside of DumpRenderTree, please disable your popup blocker!


imported/w3c/web-platform-tests/url/url-setters.html diff:
--- /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/url/url-setters-expected.txt
+++ /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/url/url-setters-actual.txt
@@ -552,12 +552,12 @@
 PASS URL: Setting <https://example.net>.search = '' 
 PASS <a>: Setting <https://example.net>.search = '' 
 PASS <area>: Setting <https://example.net>.search = '' 
-FAIL URL: Setting <a:/>.search = '\0	
-\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. assert_equals: expected "a:/?%00%01%1F%20!%22%23$%&'()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9" but got "a:/?%00%01%1F%20!%22%23$%&%27()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9"
-FAIL <a>: Setting <a:/>.search = '\0	
-\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. assert_equals: expected "a:/?%00%01%1F%20!%22%23$%&'()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9" but got "a:/?%00%01%1F%20!%22%23$%&%27()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9"
-FAIL <area>: Setting <a:/>.search = '\0	
-\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. assert_equals: expected "a:/?%00%01%1F%20!%22%23$%&'()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9" but got "a:/?%00%01%1F%20!%22%23$%&%27()*+,-./09:;%3C=%3E?@AZ[\\]^_`az{|}~%7F%C2%80%C2%81%C3%89%C3%A9"
+PASS URL: Setting <a:/>.search = '\0	
+\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. 
+PASS <a>: Setting <a:/>.search = '\0	
+\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. 
+PASS <area>: Setting <a:/>.search = '\0	
+\r !"#$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~Éé' UTF-8 percent encoding with the query encode set. Tabs and newlines are removed. 
 PASS URL: Setting <http://example.net>.search = '%c3%89té' Bytes already percent-encoded are left as-is 
 PASS <a>: Setting <http://example.net>.search = '%c3%89té' Bytes already percent-encoded are left as-is 
 PASS <area>: Setting <http://example.net>.search = '%c3%89té' Bytes already percent-encoded are left as-is

[Alexey Proskuryakov]

What was the resolution of this? Are tests still failing?

I don't understand why the patch wasn't immediately rolled out if it broke tests.

[Matt Lewis]

Reverted r219030 for reason:

This was a rebaseline of a test that was broken with revision r219024

Committed r219072: <http://trac.webkit.org/changeset/219072>

[Matt Lewis]

Reverted r219024 for reason:

This patch cause 3 didferent test to fail.

Committed r219073: <http://trac.webkit.org/changeset/219073>

[Alex Christensen]

http://trac.webkit.org/r219074

[Alex Christensen]

The tests should've been rebaselined or the patch should've been rolled out immediately.

Re-landed in http://trac.webkit.org/r219076 after tests had been fixed.  Will watch bots.

[Alex Christensen]

http://trac.webkit.org/r219086