Fix encodeHostName() for inputs containing %

https://bugs.webkit.org/show_bug.cgi?id=310982

Reviewed by Alex Christensen.

There were two issues with encodeHostName():

1. mapHostNames() would return early for ASCII-only input, even when
   that input contains %.
2. mapHostName() checked the wrong variable for %.
3. Fixing (1) meant URLs containing % could now reach the host-range
   detection code, which doesn't understand bracketed IPv6 syntax:
   for http://[::1]/path%20x it would pick '[' as the host, which
   then failed UIDNA. Skip IDN processing for host ranges starting
   with '[' in collectRangesThatNeedMapping().

We also fix various trivial issues in the URL code:

1. protocolIsFile() is redundant with hasSpecialScheme().
2. Some type mismatches.
3. A redundant call to codePointAt(i).

Test: Tools/TestWebKitAPI/Tests/WTF/cocoa/URLExtras.mm
Canonical link: https://commits.webkit.org/314167@main

Author: annevk

Source/WTF/wtf/URL.cpp

@@ -766,7 +766,7 @@ void URL::setPath(StringView path)
 
     parseAllowingC0AtEnd(makeString(
         StringView(m_string).left(pathStart()),
-        path.startsWith('/') || (path.startsWith('\\') && (hasSpecialScheme() || protocolIsFile())) || (!hasSpecialScheme() && path.isEmpty() && m_schemeEnd + 1U < pathStart()) ? ""_s : "/"_s,
+        path.startsWith('/') || (path.startsWith('\\') && hasSpecialScheme()) || (!hasSpecialScheme() && path.isEmpty() && m_schemeEnd + 1U < pathStart()) ? ""_s : "/"_s,
         !hasSpecialScheme() && host().isEmpty() && path.startsWith("//"_s) && path.length() > 2 ? "/."_s : ""_s,
         escapePathWithoutCopying(path),
         StringView(m_string).substring(m_pathEnd)

Source/WTF/wtf/URL.h

@@ -138,7 +138,7 @@ class URL {
     bool isValid() const;
 
     // Since we overload operator NSURL * we have this to prevent accidentally using that operator
-    // when placing a URL in an if statment.
+    // when placing a URL in an if statement.
     operator bool() const = delete;
 
     const String& string() const LIFETIME_BOUND { return m_string; }

Source/WTF/wtf/URLHelpers.cpp

@@ -643,7 +643,7 @@ std::optional<String> mapHostName(const String& hostName, URLDecodeFunction deco
         return String();
 
     String string;
-    if (decodeFunction && string.contains('%'))
+    if (decodeFunction && hostName.contains('%'))
         string = (*decodeFunction)(hostName);
     else
         string = hostName;
@@ -680,6 +680,10 @@ static void collectRangesThatNeedMapping(const String& string, unsigned location
     // Generally, we want to optimize for the case where there is one host name that does not need mapping.
     // Therefore, we use null to indicate no mapping here and an empty array to indicate error.
 
+    // IPv6 addresses are bracketed and don't need IDN processing.
+    if (length && string[location] == '[')
+        return;
+
     String substring = string.substringSharingImpl(location, length);
     std::optional<String> host = mapHostName(substring, decodeFunction);
 
@@ -731,7 +735,7 @@ static void applyHostNameFunctionToMailToURLString(const String& string, URLDeco
                 current = hostNameEnd;
                 done = false;
             }
-            
+
             // Process host name range.
             collectRangesThatNeedMapping(string, hostNameStart, hostNameEnd - hostNameStart, array, decodeFunction);
 
@@ -812,7 +816,7 @@ String mapHostNames(const String& string, URLDecodeFunction decodeFunction)
 {
     // Generally, we want to optimize for the case where there is one host name that does not need mapping.
 
-    if (decodeFunction && string.containsOnlyASCII())
+    if (decodeFunction && string.containsOnlyASCII() && !string.contains('%'))
         return string;
 
     // Make a list of ranges that actually need mapping.
@@ -842,7 +846,7 @@ static String escapeUnsafeCharacters(const String& sourceBuffer)
     unsigned i;
     for (i = 0; i < length; ) {
         char32_t c = sourceBuffer.codePointAt(i);
-        if (isLookalikeCharacter(previousCodePoint, sourceBuffer.codePointAt(i)))
+        if (isLookalikeCharacter(previousCodePoint, c))
             break;
         previousCodePoint = c;
         i += U16_LENGTH(c);
@@ -888,7 +892,7 @@ static String escapeUnsafeCharacters(const String& sourceBuffer)
 String userVisibleURL(const CString& url)
 {
     auto before = url.span();
-    int length = url.length();
+    size_t length = url.length();
 
     if (!length)
         return { };
@@ -904,7 +908,7 @@ String userVisibleURL(const CString& url)
     size_t afterIndex = 0;
     {
         auto p = before;
-        for (int i = 0; i < length; i++) {
+        for (size_t i = 0; i < length; i++) {
             unsigned char c = p[i];
             // unescape escape sequences that indicate bytes greater than 0x7f
             if (c == '%' && i + 2 < length && isASCIIHexDigit(p[i + 1]) && isASCIIHexDigit(p[i + 2])) {
@@ -938,7 +942,7 @@ String userVisibleURL(const CString& url)
         // Shift current string to the end of the buffer
         // then we will copy back bytes to the start of the buffer 
         // as we convert.
-        int afterlength = afterIndex;
+        size_t afterlength = afterIndex;
         auto p = after.mutableSpan().subspan(bufferLength.value() - afterlength - 1);
         memmoveSpan(p, after.span().first(afterlength + 1)); // copies trailing '\0'
         afterIndex = 0;

Source/WTF/wtf/URLParser.cpp

@@ -350,7 +350,7 @@ ALWAYS_INLINE bool URLParser::isForbiddenDomainCodePoint(CharacterType character
     return character <= 0x7F && characterClassTable[character] & ForbiddenDomain;
 }
 
-ALWAYS_INLINE static bool shouldPercentEncodeQueryByte(uint8_t byte, const bool& urlIsSpecial)
+ALWAYS_INLINE static bool shouldPercentEncodeQueryByte(uint8_t byte, bool urlIsSpecial)
 {
     if (characterClassTable[byte] & QueryEncode)
         return true;

Tools/TestWebKitAPI/Tests/WTF/cocoa/URLExtras.mm

@@ -294,6 +294,30 @@
     EXPECT_WK_STREQ("site.com\xE3\x80\x80othersite.org", [@"site.com\xE3\x80\x80othersite.org" _wk_decodeHostName]);
 }
 
+TEST(URLExtras, URLExtras_PercentEncodedIDN)
+{
+    // Percent-encoded IDN hostnames should be decoded before UIDNA processing.
+    // m%C3%BCnchen.de should decode to münchen.de, then encode to xn--mnchen-3ya.de.
+    EXPECT_STREQ("xn--mnchen-3ya.de", [WTF::encodeHostName(@"m%C3%BCnchen.de") UTF8String]);
+    EXPECT_STREQ("http://xn--mnchen-3ya.de/", originalDataAsString(WTF::URLWithUserTypedString(@"http://m%C3%BCnchen.de/", nil)));
+}
+
+TEST(URLExtras, URLExtras_IPv6)
+{
+    // IPv6 hosts pass through unchanged.
+    EXPECT_STREQ("http://[::1]/", originalDataAsString(WTF::URLWithUserTypedString(@"http://[::1]/", nil)));
+    EXPECT_STREQ("http://[::1]:8080/", originalDataAsString(WTF::URLWithUserTypedString(@"http://[::1]:8080/", nil)));
+
+    // IPv6 hosts in URLs containing '%' must skip IDN processing rather than be
+    // mistaken for a hostname starting with '['.
+    EXPECT_STREQ("http://[::1]/path%20x", originalDataAsString(WTF::URLWithUserTypedString(@"http://[::1]/path%20x", nil)));
+    EXPECT_STREQ("http://[::1]:8080/?q=%20", originalDataAsString(WTF::URLWithUserTypedString(@"http://[::1]:8080/?q=%20", nil)));
+    EXPECT_STREQ("http://user@[::1]/path%20x", originalDataAsString(WTF::URLWithUserTypedString(@"http://user@[::1]/path%20x", nil)));
+
+    // Same for mailto: URLs whose address is an IPv6 literal.
+    EXPECT_STREQ("mailto:user@[::1]?subject=hi%20there", originalDataAsString(WTF::URLWithUserTypedString(@"mailto:user@[::1]?subject=hi%20there", nil)));
+}
+
 TEST(URLExtras, URLExtras_File)
 {
     EXPECT_STREQ("file:///%E2%98%83", [[WTF::URLWithUserTypedString(@"file:///☃", nil) absoluteString] UTF8String]);