URLParser should match old URL::parse with %2E in path

https://bugs.webkit.org/show_bug.cgi?id=163929 Reviewed by Alexey Proskuryakov. LayoutTests/imported/w3c: * web-platform-tests/url/a-element-expected.txt: * web-platform-tests/url/a-element-xhtml-expected.txt: * web-platform-tests/url/url-constructor-expected.txt: Source/WebCore: Covered by updated API tests, which show that URLParser now matches URL::parse in these cases. Also covered by newly failing web platform tests, which were failing before URLParser was enabled. If whatwg/url#87 is resolved we can change behavior to match. * platform/URLParser.cpp: (WebCore::URLParser::isSingleDotPathSegment): (WebCore::URLParser::isDoubleDotPathSegment): (WebCore::URLParser::consumeSingleDotPathSegment): (WebCore::URLParser::consumeDoubleDotPathSegment): (WebCore::URLParser::parse): (WebCore::URLParser::isPercentEncodedDot): Deleted. Tools: * TestWebKitAPI/Tests/WebCore/URLParser.cpp: (TestWebKitAPI::TEST_F): LayoutTests: * fast/url/path-expected.txt: * fast/url/standard-url-expected.txt: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@207805 268f45cc-cd09-0410-ab3c-d52691b4dbfc
paul99 · Oct 25, 2016 · a804f38 · a804f38
1 parent 6fe0acd
commit a804f38
Show file tree

Hide file tree

Showing 11 changed files with 90 additions and 144 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2016-10-24  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should match old URL::parse with %2E in path
+        https://bugs.webkit.org/show_bug.cgi?id=163929
+
+        Reviewed by Alexey Proskuryakov.
+
+        * fast/url/path-expected.txt:
+        * fast/url/standard-url-expected.txt:
+
 2016-10-24  Zalan Bujtas  <zalan@apple.com>
 
         Do not update selection rect on dirty lineboxes.

diff --git a/LayoutTests/fast/url/path-expected.txt b/LayoutTests/fast/url/path-expected.txt
@@ -14,9 +14,9 @@ PASS canonicalize('http://example.com/foo/bar/../ton') is 'http://example.com/fo
 PASS canonicalize('http://example.com/foo/bar/../ton/../../a') is 'http://example.com/a'
 PASS canonicalize('http://example.com/foo/../../..') is 'http://example.com/'
 PASS canonicalize('http://example.com/foo/../../../ton') is 'http://example.com/ton'
-PASS canonicalize('http://example.com/foo/%2e') is 'http://example.com/foo/'
-PASS canonicalize('http://example.com/foo/%2e%2') is 'http://example.com/foo/.%2'
-PASS canonicalize('http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar') is 'http://example.com/..bar'
+FAIL canonicalize('http://example.com/foo/%2e') should be http://example.com/foo/. Was http://example.com/foo/%2e.
+FAIL canonicalize('http://example.com/foo/%2e%2') should be http://example.com/foo/.%2. Was http://example.com/foo/%2e%2.
+FAIL canonicalize('http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar') should be http://example.com/..bar. Was http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar.
 PASS canonicalize('http://example.com////../..') is 'http://example.com//'
 PASS canonicalize('http://example.com/foo/bar//../..') is 'http://example.com/foo/'
 PASS canonicalize('http://example.com/foo/bar//..') is 'http://example.com/foo/bar/'

diff --git a/LayoutTests/fast/url/standard-url-expected.txt b/LayoutTests/fast/url/standard-url-expected.txt
@@ -9,7 +9,7 @@ PASS canonicalize('http://[www.google.com]/') is 'http://[www.google.com]/'
 PASS canonicalize('http://www.google.com') is 'http://www.google.com/'
 PASS canonicalize('http:////////user:@google.com:99?foo') is 'http://user@google.com:99/?foo'
 PASS canonicalize('http://192.0x00A80001') is 'http://192.168.0.1/'
-PASS canonicalize('http://www/foo%2Ehtml') is 'http://www/foo.html'
+FAIL canonicalize('http://www/foo%2Ehtml') should be http://www/foo.html. Was http://www/foo%2Ehtml.
 PASS canonicalize('http://user:pass@/') is 'http://user:pass@/'
 PASS canonicalize('http://%25DOMAIN:foobar@foodomain.com/') is 'http://%25DOMAIN:foobar@foodomain.com/'
 PASS canonicalize('http:\\\\www.google.com\\foo') is 'http://www.google.com/foo'

diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,14 @@
+2016-10-24  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should match old URL::parse with %2E in path
+        https://bugs.webkit.org/show_bug.cgi?id=163929
+
+        Reviewed by Alexey Proskuryakov.
+
+        * web-platform-tests/url/a-element-expected.txt:
+        * web-platform-tests/url/a-element-xhtml-expected.txt:
+        * web-platform-tests/url/url-constructor-expected.txt:
+
 2016-10-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r207795.

diff --git a/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt
@@ -126,9 +126,9 @@ PASS Parsing: <http://example.com/foo/bar/../ton> against <about:blank>
 PASS Parsing: <http://example.com/foo/bar/../ton/../../a> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../../ton> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e%2> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> 
+FAIL Parsing: <http://example.com/foo/%2e> against <about:blank> assert_equals: href expected "http://example.com/foo/" but got "http://example.com/foo/%2e"
+FAIL Parsing: <http://example.com/foo/%2e%2> against <about:blank> assert_equals: href expected "http://example.com/foo/.%2" but got "http://example.com/foo/%2e%2"
+FAIL Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> assert_equals: href expected "http://example.com/..bar" but got "http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar"
 PASS Parsing: <http://example.com////../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//..> against <about:blank> 
@@ -157,8 +157,8 @@ FAIL Parsing: <data:test# »> against <about:blank> assert_equals: href expected
 PASS Parsing: <http://[www.google.com]/> against <about:blank> 
 PASS Parsing: <http://www.google.com> against <about:blank> 
 PASS Parsing: <http://192.0x00A80001> against <about:blank> 
-PASS Parsing: <http://www/foo%2Ehtml> against <about:blank> 
-PASS Parsing: <http://www/foo/%2E/html> against <about:blank> 
+FAIL Parsing: <http://www/foo%2Ehtml> against <about:blank> assert_equals: href expected "http://www/foo.html" but got "http://www/foo%2Ehtml"
+FAIL Parsing: <http://www/foo/%2E/html> against <about:blank> assert_equals: href expected "http://www/foo/html" but got "http://www/foo/%2E/html"
 PASS Parsing: <http://user:pass@/> against <about:blank> 
 PASS Parsing: <http://%25DOMAIN:foobar@foodomain.com/> against <about:blank> 
 PASS Parsing: <http:\\www.google.com\foo> against <about:blank> 

diff --git a/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt
@@ -126,9 +126,9 @@ PASS Parsing: <http://example.com/foo/bar/../ton> against <about:blank>
 PASS Parsing: <http://example.com/foo/bar/../ton/../../a> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../../ton> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e%2> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> 
+FAIL Parsing: <http://example.com/foo/%2e> against <about:blank> assert_equals: href expected "http://example.com/foo/" but got "http://example.com/foo/%2e"
+FAIL Parsing: <http://example.com/foo/%2e%2> against <about:blank> assert_equals: href expected "http://example.com/foo/.%2" but got "http://example.com/foo/%2e%2"
+FAIL Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> assert_equals: href expected "http://example.com/..bar" but got "http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar"
 PASS Parsing: <http://example.com////../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//..> against <about:blank> 
@@ -157,8 +157,8 @@ FAIL Parsing: <data:test# »> against <about:blank> assert_equals: href expected
 PASS Parsing: <http://[www.google.com]/> against <about:blank> 
 PASS Parsing: <http://www.google.com> against <about:blank> 
 PASS Parsing: <http://192.0x00A80001> against <about:blank> 
-PASS Parsing: <http://www/foo%2Ehtml> against <about:blank> 
-PASS Parsing: <http://www/foo/%2E/html> against <about:blank> 
+FAIL Parsing: <http://www/foo%2Ehtml> against <about:blank> assert_equals: href expected "http://www/foo.html" but got "http://www/foo%2Ehtml"
+FAIL Parsing: <http://www/foo/%2E/html> against <about:blank> assert_equals: href expected "http://www/foo/html" but got "http://www/foo/%2E/html"
 PASS Parsing: <http://user:pass@/> against <about:blank> 
 PASS Parsing: <http://%25DOMAIN:foobar@foodomain.com/> against <about:blank> 
 PASS Parsing: <http:\\www.google.com\foo> against <about:blank> 

diff --git a/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt
@@ -130,9 +130,9 @@ PASS Parsing: <http://example.com/foo/bar/../ton> against <about:blank>
 PASS Parsing: <http://example.com/foo/bar/../ton/../../a> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/../../../ton> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e%2> against <about:blank> 
-PASS Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> 
+FAIL Parsing: <http://example.com/foo/%2e> against <about:blank> assert_equals: href expected "http://example.com/foo/" but got "http://example.com/foo/%2e"
+FAIL Parsing: <http://example.com/foo/%2e%2> against <about:blank> assert_equals: href expected "http://example.com/foo/.%2" but got "http://example.com/foo/%2e%2"
+FAIL Parsing: <http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar> against <about:blank> assert_equals: href expected "http://example.com/..bar" but got "http://example.com/foo/%2e./%2e%2e/.%2e/%2e.bar"
 PASS Parsing: <http://example.com////../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//../..> against <about:blank> 
 PASS Parsing: <http://example.com/foo/bar//..> against <about:blank> 
@@ -161,8 +161,8 @@ FAIL Parsing: <data:test# »> against <about:blank> assert_equals: href expected
 PASS Parsing: <http://[www.google.com]/> against <about:blank> 
 PASS Parsing: <http://www.google.com> against <about:blank> 
 PASS Parsing: <http://192.0x00A80001> against <about:blank> 
-PASS Parsing: <http://www/foo%2Ehtml> against <about:blank> 
-PASS Parsing: <http://www/foo/%2E/html> against <about:blank> 
+FAIL Parsing: <http://www/foo%2Ehtml> against <about:blank> assert_equals: href expected "http://www/foo.html" but got "http://www/foo%2Ehtml"
+FAIL Parsing: <http://www/foo/%2E/html> against <about:blank> assert_equals: href expected "http://www/foo/html" but got "http://www/foo/%2E/html"
 PASS Parsing: <http://user:pass@/> against <about:blank> 
 PASS Parsing: <http://%25DOMAIN:foobar@foodomain.com/> against <about:blank> 
 PASS Parsing: <http:\\www.google.com\foo> against <about:blank> 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2016-10-24  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should match old URL::parse with %2E in path
+        https://bugs.webkit.org/show_bug.cgi?id=163929
+
+        Reviewed by Alexey Proskuryakov.
+
+        Covered by updated API tests, which show that URLParser now matches URL::parse in these cases.
+        Also covered by newly failing web platform tests, which were failing before URLParser was enabled.
+        If https://github.com/whatwg/url/issues/87 is resolved we can change behavior to match.
+
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::isSingleDotPathSegment):
+        (WebCore::URLParser::isDoubleDotPathSegment):
+        (WebCore::URLParser::consumeSingleDotPathSegment):
+        (WebCore::URLParser::consumeDoubleDotPathSegment):
+        (WebCore::URLParser::parse):
+        (WebCore::URLParser::isPercentEncodedDot): Deleted.
+
 2016-10-24  Zalan Bujtas  <zalan@apple.com>
 
         Do not update selection rect on dirty lineboxes.

diff --git a/Source/WebCore/platform/URLParser.cpp b/Source/WebCore/platform/URLParser.cpp
@@ -891,26 +891,6 @@ void URLParser::copyURLPartsUntil(const URL& base, URLPart part, const CodePoint
     ASSERT_NOT_REACHED();
 }
 
-static const char* dotASCIICode = "2e";
-
-template<typename CharacterType>
-ALWAYS_INLINE bool URLParser::isPercentEncodedDot(CodePointIterator<CharacterType> c)
-{
-    if (c.atEnd())
-        return false;
-    if (*c != '%')
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd())
-        return false;
-    if (*c != dotASCIICode[0])
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd())
-        return false;
-    return toASCIILower(*c) == dotASCIICode[1];
-}
-
 template<typename CharacterType>
 ALWAYS_INLINE bool URLParser::isSingleDotPathSegment(CodePointIterator<CharacterType> c)
 {
@@ -920,18 +900,6 @@ ALWAYS_INLINE bool URLParser::isSingleDotPathSegment(CodePointIterator<Character
         advance<CharacterType, ReportSyntaxViolation::No>(c);
         return c.atEnd() || isSlashQuestionOrHash(*c);
     }
-    if (*c != '%')
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd() || *c != dotASCIICode[0])
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd())
-        return false;
-    if (toASCIILower(*c) == dotASCIICode[1]) {
-        advance<CharacterType, ReportSyntaxViolation::No>(c);
-        return c.atEnd() || isSlashQuestionOrHash(*c);
-    }
     return false;
 }
 
@@ -944,63 +912,27 @@ ALWAYS_INLINE bool URLParser::isDoubleDotPathSegment(CodePointIterator<Character
         advance<CharacterType, ReportSyntaxViolation::No>(c);
         return isSingleDotPathSegment(c);
     }
-    if (*c != '%')
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd() || *c != dotASCIICode[0])
-        return false;
-    advance<CharacterType, ReportSyntaxViolation::No>(c);
-    if (c.atEnd())
-        return false;
-    if (toASCIILower(*c) == dotASCIICode[1]) {
-        advance<CharacterType, ReportSyntaxViolation::No>(c);
-        return isSingleDotPathSegment(c);
-    }
     return false;
 }
 
 template<typename CharacterType>
 void URLParser::consumeSingleDotPathSegment(CodePointIterator<CharacterType>& c)
 {
     ASSERT(isSingleDotPathSegment(c));
-    if (*c == '.') {
-        advance(c);
-        if (!c.atEnd()) {
-            if (*c == '/' || *c == '\\')
-                advance(c);
-            else
-                ASSERT(*c == '?' || *c == '#');
-        }
-    } else {
-        ASSERT(*c == '%');
-        advance(c);
-        ASSERT(*c == dotASCIICode[0]);
-        advance(c);
-        ASSERT(toASCIILower(*c) == dotASCIICode[1]);
-        advance(c);
-        if (!c.atEnd()) {
-            if (*c == '/' || *c == '\\')
-                advance(c);
-            else
-                ASSERT(*c == '?' || *c == '#');
-        }
+    advance(c);
+    if (!c.atEnd()) {
+        if (*c == '/' || *c == '\\')
+            advance(c);
+        else
+            ASSERT(*c == '?' || *c == '#');
     }
 }
 
 template<typename CharacterType>
 void URLParser::consumeDoubleDotPathSegment(CodePointIterator<CharacterType>& c)
 {
     ASSERT(isDoubleDotPathSegment(c));
-    if (*c == '.')
-        advance(c);
-    else {
-        ASSERT(*c == '%');
-        advance(c);
-        ASSERT(*c == dotASCIICode[0]);
-        advance(c);
-        ASSERT(toASCIILower(*c) == dotASCIICode[1]);
-        advance(c);
-    }
+    advance(c);
     consumeSingleDotPathSegment(c);
 }
 
@@ -1723,17 +1655,6 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                 state = State::Fragment;
                 break;
             }
-            if (UNLIKELY(isPercentEncodedDot(c))) {
-                syntaxViolation(c);
-                appendToASCIIBuffer('.');
-                ASSERT(*c == '%');
-                advance(c);
-                ASSERT(*c == dotASCIICode[0]);
-                advance(c);
-                ASSERT(toASCIILower(*c) == dotASCIICode[1]);
-                advance(c);
-                break;
-            }
             utf8PercentEncode<isInDefaultEncodeSet>(c);
             ++c;
             break;

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,13 @@
+2016-10-24  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should match old URL::parse with %2E in path
+        https://bugs.webkit.org/show_bug.cgi?id=163929
+
+        Reviewed by Alexey Proskuryakov.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-10-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r207795.