Title: [236528] trunk
Revision
236528
Author
achristen...@apple.com
Date
2018-09-26 14:58:29 -0700 (Wed, 26 Sep 2018)

Log Message

URLs with mismatched surrogate pairs in the host should fail to parse
https://bugs.webkit.org/show_bug.cgi?id=190005

Reviewed by Chris Dumez.

Source/WebCore:

Elsewhere in the URLParser, when we encounter mismatched surrogate pairs we use the replacement character,
but that just fails later on in domainToASCII, so we may as well just fail.
This behavior matches Chrome, but is unclear in the spec.  There are no valid uses of hosts containing mismatched surrogate pairs.
Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::URLParser::parseHostAndPort):

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (236527 => 236528)


--- trunk/Source/WebCore/ChangeLog	2018-09-26 21:56:31 UTC (rev 236527)
+++ trunk/Source/WebCore/ChangeLog	2018-09-26 21:58:29 UTC (rev 236528)
@@ -1,5 +1,20 @@
 2018-09-26  Alex Christensen  <achristen...@webkit.org>
 
+        URLs with mismatched surrogate pairs in the host should fail to parse
+        https://bugs.webkit.org/show_bug.cgi?id=190005
+
+        Reviewed by Chris Dumez.
+
+        Elsewhere in the URLParser, when we encounter mismatched surrogate pairs we use the replacement character,
+        but that just fails later on in domainToASCII, so we may as well just fail.
+        This behavior matches Chrome, but is unclear in the spec.  There are no valid uses of hosts containing mismatched surrogate pairs.
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::parseHostAndPort):
+
+2018-09-26  Alex Christensen  <achristen...@webkit.org>
+
         uidna_nameToASCII only needs a buffer capacity of 64
         https://bugs.webkit.org/show_bug.cgi?id=190006
 

Modified: trunk/Source/WebCore/platform/URLParser.cpp (236527 => 236528)


--- trunk/Source/WebCore/platform/URLParser.cpp	2018-09-26 21:56:31 UTC (rev 236527)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2018-09-26 21:58:29 UTC (rev 236528)
@@ -2753,12 +2753,11 @@
         if (UNLIKELY(!isASCII(*iterator)))
             syntaxViolation(hostBegin);
 
+        if (!U_IS_UNICODE_CHAR(*iterator))
+            return false;
         uint8_t buffer[U8_MAX_LENGTH];
         int32_t offset = 0;
-        UBool error = false;
-        U8_APPEND(buffer, offset, U8_MAX_LENGTH, *iterator, error);
-        ASSERT_WITH_SECURITY_IMPLICATION(offset <= static_cast<int32_t>(sizeof(buffer)));
-        // FIXME: Check error.
+        U8_APPEND_UNSAFE(buffer, offset, *iterator);
         utf8Encoded.append(buffer, offset);
     }
     LCharBuffer percentDecoded = percentDecode(utf8Encoded.data(), utf8Encoded.size(), hostBegin);

Modified: trunk/Tools/ChangeLog (236527 => 236528)


--- trunk/Tools/ChangeLog	2018-09-26 21:56:31 UTC (rev 236527)
+++ trunk/Tools/ChangeLog	2018-09-26 21:58:29 UTC (rev 236528)
@@ -1,5 +1,15 @@
 2018-09-26  Alex Christensen  <achristen...@webkit.org>
 
+        URLs with mismatched surrogate pairs in the host should fail to parse
+        https://bugs.webkit.org/show_bug.cgi?id=190005
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
+2018-09-26  Alex Christensen  <achristen...@webkit.org>
+
         URLWithUserTypedString should return nil for URLs deemed to be invalid by WebCore::URL
         https://bugs.webkit.org/show_bug.cgi?id=189979
         <rdar://problem/44119696>

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (236527 => 236528)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2018-09-26 21:56:31 UTC (rev 236527)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2018-09-26 21:58:29 UTC (rev 236528)
@@ -1257,9 +1257,12 @@
     const wchar_t surrogateBegin = 0xD800;
     const wchar_t validSurrogateEnd = 0xDD55;
     const wchar_t invalidSurrogateEnd = 'A';
+    const wchar_t replacementCharacter = 0xFFFD;
     checkURL(utf16String<12>({'h', 't', 't', 'p', ':', '/', '/', 'w', '/', surrogateBegin, validSurrogateEnd, '\0'}),
         {"http", "", "", "w", 0, "/%F0%90%85%95", "", "", "http://w/%F0%90%85%95"}, testTabsValueForSurrogatePairs);
-
+    shouldFail(utf16String<10>({'h', 't', 't', 'p', ':', '/', surrogateBegin, invalidSurrogateEnd, '/', '\0'}));
+    shouldFail(utf16String<9>({'h', 't', 't', 'p', ':', '/', replacementCharacter, '/', '\0'}));
+    
     // URLParser matches Chrome and Firefox but not URL::parse.
     checkURLDifferences(utf16String<12>({'h', 't', 't', 'p', ':', '/', '/', 'w', '/', surrogateBegin, invalidSurrogateEnd}),
         {"http", "", "", "w", 0, "/%EF%BF%BDA", "", "", "http://w/%EF%BF%BDA"},
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to