Changeset 93750 in webkit

Timestamp:

Aug 24, 2011 5:59:51 PM (13 years ago)

Author:

ericu@chromium.org

Message:

Reviewed by Adam Barth.

[filesystem/Chromium] Filesystem paths need proper URL escaping
​https://bugs.webkit.org/show_bug.cgi?id=62811

Fix ​http://code.google.com/p/chromium/issues/detail?id=78860 by making
KURLChromium.cpp's escaping code actually work.

Source/WebCore:

Make encodeWithURLEscapeSequences call into googleurl to do proper
escaping. Tested in WebKit/chromium/tests/KURLTest.cpp.

• platform/KURLGoogle.cpp:

(WebCore::encodeWithURLEscapeSequences):

Source/WebKit/chromium:

Here I added the needed calls to encodeWithURLEscapeSequences.

• src/AsyncFileSystemChromium.cpp:

(WebCore::AsyncFileSystemChromium::virtualPathToFileSystemURL):

• src/WorkerAsyncFileSystemChromium.cpp:

(WebCore::WorkerAsyncFileSystemChromium::virtualPathToFileSystemURL):

Here I updated the test to reflect the new functionality in
encodeWithURLEscapeSequences.

• tests/KURLTest.cpp:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/SecurityOrigin.cpp (modified) (2 diffs)
• WebCore/platform/KURLGoogle.cpp (modified) (2 diffs)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/AsyncFileSystemChromium.cpp (modified) (1 diff)
• WebKit/chromium/src/WorkerAsyncFileSystemChromium.cpp (modified) (1 diff)
• WebKit/chromium/tests/KURLTest.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-08-24  Eric Uhrhane  <ericu@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        [filesystem/Chromium] Filesystem paths need proper URL escaping
+        https://bugs.webkit.org/show_bug.cgi?id=62811
+
+        Fix http://code.google.com/p/chromium/issues/detail?id=78860 by making
+        KURLChromium.cpp's escaping code actually work.
+
+        Make encodeWithURLEscapeSequences call into googleurl to do proper
+        escaping.  Tested in WebKit/chromium/tests/KURLTest.cpp.
+        * platform/KURLGoogle.cpp:
+        (WebCore::encodeWithURLEscapeSequences):
+
 2011-08-24  Chris Palmer  <palmer@google.com>
 

trunk/Source/WebCore/page/SecurityOrigin.cpp

@@ -66 +66 @@
 }
 
-
 SecurityOrigin::SecurityOrigin(const KURL& url, SandboxFlags sandboxFlags)
     : m_sandboxFlags(sandboxFlags)
@@ -92 +91 @@
 #endif
     if (isBlobOrFileSystemProtocol) {
-        KURL originURL(ParsedURLString, url.path());
+        KURL originURL(ParsedURLString, decodeURLEscapeSequences(url.path()));
         if (originURL.isValid()) {
             m_protocol = originURL.protocol().lower();

trunk/Source/WebCore/platform/KURLGoogle.cpp

@@ -835 +835 @@
 }
 
-// This is called to escape a URL string. It is only used externally when
-// constructing mailto: links to set the query section. Since our query setter
-// will automatically do the correct escaping, this function does not have to
-// do any work.
-//
-// There is a possibility that a future caller may use this function in other
-// ways, and may expect to get a valid URL string. The dangerous thing we want
-// to protect against here is accidentally getting '\0' characters in a string
-// that is not supposed to have them. Therefore, we escape these characters.
 String encodeWithURLEscapeSequences(const String& notEncodedString)
 {
@@ -852 +843 @@
     const char* input = utf8.data();
     int inputLength = utf8.length();
-
-    Vector<char, 2048> buffer;
-    for (int i = 0; i < inputLength; i++) {
-        if (!input[i])
-            buffer.append("%00", 3);
-        else
-            buffer.append(input[i]);
-    }
-    return String(buffer.data(), buffer.size());
+    url_canon::RawCanonOutputT<char> buffer;
+    if (buffer.length() < inputLength * 3)
+        buffer.Resize(inputLength * 3);
+
+    url_util::EncodeURIComponent(input, inputLength, &buffer);
+    return String(buffer.data(), buffer.length());
 }
 

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-08-24  Eric Uhrhane  <ericu@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        [filesystem/Chromium] Filesystem paths need proper URL escaping
+        https://bugs.webkit.org/show_bug.cgi?id=62811
+
+        Fix http://code.google.com/p/chromium/issues/detail?id=78860 by making
+        KURLChromium.cpp's escaping code actually work.
+
+        Here I added the needed calls to encodeWithURLEscapeSequences.
+        * src/AsyncFileSystemChromium.cpp:
+        (WebCore::AsyncFileSystemChromium::virtualPathToFileSystemURL):
+        * src/WorkerAsyncFileSystemChromium.cpp:
+        (WebCore::WorkerAsyncFileSystemChromium::virtualPathToFileSystemURL):
+
+        Here I updated the test to reflect the new functionality in
+        encodeWithURLEscapeSequences.
+        * tests/KURLTest.cpp:
+
 2011-08-24  Ilya Sherman  <isherman@chromium.org>
 

trunk/Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp

@@ -178 +178 @@
     KURL url = m_filesystemRootURL;
     // Remove the extra leading slash.
-    url.setPath(url.path() + virtualPath.substring(1));
+    url.setPath(url.path() + encodeWithURLEscapeSequences(virtualPath.substring(1)));
     return url;
 }

trunk/Source/WebKit/chromium/src/WorkerAsyncFileSystemChromium.cpp

@@ -228 +228 @@
     KURL url = m_filesystemRootURL;
     // Remove the extra leading slash.
-    url.setPath(url.path() + virtualPath.substring(1));
+    url.setPath(url.path() + encodeWithURLEscapeSequences(virtualPath.substring(1)));
     return url;
 }

trunk/Source/WebKit/chromium/tests/KURLTest.cpp

@@ -307 +307 @@
 TEST(KURLTest, Encode)
 {
+    struct EncodeCase {
+        const char* input;
+        const char* output;
+    } encode_cases[] = {
+        {"hello, world", "hello%2C%20world"},
+        {"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F",
+          "%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F"},
+        {"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F",
+          "%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F"},
+        {" !\"#$%&'()*+,-./",
+          "%20!%22%23%24%25%26'()*%2B%2C-.%2F"},
+        {"0123456789:;<=>?",
+          "0123456789%3A%3B%3C%3D%3E%3F"},
+        {"@ABCDEFGHIJKLMNO",
+          "%40ABCDEFGHIJKLMNO"},
+        {"PQRSTUVWXYZ[\\]^_",
+          "PQRSTUVWXYZ%5B%5C%5D%5E_"},
+        {"`abcdefghijklmno",
+          "%60abcdefghijklmno"},
+        {"pqrstuvwxyz{|}~\x7f",
+          "pqrstuvwxyz%7B%7C%7D~%7F"},
+    };
+
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(encode_cases); i++) {
+        WTF::String input(encode_cases[i].input);
+        WTF::String expectedOutput(encode_cases[i].output);
+        WTF::String output = WebCore::encodeWithURLEscapeSequences(input);
+        EXPECT_EQ(expectedOutput, output);
+    }
+
+    // Our encode escapes NULLs for safety, so we need to check that too.
+    WTF::String input("\x00\x01", 2);
+    WTF::String reference("%00%01");
+
+    WTF::String output = WebCore::encodeWithURLEscapeSequences(input);
+    EXPECT_EQ(reference, output);
+
     // Also test that it gets converted to UTF-8 properly.
     char16 wideInputHelper[3] = { 0x4f60, 0x597d, 0 };
     WTF::String wideInput(
         reinterpret_cast<const ::UChar*>(wideInputHelper), 2);
-    WTF::String wideReference("\xe4\xbd\xa0\xe5\xa5\xbd", 6);
+    WTF::String wideReference("%E4%BD%A0%E5%A5%BD");
     WTF::String wideOutput =
         WebCore::encodeWithURLEscapeSequences(wideInput);
     EXPECT_EQ(wideReference, wideOutput);
-
-    // Our encode only escapes NULLs for safety (see the implementation for
-    // more), so we only bother to test a few cases.
-    WTF::String input(
-        "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f", 16);
-    WTF::String reference(
-        "%00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f", 18);
-    WTF::String output = WebCore::encodeWithURLEscapeSequences(input);
-    EXPECT_EQ(reference, output);
 }