Fix Windows file URL quirks in URLParser
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 18:27:37 +0000 (18:27 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 18:27:37 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162303

Reviewed by Tim Horton.

Source/WebCore:

Windows file urls allow c:\ and c|\ to have the same meaning, but when serialized they should both be c:/.
This is now standardized to allow cross-platform uniform behavior of URLs.

Covered by new API tests and progress on web platform tests when URLParser is enabled.

* platform/URLParser.cpp:
(WebCore::incrementIteratorSkippingTabAndNewLine):
(WebCore::isWindowsDriveLetter):
(WebCore::checkWindowsDriveLetter):
(WebCore::shouldCopyFileURL):
(WebCore::URLParser::parseSerializedURL):
(WebCore::URLParser::parse):

Tools:

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

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

Source/WebCore/ChangeLog
Source/WebCore/platform/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index f54c1d8..45ce678 100644 (file)
@@ -1,3 +1,23 @@
+2016-09-20  Alex Christensen  <achristensen@webkit.org>
+
+        Fix Windows file URL quirks in URLParser
+        https://bugs.webkit.org/show_bug.cgi?id=162303
+
+        Reviewed by Tim Horton.
+
+        Windows file urls allow c:\ and c|\ to have the same meaning, but when serialized they should both be c:/.
+        This is now standardized to allow cross-platform uniform behavior of URLs.
+
+        Covered by new API tests and progress on web platform tests when URLParser is enabled.
+
+        * platform/URLParser.cpp:
+        (WebCore::incrementIteratorSkippingTabAndNewLine):
+        (WebCore::isWindowsDriveLetter):
+        (WebCore::checkWindowsDriveLetter):
+        (WebCore::shouldCopyFileURL):
+        (WebCore::URLParser::parseSerializedURL):
+        (WebCore::URLParser::parse):
+
 2016-09-20  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Rename FrameData to ImageFrame, move it to a separate file and use it for all ports
index de0ef43..b989118 100644 (file)
@@ -393,13 +393,21 @@ template<typename CharacterType> inline static bool isInvalidDomainCharacter(Cha
 template<typename CharacterType> inline static bool isPercentOrNonASCII(CharacterType character) { return !isASCII(character) || character == '%'; }
 template<typename CharacterType> inline static bool isSlashQuestionOrHash(CharacterType character) { return character <= '\\' && characterClassTable[character] & SlashQuestionOrHash; }
 static bool shouldPercentEncodeQueryByte(uint8_t byte) { return characterClassTable[byte] & QueryPercent; }
-    
-template<typename CharacterType>
+
+template<bool serialized, typename CharacterType>
+void incrementIteratorSkippingTabAndNewLine(CodePointIterator<CharacterType>& iterator)
+{
+    ++iterator;
+    while (!serialized && !iterator.atEnd() && isTabOrNewline(*iterator))
+        ++iterator;
+}
+
+template<bool serialized, typename CharacterType>
 inline static bool isWindowsDriveLetter(CodePointIterator<CharacterType> iterator)
 {
     if (iterator.atEnd() || !isASCIIAlpha(*iterator))
         return false;
-    ++iterator;
+    incrementIteratorSkippingTabAndNewLine<serialized>(iterator);
     if (iterator.atEnd())
         return false;
     return *iterator == ':' || *iterator == '|';
@@ -412,17 +420,31 @@ inline static bool isWindowsDriveLetter(const Vector<LChar>& buffer, size_t inde
     return isASCIIAlpha(buffer[index]) && (buffer[index + 1] == ':' || buffer[index + 1] == '|');
 }
 
-template<typename CharacterType>
+template<bool serialized, typename CharacterType>
+inline static void checkWindowsDriveLetter(CodePointIterator<CharacterType>& iterator, Vector<LChar>& asciiBuffer)
+{
+    if (isWindowsDriveLetter<serialized>(iterator)) {
+        asciiBuffer.reserveCapacity(asciiBuffer.size() + 2);
+        asciiBuffer.uncheckedAppend(*iterator);
+        incrementIteratorSkippingTabAndNewLine<serialized>(iterator);
+        ASSERT(!iterator.atEnd());
+        ASSERT(*iterator == ':' || *iterator == '|');
+        asciiBuffer.uncheckedAppend(':');
+        incrementIteratorSkippingTabAndNewLine<serialized>(iterator);
+    }
+}
+
+template<bool serialized, typename CharacterType>
 inline static bool shouldCopyFileURL(CodePointIterator<CharacterType> iterator)
 {
-    if (isWindowsDriveLetter(iterator))
+    if (!isWindowsDriveLetter<serialized>(iterator))
         return true;
     if (iterator.atEnd())
         return false;
-    ++iterator;
+    incrementIteratorSkippingTabAndNewLine<serialized>(iterator);
     if (iterator.atEnd())
         return true;
-    ++iterator;
+    incrementIteratorSkippingTabAndNewLine<serialized>(iterator);
     if (iterator.atEnd())
         return true;
     return !isSlashQuestionOrHash(*iterator);
@@ -890,14 +912,6 @@ URL URLParser::parseSerializedURL(const String& input)
         return parse<serialized>(input.characters8(), input.length(), { }, UTF8Encoding());
     return parse<serialized>(input.characters16(), input.length(), { }, UTF8Encoding());
 }
-
-template<bool serialized, typename CharacterType>
-void incrementIteratorSkippingTabAndNewLine(CodePointIterator<CharacterType>& iterator)
-{
-    ++iterator;
-    while (!serialized && !iterator.atEnd() && isTabOrNewline(*iterator))
-        ++iterator;
-}
     
 template<bool serialized, typename CharacterType>
 URL URLParser::parse(const CharacterType* input, const unsigned length, const URL& base, const TextEncoding& encoding)
@@ -1204,7 +1218,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 ++c;
                 break;
             default:
-                if (!base.isNull() && base.protocolIs("file") && shouldCopyFileURL(c))
+                if (!base.isNull() && base.protocolIs("file") && shouldCopyFileURL<serialized>(c))
                     copyURLPartsUntil(base, URLPart::PathAfterLastSlash);
                 else {
                     m_asciiBuffer.append("///", 3);
@@ -1214,6 +1228,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                     m_url.m_hostEnd = m_url.m_userStart;
                     m_url.m_portEnd = m_url.m_userStart;
                     m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
+                    checkWindowsDriveLetter<serialized>(c, m_asciiBuffer);
                 }
                 state = State::Path;
                 break;
@@ -1238,15 +1253,13 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 String basePath = base.path();
                 if (basePath.length() >= 2) {
                     bool windowsQuirk = basePath.is8Bit()
-                        ? isWindowsDriveLetter(CodePointIterator<LChar>(basePath.characters8(), basePath.characters8() + basePath.length()))
-                        : isWindowsDriveLetter(CodePointIterator<UChar>(basePath.characters16(), basePath.characters16() + basePath.length()));
+                        ? isWindowsDriveLetter<serialized>(CodePointIterator<LChar>(basePath.characters8(), basePath.characters8() + basePath.length()))
+                        : isWindowsDriveLetter<serialized>(CodePointIterator<UChar>(basePath.characters16(), basePath.characters16() + basePath.length()));
                     if (windowsQuirk) {
                         m_asciiBuffer.append(basePath[0]);
                         m_asciiBuffer.append(basePath[1]);
                     }
                 }
-                state = State::Path;
-                break;
             }
             m_asciiBuffer.append("//", 2);
             m_url.m_userStart = m_asciiBuffer.size() - 1;
@@ -1255,6 +1268,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
             m_url.m_hostEnd = m_url.m_userStart;
             m_url.m_portEnd = m_url.m_userStart;
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
+            checkWindowsDriveLetter<serialized>(c, m_asciiBuffer);
             state = State::Path;
             break;
         case State::FileHost:
index 556c5b7..07ea67b 100644 (file)
@@ -1,3 +1,13 @@
+2016-09-20  Alex Christensen  <achristensen@webkit.org>
+
+        Fix Windows file URL quirks in URLParser
+        https://bugs.webkit.org/show_bug.cgi?id=162303
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-20  Filip Pizlo  <fpizlo@apple.com>
 
         Make MarkedBlock state tracking support overlapped allocation and marking state
index 9db2cb4..273cc83 100644 (file)
@@ -526,6 +526,21 @@ TEST_F(URLParserTest, ParserDifferences)
     checkURLDifferences("http://123.234.12",
         {"http", "", "", "123.234.0.12", 0, "/", "", "", "http://123.234.0.12/"},
         {"http", "", "", "123.234.12", 0, "/", "", "", "http://123.234.12/"});
+    checkRelativeURLDifferences("file:c:\\foo\\bar.html", "file:///tmp/mock/path",
+        {"file", "", "", "", 0, "/c:/foo/bar.html", "", "", "file:///c:/foo/bar.html"},
+        {"file", "", "", "", 0, "/tmp/mock/c:/foo/bar.html", "", "", "file:///tmp/mock/c:/foo/bar.html"});
+    checkRelativeURLDifferences("  File:c|////foo\\bar.html", "file:///tmp/mock/path",
+        {"file", "", "", "", 0, "/c:////foo/bar.html", "", "", "file:///c:////foo/bar.html"},
+        {"file", "", "", "", 0, "/tmp/mock/c|////foo/bar.html", "", "", "file:///tmp/mock/c|////foo/bar.html"});
+    checkRelativeURLDifferences("  Fil\t\n\te\n\t\n:\t\n\tc\t\n\t|\n\t\n/\t\n\t/\n\t\n//foo\\bar.html", "file:///tmp/mock/path",
+        {"file", "", "", "", 0, "/c:////foo/bar.html", "", "", "file:///c:////foo/bar.html"},
+        {"file", "", "", "", 0, "/tmp/mock/c|////foo/bar.html", "", "", "file:///tmp/mock/c|////foo/bar.html"});
+    checkRelativeURLDifferences("C|/foo/bar", "file:///tmp/mock/path",
+        {"file", "", "", "", 0, "/C:/foo/bar", "", "", "file:///C:/foo/bar"},
+        {"file", "", "", "", 0, "/tmp/mock/C|/foo/bar", "", "", "file:///tmp/mock/C|/foo/bar"});
+    checkRelativeURLDifferences("/C|/foo/bar", "file:///tmp/mock/path",
+        {"file", "", "", "", 0, "/C:/foo/bar", "", "", "file:///C:/foo/bar"},
+        {"file", "", "", "", 0, "/C|/foo/bar", "", "", "file:///C|/foo/bar"});
 }
 
 TEST_F(URLParserTest, DefaultPort)