URL Parsing should signal failure for illegal IDN
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Mar 2016 18:31:32 +0000 (18:31 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Mar 2016 18:31:32 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154945
<rdar://problem/8014795>

Reviewed by Brent Fulgham.

Source/WebCore:

WebCore::URL will now invalidate URLs with illegal IDN. And functions inside WebCoreNSURLExtras.h
that deal with IDN mapping will now return nil to signal error.

Test: fast/url/invalid-idn.html

* platform/URL.cpp:
(WebCore::isSchemeFirstChar):
(WebCore::URL::init):
(WebCore::appendEncodedHostname):
(WebCore::encodeHostnames):
(WebCore::encodeRelativeString):
* platform/mac/WebCoreNSURLExtras.h:
* platform/mac/WebCoreNSURLExtras.mm:
(WebCore::mapHostNameWithRange):
(WebCore::hostNameNeedsDecodingWithRange):
(WebCore::hostNameNeedsEncodingWithRange):
(WebCore::decodeHostNameWithRange):
(WebCore::encodeHostNameWithRange):
(WebCore::decodeHostName):
(WebCore::encodeHostName):
(WebCore::collectRangesThatNeedMapping):
(WebCore::mapHostNames):
(WebCore::URLWithData):
(WebCore::dataWithUserTypedString):
(WebCore::URLWithUserTypedString):
(WebCore::URLWithUserTypedStringDeprecated):
(WebCore::userVisibleString):

Source/WebKit/ios:

* Misc/WebNSStringExtrasIOS.m:
(-[NSString _web_possibleURLsForForUserTypedString:]):
* WebView/WebPDFViewPlaceholder.mm:
(-[WebPDFViewPlaceholder _updateTitleForURL:]):

Source/WebKit/mac:

In this patch, we add new SPIs _webkit_URLWithUserTypedString, _webkit_decodeHostName and
_webkit_encodeHostName which will return nil while dealing with illegal IDN.

Old SPIs _web_URLWithUserTypedString, _web_decodeHostName and _web_encodeHostName are marked
deprecated as they ignore URL parsing failure.

* History/WebHistoryItem.mm:
(-[WebHistoryItem initFromDictionaryRepresentation:]):
* Misc/WebKitErrors.m:
(+[NSError _webKitErrorWithCode:failingURL:]):
* Misc/WebNSFileManagerExtras.mm:
(-[NSFileManager _webkit_setMetadataURL:referrer:atPath:]):
* Misc/WebNSPasteboardExtras.mm:
(-[NSPasteboard _web_bestURL]):
* Misc/WebNSURLExtras.h:
* Misc/WebNSURLExtras.mm:
(+[NSURL _web_URLWithUserTypedString:]):
(+[NSURL _webkit_URLWithUserTypedString:relativeToURL:]):
(+[NSURL _webkit_URLWithUserTypedString:]):
(-[NSString _web_decodeHostName]):
(-[NSString _web_encodeHostName]):
(-[NSString _webkit_decodeHostName]):
(-[NSString _webkit_encodeHostName]):
* Panels/WebAuthenticationPanel.m:
(-[WebAuthenticationPanel setUpForChallenge:]):
* WebCoreSupport/WebEditorClient.mm:
(WebEditorClient::canonicalizeURLString):

Tools:

* MiniBrowser/mac/WK2BrowserWindowController.m:
(-[WK2BrowserWindowController fetch:]):
* TestWebKitAPI/Tests/Cocoa/URLExtras.mm:
(TestWebKitAPI::TEST):

LayoutTests:

* fast/url/host-expected.txt:
* fast/url/idna2003-expected.txt:
* fast/url/invalid-idn-expected.txt: Added.
* fast/url/invalid-idn.html: Added.

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

24 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/url/host-expected.txt
LayoutTests/fast/url/idna2003-expected.txt
LayoutTests/fast/url/invalid-idn-expected.txt [new file with mode: 0644]
LayoutTests/fast/url/invalid-idn.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/URL.cpp
Source/WebCore/platform/mac/WebCoreNSURLExtras.h
Source/WebCore/platform/mac/WebCoreNSURLExtras.mm
Source/WebKit/ios/ChangeLog
Source/WebKit/ios/Misc/WebNSStringExtrasIOS.m
Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/History/WebHistoryItem.mm
Source/WebKit/mac/Misc/WebKitErrors.m
Source/WebKit/mac/Misc/WebNSFileManagerExtras.mm
Source/WebKit/mac/Misc/WebNSPasteboardExtras.mm
Source/WebKit/mac/Misc/WebNSURLExtras.h
Source/WebKit/mac/Misc/WebNSURLExtras.mm
Source/WebKit/mac/Panels/WebAuthenticationPanel.m
Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
Tools/ChangeLog
Tools/MiniBrowser/mac/WK2BrowserWindowController.m
Tools/TestWebKitAPI/Tests/Cocoa/URLExtras.mm

index 3e23d13..2736308 100644 (file)
@@ -1,3 +1,16 @@
+2016-03-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        URL Parsing should signal failure for illegal IDN
+        https://bugs.webkit.org/show_bug.cgi?id=154945
+        <rdar://problem/8014795>
+
+        Reviewed by Brent Fulgham.
+
+        * fast/url/host-expected.txt:
+        * fast/url/idna2003-expected.txt:
+        * fast/url/invalid-idn-expected.txt: Added.
+        * fast/url/invalid-idn.html: Added.
+
 2016-03-16  Mark Lam  <mark.lam@apple.com>
 
         Add support for setting Function.name from computed properties.
index 87402a8..31606c5 100644 (file)
@@ -8,7 +8,7 @@ FAIL canonicalize('http://Goo%20 goo%7C|.com/') should be http://goo%20%20goo%7C
 FAIL canonicalize('http://GOO  goo.com/') should be http://goo%20%20goo.com/. Was http://GOO  goo.com/.
 PASS canonicalize('http://GOO​⁠goo.com/') is 'http://googoo.com/'
 PASS canonicalize('http://www.foo。bar.com/') is 'http://www.foo.bar.com/'
-FAIL canonicalize('http://﷐zyx.com/') should be http://%EF%BF%BDzyx.com/. Was http:/.
+FAIL canonicalize('http://﷐zyx.com/') should be http://%EF%BF%BDzyx.com/. Was about:blank.
 FAIL canonicalize('http://%ef%b7%90zyx.com/') should be http://%EF%BF%BDzyx.com/. Was http://%ef%b7%90zyx.com/.
 PASS canonicalize('http://Go.com/') is 'http://go.com/'
 FAIL canonicalize('http://%41.com/') should be http://a.com/. Was http://%41.com/.
index 790ba2c..e871f8c 100644 (file)
@@ -17,21 +17,21 @@ PASS canonicalize('http://www.lookout‧net/') is 'http://www.xn--lookoutnet-406
 PASS canonicalize('http://www.looĸout.net/') is 'http://www.xn--looout-5bb.net/'
 FAIL canonicalize('http://www.lookout.net⩴80/') should be http://www.lookout.net::%3D80/. Was http://www.lookout.net⩴80/.
 FAIL canonicalize('http://www .lookout.net/') should be http://www%20.lookout.net/. Was http://www .lookout.net/.
-FAIL canonicalize('http:// lookout.net/') should be http://%E1%9A%80lookout.net/. Was http:/.
+FAIL canonicalize('http:// lookout.net/') should be http://%E1%9A%80lookout.net/. Was about:blank.
 FAIL canonicalize('http://\1flookout.net/') should be http://%1Flookout.net/. Was http://\1flookout.net/.
-FAIL canonicalize('http://look۝out.net/') should be http://look%DB%9Dout.net/. Was http:/.
-FAIL canonicalize('http://look᠎out.net/') should be http://look%E1%A0%8Eout.net/. Was http:/.
+FAIL canonicalize('http://look۝out.net/') should be http://look%DB%9Dout.net/. Was about:blank.
+FAIL canonicalize('http://look᠎out.net/') should be http://look%E1%A0%8Eout.net/. Was about:blank.
 FAIL canonicalize('http://look⁠out.net/') should be http://look%E2%81%A0out.net/. Was http://lookout.net/.
 FAIL canonicalize('http://lookout.net/') should be http://look%EF%BB%BFout.net/. Was http://lookout.net/.
-FAIL canonicalize('http://look🿾out.net/') should be http://look%F0%9F%BF%BEout.net/. Was http:/.
-FAIL canonicalize('http://lookout.net/') should be http://look%EF%BF%BAout.net/. Was http:/.
-FAIL canonicalize('http://look⿰out.net/') should be http://look%E2%BF%B0out.net/. Was http:/.
+FAIL canonicalize('http://look🿾out.net/') should be http://look%F0%9F%BF%BEout.net/. Was about:blank.
+FAIL canonicalize('http://lookout.net/') should be http://look%EF%BF%BAout.net/. Was about:blank.
+FAIL canonicalize('http://look⿰out.net/') should be http://look%E2%BF%B0out.net/. Was about:blank.
 FAIL canonicalize('http://looḱout.net/') should be http://look%CD%81out.net/. Was http://xn--looout-kp7b.net/.
-FAIL canonicalize('http://look‮out.net/') should be http://look%E2%80%AEout.net/. Was http:/.
-FAIL canonicalize('http://lookout.net/') should be http://look%E2%81%ABout.net/. Was http:/.
-FAIL canonicalize('http://look󠀁out.net/') should be http://look%F3%A0%80%81out.net/. Was http:/.
-FAIL canonicalize('http://look󠀠out.net/') should be http://look%F3%A0%80%A0out.net/. Was http:/.
-FAIL canonicalize('http://look־out.net/') should be http://look%D6%BEout.net/. Was http:/.
+FAIL canonicalize('http://look‮out.net/') should be http://look%E2%80%AEout.net/. Was about:blank.
+FAIL canonicalize('http://lookout.net/') should be http://look%E2%81%ABout.net/. Was about:blank.
+FAIL canonicalize('http://look󠀁out.net/') should be http://look%F3%A0%80%81out.net/. Was about:blank.
+FAIL canonicalize('http://look󠀠out.net/') should be http://look%F3%A0%80%A0out.net/. Was about:blank.
+FAIL canonicalize('http://look־out.net/') should be http://look%D6%BEout.net/. Was about:blank.
 PASS successfullyParsed is true
 
 TEST COMPLETE
diff --git a/LayoutTests/fast/url/invalid-idn-expected.txt b/LayoutTests/fast/url/invalid-idn-expected.txt
new file mode 100644 (file)
index 0000000..5ab6592
--- /dev/null
@@ -0,0 +1,4 @@
+Test passes if all the invalid urls are converted to about:blank.
+about:blank
+about:blank
+
diff --git a/LayoutTests/fast/url/invalid-idn.html b/LayoutTests/fast/url/invalid-idn.html
new file mode 100644 (file)
index 0000000..b746c1a
--- /dev/null
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-16">
+<!-- Set the base so that the current URL does not affect the tests. -->
+<base href="">
+</head>
+<body>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+// Start the bidding at 42 for no particular reason.
+var lastID = 42;
+
+function canonicalize(url)
+{
+    var id = ++lastID;
+    document.write("<a id='" + id + "' href='" + url + "'></a>");
+    return document.getElementById(id).href;
+}
+
+// Those are all invalid URLs. They should not be accepted by the parser.
+var testSet = [
+    'http://.com',
+    'http://www.اast.fm'
+];
+
+document.write("Test passes if all the invalid urls are converted to about:blank.<br>");
+for (var i = 0; i < testSet.length; ++i) {
+    src = canonicalize(testSet[i]);
+    document.write(src + "<br>");
+}
+</script>
+</body>
+</html>
index 4b4c9ce..656df16 100644 (file)
@@ -1,3 +1,39 @@
+2016-03-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        URL Parsing should signal failure for illegal IDN
+        https://bugs.webkit.org/show_bug.cgi?id=154945
+        <rdar://problem/8014795>
+
+        Reviewed by Brent Fulgham.
+
+        WebCore::URL will now invalidate URLs with illegal IDN. And functions inside WebCoreNSURLExtras.h
+        that deal with IDN mapping will now return nil to signal error.
+
+        Test: fast/url/invalid-idn.html
+
+        * platform/URL.cpp:
+        (WebCore::isSchemeFirstChar):
+        (WebCore::URL::init):
+        (WebCore::appendEncodedHostname):
+        (WebCore::encodeHostnames):
+        (WebCore::encodeRelativeString):
+        * platform/mac/WebCoreNSURLExtras.h:
+        * platform/mac/WebCoreNSURLExtras.mm:
+        (WebCore::mapHostNameWithRange):
+        (WebCore::hostNameNeedsDecodingWithRange):
+        (WebCore::hostNameNeedsEncodingWithRange):
+        (WebCore::decodeHostNameWithRange):
+        (WebCore::encodeHostNameWithRange):
+        (WebCore::decodeHostName):
+        (WebCore::encodeHostName):
+        (WebCore::collectRangesThatNeedMapping):
+        (WebCore::mapHostNames):
+        (WebCore::URLWithData):
+        (WebCore::dataWithUserTypedString):
+        (WebCore::URLWithUserTypedString):
+        (WebCore::URLWithUserTypedStringDeprecated):
+        (WebCore::userVisibleString):
+
 2016-03-16  Antti Koivisto  <antti@apple.com>
 
         Don't invalidate style unnecessarily when setting inline style cssText
index 6abb53e..e42e9ec 100644 (file)
@@ -337,7 +337,7 @@ static const unsigned char percentEncodeClassTable[256] = {
 };
 
 static int copyPathRemovingDots(char* dst, const char* src, int srcStart, int srcEnd);
-static void encodeRelativeString(const String& rel, const TextEncoding&, CharBuffer& ouput);
+static bool encodeRelativeString(const String& rel, const TextEncoding&, CharBuffer& ouput);
 static String substituteBackslashes(const String&);
 
 static inline bool isSchemeFirstChar(char c) { return characterClassTable[static_cast<unsigned char>(c)] & SchemeFirstChar; }
@@ -490,7 +490,12 @@ void URL::init(const URL& base, const String& relative, const TextEncoding& enco
         strBuffer[len] = 0;
         str = strBuffer.data();
     } else {
-        encodeRelativeString(rel, encoding, strBuffer);
+        if (!encodeRelativeString(rel, encoding, strBuffer)) {
+            m_string = blankURL();
+            invalidate();
+            return;
+        }
+
         str = strBuffer.data();
         len = strlen(str);
     }
@@ -1681,7 +1686,8 @@ static bool protocolIs(StringView stringURL, const char* protocol)
 
 // Appends the punycoded hostname identified by the given string and length to
 // the output buffer. The result will not be null terminated.
-static void appendEncodedHostname(UCharBuffer& buffer, StringView string)
+// Return value of false means error in encoding.
+static bool appendEncodedHostname(UCharBuffer& buffer, StringView string)
 {
     // Needs to be big enough to hold an IDN-encoded name.
     // For host names bigger than this, we won't do IDN encoding, which is almost certainly OK.
@@ -1689,7 +1695,7 @@ static void appendEncodedHostname(UCharBuffer& buffer, StringView string)
 
     if (string.length() > hostnameBufferLength || containsOnlyASCII(string)) {
         append(buffer, string);
-        return;
+        return true;
     }
 
     UChar hostnameBuffer[hostnameBufferLength];
@@ -1705,8 +1711,11 @@ static void appendEncodedHostname(UCharBuffer& buffer, StringView string)
 #pragma GCC diagnostic pop
 #endif
 
-    if (error == U_ZERO_ERROR)
+    if (error == U_ZERO_ERROR) {
         buffer.append(hostnameBuffer, numCharactersConverted);
+        return true;
+    }
+    return false;
 }
 
 static void findHostnamesInMailToURL(StringView string, Vector<std::pair<int, int>>& nameRanges)
@@ -1819,7 +1828,8 @@ static bool findHostnameInHierarchicalURL(StringView string, int& startOffset, i
 
 // Converts all hostnames found in the given input to punycode, preserving the
 // rest of the URL unchanged. The output will NOT be null-terminated.
-static void encodeHostnames(StringView string, UCharBuffer& buffer)
+// Return value of false means error in encoding.
+static bool encodeHostnames(StringView string, UCharBuffer& buffer)
 {
     buffer.clear();
 
@@ -1831,7 +1841,8 @@ static void encodeHostnames(StringView string, UCharBuffer& buffer)
         for (int i = 0; i < n; ++i) {
             const std::pair<int, int>& r = hostnameRanges[i];
             append(buffer, string.substring(p, r.first - p));
-            appendEncodedHostname(buffer, string.substring(r.first, r.second - r.first));
+            if (!appendEncodedHostname(buffer, string.substring(r.first, r.second - r.first)))
+                return false;
             p = r.second;
         }
         // This will copy either everything after the last hostname, or the
@@ -1841,19 +1852,24 @@ static void encodeHostnames(StringView string, UCharBuffer& buffer)
         int hostStart, hostEnd;
         if (findHostnameInHierarchicalURL(string, hostStart, hostEnd)) {
             append(buffer, string.substring(0, hostStart)); // Before hostname.
-            appendEncodedHostname(buffer, string.substring(hostStart, hostEnd - hostStart));
+            if (!appendEncodedHostname(buffer, string.substring(hostStart, hostEnd - hostStart)))
+                return false;
             append(buffer, string.substring(hostEnd)); // After hostname.
         } else {
             // No hostname to encode, return the input.
             append(buffer, string);
         }
     }
+
+    return true;
 }
 
-static void encodeRelativeString(const String& rel, const TextEncoding& encoding, CharBuffer& output)
+// Return value of false means error in encoding.
+static bool encodeRelativeString(const String& rel, const TextEncoding& encoding, CharBuffer& output)
 {
     UCharBuffer s;
-    encodeHostnames(rel, s);
+    if (!encodeHostnames(rel, s))
+        return false;
 
     TextEncoding pathEncoding(UTF8Encoding()); // Path is always encoded as UTF-8; other parts may depend on the scheme.
 
@@ -1878,6 +1894,8 @@ static void encodeRelativeString(const String& rel, const TextEncoding& encoding
         memcpy(output.data() + pathDecoded.length(), otherDecoded.data(), otherDecoded.length());
     }
     output.append('\0'); // null-terminate the output.
+
+    return true;
 }
 
 static String substituteBackslashes(const String& string)
index e486563..fe10681 100644 (file)
@@ -33,18 +33,19 @@ namespace WebCore {
 
 WEBCORE_EXPORT NSString *userVisibleString(NSURL *);
 WEBCORE_EXPORT NSURL *URLByCanonicalizingURL(NSURL *);
-WEBCORE_EXPORT NSURL *URLWithUserTypedString(NSString *, NSURL *baseURL);
+WEBCORE_EXPORT NSURL *URLWithUserTypedString(NSString *, NSURL *baseURL); // Return value of nil means error.
 WEBCORE_EXPORT NSURL *URLByRemovingUserInfo(NSURL *);
-WEBCORE_EXPORT BOOL hostNameNeedsDecodingWithRange(NSString *, NSRange);
-WEBCORE_EXPORT BOOL hostNameNeedsEncodingWithRange(NSString *, NSRange);
-WEBCORE_EXPORT NSString *decodeHostNameWithRange(NSString *, NSRange);
-WEBCORE_EXPORT NSString *encodeHostNameWithRange(NSString *, NSRange);
-WEBCORE_EXPORT NSString *decodeHostName(NSString *);
-WEBCORE_EXPORT NSString *encodeHostName(NSString *);
+WEBCORE_EXPORT BOOL hostNameNeedsDecodingWithRange(NSString *, NSRange, BOOL* error);
+WEBCORE_EXPORT BOOL hostNameNeedsEncodingWithRange(NSString *, NSRange, BOOL* error);
+WEBCORE_EXPORT NSString *decodeHostNameWithRange(NSString *, NSRange); // Return value of nil means error.
+WEBCORE_EXPORT NSString *encodeHostNameWithRange(NSString *, NSRange); // Return value of nil means error.
+WEBCORE_EXPORT NSString *decodeHostName(NSString *); // Return value of nil means error.
+WEBCORE_EXPORT NSString *encodeHostName(NSString *); // Return value of nil means error.
 WEBCORE_EXPORT NSURL *URLByTruncatingOneCharacterBeforeComponent(NSURL *, CFURLComponentType);
 WEBCORE_EXPORT NSURL *URLWithData(NSData *, NSURL *baseURL);
 WEBCORE_EXPORT NSData *originalURLData(NSURL *);
 WEBCORE_EXPORT NSData *dataForURLComponentType(NSURL *, CFURLComponentType);
+WEBCORE_EXPORT NSURL *URLWithUserTypedStringDeprecated(NSString *, NSURL *baseURL);
 
 NSRange rangeOfURLScheme(NSString *);
 WEBCORE_EXPORT BOOL isUserVisibleURL(NSString *);
index f0c0477..91f6dfd 100644 (file)
@@ -453,7 +453,7 @@ static BOOL allCharactersAllowedByTLDRules(const UChar* buffer, int32_t length)
 // Return value of nil means no mapping is necessary.
 // If makeString is NO, then return value is either nil or self to indicate mapping is necessary.
 // If makeString is YES, then return value is either nil or the mapped string.
-static NSString *mapHostNameWithRange(NSString *string, NSRange range, BOOL encode, BOOL makeString)
+static NSString *mapHostNameWithRange(NSString *string, NSRange range, BOOL encode, BOOL makeString, BOOL *error)
 {
     if (range.length > HOST_NAME_BUFFER_LENGTH)
         return nil;
@@ -476,10 +476,12 @@ static NSString *mapHostNameWithRange(NSString *string, NSRange range, BOOL enco
     int length = range.length;
     [string getCharacters:sourceBuffer range:range];
     
-    UErrorCode error = U_ZERO_ERROR;
-    int32_t numCharactersConverted = (encode ? uidna_IDNToASCII : uidna_IDNToUnicode)(sourceBuffer, length, destinationBuffer, HOST_NAME_BUFFER_LENGTH, UIDNA_ALLOW_UNASSIGNED, NULL, &error);
-    if (error != U_ZERO_ERROR)
+    UErrorCode uerror = U_ZERO_ERROR;
+    int32_t numCharactersConverted = (encode ? uidna_IDNToASCII : uidna_IDNToUnicode)(sourceBuffer, length, destinationBuffer, HOST_NAME_BUFFER_LENGTH, UIDNA_ALLOW_UNASSIGNED, NULL, &uerror);
+    if (U_FAILURE(uerror)) {
+        *error = YES;
         return nil;
+    }
     
     if (numCharactersConverted == length && !memcmp(sourceBuffer, destinationBuffer, length * sizeof(UChar)))
         return nil;
@@ -487,52 +489,71 @@ static NSString *mapHostNameWithRange(NSString *string, NSRange range, BOOL enco
     if (!encode && !allCharactersInIDNScriptWhiteList(destinationBuffer, numCharactersConverted) && !allCharactersAllowedByTLDRules(destinationBuffer, numCharactersConverted))
         return nil;
     
-    return makeString ? (NSString *)[NSString stringWithCharacters:destinationBuffer length:numCharactersConverted] : string;
+    return makeString ? [NSString stringWithCharacters:destinationBuffer length:numCharactersConverted] : string;
 }
 
-BOOL hostNameNeedsDecodingWithRange(NSString *string, NSRange range)
+BOOL hostNameNeedsDecodingWithRange(NSString *string, NSRange range, BOOL *error)
 {
-     return mapHostNameWithRange(string, range, NO, NO) != nil;
+    return mapHostNameWithRange(string, range, NO, NO, error) != nil;
 }
  
-BOOL hostNameNeedsEncodingWithRange(NSString *string, NSRange range)
+BOOL hostNameNeedsEncodingWithRange(NSString *string, NSRange range, BOOL *error)
 {
-     return mapHostNameWithRange(string, range, YES,  NO) != nil;
+    return mapHostNameWithRange(string, range, YES,  NO, error) != nil;
 }
 
 NSString *decodeHostNameWithRange(NSString *string, NSRange range)
 {
-    return mapHostNameWithRange(string, range, NO, YES);
+    BOOL error = NO;
+    NSString *host = mapHostNameWithRange(string, range, NO, YES, &error);
+    if (error)
+        return nil;
+    return !host ? string : host;
 }
 
 NSString *encodeHostNameWithRange(NSString *string, NSRange range)
 {
-    return mapHostNameWithRange(string, range, YES, YES);
+    BOOL error = NO;
+    NSString *host = mapHostNameWithRange(string, range, YES, YES, &error);
+    if (error)
+        return nil;
+    return !host ? string : host;
 }
 
 NSString *decodeHostName(NSString *string)
 {
-    NSString *name = mapHostNameWithRange(string, NSMakeRange(0, [string length]), NO, YES);
-    return !name ? string : name;
+    BOOL error = NO;
+    NSString *host = mapHostNameWithRange(string, NSMakeRange(0, [string length]), NO, YES, &error);
+    if (error)
+        return nil;
+    return !host ? string : host;
 }
 
 NSString *encodeHostName(NSString *string)
 {
-    NSString *name =  mapHostNameWithRange(string, NSMakeRange(0, [string length]), YES, YES);
-    return !name ? string : name;
+    BOOL error = NO;
+    NSString *host = mapHostNameWithRange(string, NSMakeRange(0, [string length]), YES, YES, &error);
+    if (error)
+        return nil;
+    return !host ? string : host;
 }
 
 static void collectRangesThatNeedMapping(NSString *string, NSRange range, void *context, BOOL encode)
 {
-    BOOL needsMapping = encode ? hostNameNeedsEncodingWithRange(string, range) : hostNameNeedsDecodingWithRange(string, range);
-    if (!needsMapping)
+    // Generally, we want to optimize for the case where there is one host name that does not need mapping.
+    // Therefore, we use nil to indicate no mapping here and an empty array to indicate error.
+
+    BOOL error = NO;
+    BOOL needsMapping = encode ? hostNameNeedsEncodingWithRange(string, range, &error) : hostNameNeedsDecodingWithRange(string, range, &error);
+    if (!error && !needsMapping)
         return;
     
     NSMutableArray **array = (NSMutableArray **)context;
     if (!*array)
         *array = [[NSMutableArray alloc] init];
     
-    [*array addObject:[NSValue valueWithRange:range]];
+    if (!error)
+        [*array addObject:[NSValue valueWithRange:range]];
 }
 
 static void collectRangesThatNeedEncoding(NSString *string, NSRange range, void *context)
@@ -681,6 +702,9 @@ static NSString *mapHostNames(NSString *string, BOOL encode)
     applyHostNameFunctionToURLString(string, f, &hostNameRanges);
     if (!hostNameRanges)
         return string;
+
+    if (![hostNameRanges count])
+        return nil;
     
     // Do the mapping.
     NSMutableString *mutableCopy = [string mutableCopy];
@@ -787,24 +811,18 @@ NSURL *URLWithData(NSData *data, NSURL *baseURL)
             result = CFBridgingRelease(CFURLCreateAbsoluteURLWithBytes(NULL, bytes, length, kCFStringEncodingISOLatin1, (CFURLRef)baseURL, YES));
     } else
         result = [NSURL URLWithString:@""];
-                
+
     return result;
 }
-
-NSURL *URLWithUserTypedString(NSString *string, NSURL *URL)
+static NSData *dataWithUserTypedString(NSString *string)
 {
-    if (!string)
-        return nil;
-
-    string = mapHostNames(stringByTrimmingWhitespace(string), YES);
-    
     NSData *userTypedData = [string dataUsingEncoding:NSUTF8StringEncoding];
     ASSERT(userTypedData);
     
     const UInt8* inBytes = static_cast<const UInt8 *>([userTypedData bytes]);
     int inLength = [userTypedData length];
     if (!inLength)
-        return [NSURL URLWithString:@""];
+        return nil;
     
     char* outBytes = static_cast<char *>(malloc(inLength * 3)); // large enough to %-escape every character
     char* p = outBytes;
@@ -822,10 +840,41 @@ NSURL *URLWithUserTypedString(NSString *string, NSURL *URL)
         }
     }
     
-    NSData *data = [NSData dataWithBytesNoCopy:outBytes length:outLength]; // adopts outBytes
+    return [NSData dataWithBytesNoCopy:outBytes length:outLength]; // adopts outBytes
+}
+
+NSURL *URLWithUserTypedString(NSString *string, NSURL *URL)
+{
+    if (!string)
+        return nil;
+
+    string = mapHostNames(stringByTrimmingWhitespace(string), YES);
+    if (!string)
+        return nil;
+
+    NSData *data = dataWithUserTypedString(string);
+    if (!data)
+        return [NSURL URLWithString:@""];
+
     return URLWithData(data, URL);
 }
 
+NSURL *URLWithUserTypedStringDeprecated(NSString *string, NSURL *URL)
+{
+    if (!string)
+        return nil;
+
+    NSURL *result = URLWithUserTypedString(string, URL);
+    if (!result) {
+        NSData *resultData = dataWithUserTypedString(string);
+        if (!resultData)
+            return [NSURL URLWithString:@""];
+        result = URLWithData(resultData, URL);
+    }
+
+    return result;
+}
+
 static BOOL hasQuestionMarkOnlyQueryString(NSURL *URL)
 {
     CFRange rangeWithSeparators;
@@ -1071,8 +1120,13 @@ NSString *userVisibleString(NSURL *URL)
     
     free(after);
     
-    if (mayNeedHostNameDecoding)
-        result = mapHostNames(result, NO);
+    if (mayNeedHostNameDecoding) {
+        // FIXME: Is it good to ignore the failure of mapHostNames and keep result intact?
+        NSString *mappedResult = mapHostNames(result, NO);
+        if (mappedResult)
+            result = mappedResult;
+    }
+
     result = [result precomposedStringWithCanonicalMapping];
     return CFBridgingRelease(createStringWithEscapedUnsafeCharacters((CFStringRef)result));
 }
index 7779798..8c60fe5 100644 (file)
@@ -1,3 +1,16 @@
+2016-03-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        URL Parsing should signal failure for illegal IDN
+        https://bugs.webkit.org/show_bug.cgi?id=154945
+        <rdar://problem/8014795>
+
+        Reviewed by Brent Fulgham.
+
+        * Misc/WebNSStringExtrasIOS.m:
+        (-[NSString _web_possibleURLsForForUserTypedString:]):
+        * WebView/WebPDFViewPlaceholder.mm:
+        (-[WebPDFViewPlaceholder _updateTitleForURL:]):
+
 2016-03-15  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, rolling out r198203.
index 6c6b524..e57e2e8 100644 (file)
@@ -96,7 +96,7 @@
             
             if (URLHasScheme) {
                 // - use it unchanged
-                NSURL *URL = [NSURL _web_URLWithUserTypedString:workString];
+                NSURL *URL = [NSURL _webkit_URLWithUserTypedString:workString];
                 if (URL != nil) {
                     [result addObject:URL];
                 }
                 if ([[workString lowercaseString] hasPrefix:@"www."]) {
                     // - just prepend "http://"
                     [workString insertString:@"http://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                 } else if ([[workString lowercaseString] hasPrefix:@"ftp."]) {
                     // - just prepend "ftp://"
                     [workString insertString:@"ftp://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                 } else if (firstAtRange.location != NSNotFound && firstAtRange.location < firstSlashRange.location) {
                     // - justprepend "http://"
                     [workString insertString:@"http://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                 } else if (firstDotRange.location != NSNotFound && firstDotRange.location < firstSlashRange.location) {
                     // - try prepending "http://"
                     [workString insertString:@"http://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                     
                     // - try prepending "http://www."
                     [workString insertString:@"www." atIndex:strlen("http://")];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                            [[workString lowercaseString] hasPrefix:@"localhost/"]) {
                     // - just prepend "http://"
                     [workString insertString:@"http://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                 } else {
                     // - try prepending "http://"
                     [workString insertString:@"http://" atIndex:0];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
                         [workString insertString:@".com" atIndex:endOfHostnameOrPort];
                     }
                     [workString insertString:@"www." atIndex:strlen("http://")];
-                    URL = [NSURL _web_URLWithUserTypedString:workString];
+                    URL = [NSURL _webkit_URLWithUserTypedString:workString];
                     if (URL != nil) {
                         [result addObject:URL];
                     }
index 3da4421..2dcd7b1 100644 (file)
@@ -317,7 +317,7 @@ static const float PAGE_HEIGHT_INSET = 4.0f * 2.0f;
 {
     NSString *titleFromURL = [URL lastPathComponent];
     if (![titleFromURL length] || [titleFromURL isEqualToString:@"/"])
-        titleFromURL = [[URL _web_hostString] _web_decodeHostName];
+        titleFromURL = [[URL _web_hostString] _webkit_decodeHostName];
 
     [self setTitle:titleFromURL];
     [[self _frame] _dispatchDidReceiveTitle:titleFromURL];
index dbc0dfd..915273e 100644 (file)
@@ -1,3 +1,39 @@
+2016-03-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        URL Parsing should signal failure for illegal IDN
+        https://bugs.webkit.org/show_bug.cgi?id=154945
+        <rdar://problem/8014795>
+
+        Reviewed by Brent Fulgham.
+
+        In this patch, we add new SPIs _webkit_URLWithUserTypedString, _webkit_decodeHostName and
+        _webkit_encodeHostName which will return nil while dealing with illegal IDN.
+
+        Old SPIs _web_URLWithUserTypedString, _web_decodeHostName and _web_encodeHostName are marked
+        deprecated as they ignore URL parsing failure.
+
+        * History/WebHistoryItem.mm:
+        (-[WebHistoryItem initFromDictionaryRepresentation:]):
+        * Misc/WebKitErrors.m:
+        (+[NSError _webKitErrorWithCode:failingURL:]):
+        * Misc/WebNSFileManagerExtras.mm:
+        (-[NSFileManager _webkit_setMetadataURL:referrer:atPath:]):
+        * Misc/WebNSPasteboardExtras.mm:
+        (-[NSPasteboard _web_bestURL]):
+        * Misc/WebNSURLExtras.h:
+        * Misc/WebNSURLExtras.mm:
+        (+[NSURL _web_URLWithUserTypedString:]):
+        (+[NSURL _webkit_URLWithUserTypedString:relativeToURL:]):
+        (+[NSURL _webkit_URLWithUserTypedString:]):
+        (-[NSString _web_decodeHostName]):
+        (-[NSString _web_encodeHostName]):
+        (-[NSString _webkit_decodeHostName]):
+        (-[NSString _webkit_encodeHostName]):
+        * Panels/WebAuthenticationPanel.m:
+        (-[WebAuthenticationPanel setUpForChallenge:]):
+        * WebCoreSupport/WebEditorClient.mm:
+        (WebEditorClient::canonicalizeURLString):
+
 2016-03-15  Jer Noble  <jer.noble@apple.com>
 
         [ios-sim debug] API test WebKit1.AudioSessionCategoryIOS timing out
index f9b7fd8..2dbcc16 100644 (file)
@@ -333,7 +333,7 @@ WebHistoryItem *kit(HistoryItem* item)
     // Check if we've read a broken URL from the file that has non-Latin1 chars.  If so, try to convert
     // as if it was from user typing.
     if (![URLString canBeConvertedToEncoding:NSISOLatin1StringEncoding]) {
-        NSURL *tempURL = [NSURL _web_URLWithUserTypedString:URLString];
+        NSURL *tempURL = [NSURL _webkit_URLWithUserTypedString:URLString];
         ASSERT(tempURL);
         NSString *newURLString = [tempURL _web_originalDataAsString];
         core(_private)->setURLString(newURLString);
index a73f51a..9834413 100644 (file)
@@ -98,7 +98,7 @@ static NSMutableDictionary *descriptions = nil;
 
 + (NSError *)_webKitErrorWithCode:(int)code failingURL:(NSString *)URLString
 {
-    return [self _webKitErrorWithDomain:WebKitErrorDomain code:code URL:[NSURL _web_URLWithUserTypedString:URLString]];
+    return [self _webKitErrorWithDomain:WebKitErrorDomain code:code URL:[NSURL _webkit_URLWithUserTypedString:URLString]];
 }
 
 - (id)_initWithPluginErrorCode:(int)code
index 475f42d..a60d898 100644 (file)
@@ -75,7 +75,7 @@ static void *setMetaData(void* context)
     ASSERT(URLString);
     ASSERT(path);
 
-    NSURL *URL = [NSURL _web_URLWithUserTypedString:URLString];
+    NSURL *URL = [NSURL _webkit_URLWithUserTypedString:URLString];
     if (URL)
         URLString = [[URL _web_URLByRemovingUserInfo] _web_userVisibleString];
  
index b4d9660..d083b6b 100644 (file)
@@ -131,7 +131,7 @@ static NSArray *_writableTypesForImageWithArchive (void)
     if ([types containsObject:NSStringPboardType]) {
         NSString *URLString = [self stringForType:NSStringPboardType];
         if ([URLString _webkit_looksLikeAbsoluteURL]) {
-            NSURL *URL = [[NSURL _web_URLWithUserTypedString:URLString] _webkit_canonicalize];
+            NSURL *URL = [[NSURL _webkit_URLWithUserTypedString:URLString] _webkit_canonicalize];
             if (URL) {
                 return URL;
             }
index 6fe5c09..2ba0e27 100644 (file)
 
 @interface NSURL (WebNSURLExtras)
 
+// Deprecated as it ignores URL parsing error.
+// Please use the _webkit_ counterparts.
 + (NSURL *)_web_URLWithUserTypedString:(NSString *)string;
 + (NSURL *)_web_URLWithUserTypedString:(NSString *)string relativeToURL:(NSURL *)URL;
 
+// New SPI.
+// Return value of nil means error in URL parsing.
++ (NSURL *)_webkit_URLWithUserTypedString:(NSString *)string;
++ (NSURL *)_webkit_URLWithUserTypedString:(NSString *)string relativeToURL:(NSURL *)URL;
+
 + (NSURL *)_web_URLWithDataAsString:(NSString *)string;
 + (NSURL *)_web_URLWithDataAsString:(NSString *)string relativeToURL:(NSURL *)baseURL;
 
 
 - (BOOL)_web_isUserVisibleURL;
 
-- (NSString *)_web_decodeHostName; // turns funny-looking ASCII form into Unicode, returns self if no decoding needed, convenient cover
-- (NSString *)_web_encodeHostName; // turns Unicode into funny-looking ASCII form, returns self if no decoding needed, convenient cover
+// Deprecated as it ignores URL parsing error.
+// Please use the _webkit_ counterparts.
+// turns funny-looking ASCII form into Unicode, returns self if no decoding needed, convenient cover
+- (NSString *)_web_decodeHostName;
+// turns Unicode into funny-looking ASCII form, returns self if no decoding needed, convenient cover
+- (NSString *)_web_encodeHostName;
+
+// New SPI.
+// Return value of nil means error in URL parsing.
+- (NSString *)_webkit_decodeHostName;
+- (NSString *)_webkit_encodeHostName;
 
 - (BOOL)_webkit_isJavaScriptURL;
 - (BOOL)_webkit_isFileURL;
index 501473d..1c70601 100644 (file)
@@ -57,11 +57,21 @@ using namespace WTF;
 
 + (NSURL *)_web_URLWithUserTypedString:(NSString *)string relativeToURL:(NSURL *)URL
 {
-    return URLWithUserTypedString(string, URL);
+    return URLWithUserTypedStringDeprecated(string, URL);
 }
 
 + (NSURL *)_web_URLWithUserTypedString:(NSString *)string
 {
+    return URLWithUserTypedStringDeprecated(string, nil);
+}
+
++ (NSURL *)_webkit_URLWithUserTypedString:(NSString *)string relativeToURL:(NSURL *)URL
+{
+    return URLWithUserTypedString(string, URL);
+}
+
++ (NSURL *)_webkit_URLWithUserTypedString:(NSString *)string
+{
     return URLWithUserTypedString(string, nil);
 }
 
@@ -255,11 +265,23 @@ using namespace WTF;
 
 - (NSString *)_web_decodeHostName
 {
-    return decodeHostName(self);
+    NSString *name = decodeHostName(self);
+    return !name ? self : name;
 }
 
 - (NSString *)_web_encodeHostName
 {
+    NSString *name = encodeHostName(self);
+    return !name ? self : name;
+}
+
+- (NSString *)_webkit_decodeHostName
+{
+    return decodeHostName(self);
+}
+
+- (NSString *)_webkit_encodeHostName
+{
     return encodeHostName(self);
 }
 
index 53dda6a..23aa2d9 100644 (file)
 
     NSString *host;
     if ([space port] == 0) {
-        host = [[space host] _web_decodeHostName];
+        host = [[space host] _webkit_decodeHostName];
     } else {
-        host = [NSString stringWithFormat:@"%@:%ld", [[space host] _web_decodeHostName], (long)[space port]];
+        host = [NSString stringWithFormat:@"%@:%ld", [[space host] _webkit_decodeHostName], (long)[space port]];
     }
 
     NSString *realm = [space realm];
index 88fce35..cfb2e96 100644 (file)
@@ -419,7 +419,7 @@ NSURL *WebEditorClient::canonicalizeURLString(NSString *URLString)
 {
     NSURL *URL = nil;
     if ([URLString _webkit_looksLikeAbsoluteURL])
-        URL = [[NSURL _web_URLWithUserTypedString:URLString] _webkit_canonicalize];
+        URL = [[NSURL _webkit_URLWithUserTypedString:URLString] _webkit_canonicalize];
     return URL;
 }
 
index 401b3cb..16fa240 100644 (file)
@@ -1,3 +1,16 @@
+2016-03-16  Jiewen Tan  <jiewen_tan@apple.com>
+
+        URL Parsing should signal failure for illegal IDN
+        https://bugs.webkit.org/show_bug.cgi?id=154945
+        <rdar://problem/8014795>
+
+        Reviewed by Brent Fulgham.
+
+        * MiniBrowser/mac/WK2BrowserWindowController.m:
+        (-[WK2BrowserWindowController fetch:]):
+        * TestWebKitAPI/Tests/Cocoa/URLExtras.mm:
+        (TestWebKitAPI::TEST):
+
 2016-03-15  Tim Horton  <timothy_horton@apple.com>
 
         [iOS Simulator] Test result snapshots are upside down
index db47b09..4f5dfa6 100644 (file)
@@ -110,7 +110,7 @@ static void* keyValueObservingContext = &keyValueObservingContext;
 {
     [urlText setStringValue:[self addProtocolIfNecessary:[urlText stringValue]]];
 
-    [_webView loadRequest:[NSURLRequest requestWithURL:[NSURL _web_URLWithUserTypedString:[urlText stringValue]]]];
+    [_webView loadRequest:[NSURLRequest requestWithURL:[NSURL _webkit_URLWithUserTypedString:[urlText stringValue]]]];
 }
 
 - (IBAction)showHideWebView:(id)sender
index bf8ada5..8ad933e 100644 (file)
@@ -127,4 +127,23 @@ TEST(WebCore, URLExtras_Space)
     EXPECT_STREQ("site.com\xE3\x80\x80othersite.org", [WebCore::decodeHostName(@"site.com\xE3\x80\x80othersite.org") UTF8String]);
 }
 
+TEST(WebCore, URLExtras_ParsingError)
+{
+    // Expect IDN failure.
+    NSURL *url = WebCore::URLWithUserTypedString(@"http://.com", nil);
+    EXPECT_TRUE(url == nil);
+
+    NSString *encodedHostName = WebCore::encodeHostName(@"http://.com");
+    EXPECT_TRUE(encodedHostName == nil);
+}
+
+TEST(WebCore, URLExtras_Nil)
+{
+    NSURL *url1 = WebCore::URLWithUserTypedString(nil, nil);
+    EXPECT_TRUE(url1 == nil);
+
+    NSURL *url2 = WebCore::URLWithUserTypedStringDeprecated(nil, nil);
+    EXPECT_TRUE(url2 == nil);
+}
+
 } // namespace TestWebKitAPI