Remove invalid assertion in URLParser::domainToASCII
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jul 2019 19:13:52 +0000 (19:13 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jul 2019 19:13:52 +0000 (19:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199624

Reviewed by Brady Eidson.

Source/WTF:

When a non-ASCII domain's punycode encoding is longer than 63 characters,
the URL is supposed to fail to parse, according to https://www.unicode.org/reports/tr46/#ToASCII
We and all other browsers do this.  When uidna_nameToASCII tries to punycode encode such a domain,
it can return a number larger than 63.  In this case, processingDetails.errors is UIDNA_ERROR_LABEL_TOO_LONG
and we fail to parse as desired and uidna_nameToASCII did not write out of bounds, so it was just the assertion
that was wrong.  I added some unit tests that would have hit the assertion and verify that we fail at the correct length.

* wtf/URLParser.cpp:
(WTF::URLParser::domainToASCII):

Tools:

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247268 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp

index 071fbdc..7daa26f 100644 (file)
@@ -1,5 +1,22 @@
 2019-07-09  Alex Christensen  <achristensen@webkit.org>
 
+        Remove invalid assertion in URLParser::domainToASCII
+        https://bugs.webkit.org/show_bug.cgi?id=199624
+
+        Reviewed by Brady Eidson.
+
+        When a non-ASCII domain's punycode encoding is longer than 63 characters,
+        the URL is supposed to fail to parse, according to https://www.unicode.org/reports/tr46/#ToASCII
+        We and all other browsers do this.  When uidna_nameToASCII tries to punycode encode such a domain,
+        it can return a number larger than 63.  In this case, processingDetails.errors is UIDNA_ERROR_LABEL_TOO_LONG
+        and we fail to parse as desired and uidna_nameToASCII did not write out of bounds, so it was just the assertion
+        that was wrong.  I added some unit tests that would have hit the assertion and verify that we fail at the correct length.
+
+        * wtf/URLParser.cpp:
+        (WTF::URLParser::domainToASCII):
+
+2019-07-09  Alex Christensen  <achristensen@webkit.org>
+
         URLParser should reserialize IPv6 addresses that end with a colon
         https://bugs.webkit.org/show_bug.cgi?id=199627
 
index b937660..c6b3e7d 100644 (file)
@@ -2567,13 +2567,16 @@ template<typename CharacterType> Optional<URLParser::LCharBuffer> URLParser::dom
     UErrorCode error = U_ZERO_ERROR;
     UIDNAInfo processingDetails = UIDNA_INFO_INITIALIZER;
     int32_t numCharactersConverted = uidna_nameToASCII(&internationalDomainNameTranscoder(), StringView(domain).upconvertedCharacters(), domain.length(), hostnameBuffer, maxDomainLength, &processingDetails, &error);
-    ASSERT(numCharactersConverted <= static_cast<int32_t>(maxDomainLength));
 
     if (U_SUCCESS(error) && !processingDetails.errors) {
+#if ASSERT_DISABLED
+        UNUSED_PARAM(numCharactersConverted);
+#else
         for (int32_t i = 0; i < numCharactersConverted; ++i) {
             ASSERT(isASCII(hostnameBuffer[i]));
             ASSERT(!isASCIIUpper(hostnameBuffer[i]));
         }
+#endif
         ascii.append(hostnameBuffer, numCharactersConverted);
         if (domain != StringView(ascii.data(), ascii.size()))
             syntaxViolation(iteratorForSyntaxViolationPosition);
index cb1d24a..076c062 100644 (file)
@@ -1,5 +1,15 @@
 2019-07-09  Alex Christensen  <achristensen@webkit.org>
 
+        Remove invalid assertion in URLParser::domainToASCII
+        https://bugs.webkit.org/show_bug.cgi?id=199624
+
+        Reviewed by Brady Eidson.
+
+        * TestWebKitAPI/Tests/WTF/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
+2019-07-09  Alex Christensen  <achristensen@webkit.org>
+
         URLParser should reserialize IPv6 addresses that end with a colon
         https://bugs.webkit.org/show_bug.cgi?id=199627
 
index 06ddefb..d56683d 100644 (file)
@@ -777,8 +777,11 @@ TEST_F(WTF_URLParser, ParserDifferences)
     checkURLDifferences("http://abcd%7Xefg",
         {"", "", "", "", 0, "", "", "", "http://abcd%7Xefg"},
         {"http", "", "", "abcd%7xefg", 0, "/", "", "", "http://abcd%7xefg/"});
+    checkURL("ws://äAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", {"ws", "", "", "xn--aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-rsb254a", 0, "/", "", "", "ws://xn--aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-rsb254a/"}, TestTabs::No);
+    shouldFail("ws://äAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
+    checkURL("ws://&äAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", {"ws", "", "", "xn--&aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-ssb254a", 0, "/", "", "", "ws://xn--&aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-ssb254a/"}, TestTabs::No);
+    shouldFail("ws://&äAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
 
-    
     // URLParser matches Chrome and the spec, but not URL::parse or Firefox.
     checkURLDifferences(utf16String(u"http://0Xc0.0250.01"),
         {"http", "", "", "192.168.0.1", 0, "/", "", "", "http://192.168.0.1/"},