Attachment 123995 Details for Bug 76996 – Patch

[patch] Patch

bug-76996-20120125131217.patch (text/plain), 13.82 KB, created by Daniel Cheng on 2012-01-25 13:12:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Cheng

Created: 2012-01-25 13:12:18 PST

Size: 13.82 KB

patch

obsolete

>Subversion Revision: 105862
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 76de943ca5703fefba170f7e9f2dc2e38975302c..c6143b19a9f3060dbc20b45cf3260aa525d89407 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2012-01-25  Daniel Cheng  <dcheng@chromium.org>
>+
>+        [chromium] Fix ClipboardChromium::validateFilename to actually operate on extensions
>+        https://bugs.webkit.org/show_bug.cgi?id=76996
>+
>+        As it turns out, we were always calling validateFilename on a data object with an empty
>+        extension. Now we call it on an actual extension so that it's sanitized.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Unit test: webkit_unit_tests --gtest_filter=ClipboardChromium.*
>+
>+        * platform/chromium/ClipboardChromium.cpp:
>+        (WebCore::writeImageToDataObject):
>+        * platform/chromium/ClipboardChromium.h:
>+        (ClipboardChromium):
>+        * platform/chromium/ClipboardChromiumLinux.cpp:
>+        (WebCore::ClipboardChromium::validateFilename):
>+        * platform/chromium/ClipboardChromiumMac.cpp:
>+        (WebCore):
>+        (WebCore::ClipboardChromium::validateFilename):
>+        * platform/chromium/ClipboardChromiumWin.cpp:
>+        (WebCore::ClipboardChromium::validateFilename):
>+
> 2012-01-25  Anton Muhin  <antonm@chromium.org>
> 
>         Unreview manual revert of r105843.
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index 6dc00cd9b3b38089dc51593ef656749e4bca8144..495559b49a7fba83993203064d6f05beec8f3482 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -1,3 +1,15 @@
>+2012-01-25  Daniel Cheng  <dcheng@chromium.org>
>+
>+        [chromium] Fix ClipboardChromium::validateFilename to actually operate on extensions
>+        https://bugs.webkit.org/show_bug.cgi?id=76996
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebKit.gypi:
>+        * tests/ClipboardChromiumTest.cpp: Added.
>+        (WebCore):
>+        (WebCore::TEST):
>+
> 2012-01-24  Vsevolod Vlasov  <vsevik@chromium.org>
> 
>         Unreviewed chromium test fix.
>diff --git a/Source/WebCore/platform/chromium/ClipboardChromium.cpp b/Source/WebCore/platform/chromium/ClipboardChromium.cpp
>index 9a26876057a80dfa48129e8d05140b310da3815d..f21d9c41488993a0b5dbcef3b0ebabcfdb53a6d7 100644
>--- a/Source/WebCore/platform/chromium/ClipboardChromium.cpp
>+++ b/Source/WebCore/platform/chromium/ClipboardChromium.cpp
>@@ -255,13 +255,15 @@ static void writeImageToDataObject(ChromiumDataObject* dataObject, Element* elem
>         if (extensionIndex != -1)
>             filename.truncate(extensionIndex);
>     }
>-    filename = ClipboardChromium::validateFileName(filename, dataObject);
> 
>     String extension = MIMETypeRegistry::getPreferredExtensionForMIMEType(
>         cachedImage->response().mimeType());
>-    dataObject->setFileExtension(extension.isEmpty() ? emptyString() : "." + extension);
>+    extension = extension.isEmpty() ? emptyString() : "." + extension;
> 
>-    dataObject->setFileContentFilename(filename + dataObject->fileExtension());
>+    ClipboardChromium::validateFilename(filename, extension);
>+
>+    dataObject->setFileContentFilename(filename + extension);
>+    dataObject->setFileExtension(extension);
> }
> 
> void ClipboardChromium::declareAndWriteDragImage(Element* element, const KURL& url, const String& title, Frame* frame)
>diff --git a/Source/WebCore/platform/chromium/ClipboardChromium.h b/Source/WebCore/platform/chromium/ClipboardChromium.h
>index 30e8739a2387733db17f94586cf77023372e345d..924226df32565e695723010635bf2e6f4fc009a8 100644
>--- a/Source/WebCore/platform/chromium/ClipboardChromium.h
>+++ b/Source/WebCore/platform/chromium/ClipboardChromium.h
>@@ -51,11 +51,10 @@ namespace WebCore {
>         static PassRefPtr<ClipboardChromium> create(
>             ClipboardType, PassRefPtr<ChromiumDataObject>, ClipboardAccessPolicy, Frame*);
> 
>-        // Returns the file name (not including the extension). This removes any
>-        // invalid file system characters as well as making sure the
>-        // path + extension is not bigger than allowed by the file system.
>-        // This may change the file extension in dataObject.
>-        static String validateFileName(const String& title, ChromiumDataObject* dataObject);
>+        // Validates a filename (without the extension) and the extension. This removes any invalid
>+        // file system characters as well as making sure the path + extension is not bigger than
>+        // allowed by the file system.
>+        static void validateFilename(String& name, String& extension);
> 
>         virtual void clearData(const String& type);
>         void clearAllData();
>diff --git a/Source/WebCore/platform/chromium/ClipboardChromiumLinux.cpp b/Source/WebCore/platform/chromium/ClipboardChromiumLinux.cpp
>index 2c89f6e8ee999202912b3d8db9bf5bb772577a97..959b36652fe3956017f7d645010c523853c3b38d 100644
>--- a/Source/WebCore/platform/chromium/ClipboardChromiumLinux.cpp
>+++ b/Source/WebCore/platform/chromium/ClipboardChromiumLinux.cpp
>@@ -27,15 +27,13 @@
> #include "config.h"
> #include "ClipboardChromium.h"
> 
>-#include "ChromiumDataObject.h"
> #include "NotImplemented.h"
> 
> namespace WebCore {
> 
>-String ClipboardChromium::validateFileName(const String& title, ChromiumDataObject* dataObject)
>+void ClipboardChromium::validateFilename(String& name, String& extension)
> {
>     notImplemented();
>-    return title;
> }
> 
> }  // namespace WebCore
>diff --git a/Source/WebCore/platform/chromium/ClipboardChromiumMac.cpp b/Source/WebCore/platform/chromium/ClipboardChromiumMac.cpp
>index 38021402eb929e3cc6e251d223f007466adf2f27..e7cf8b0ca9d1cd2bf73dc8fd272361f3f7633301 100644
>--- a/Source/WebCore/platform/chromium/ClipboardChromiumMac.cpp
>+++ b/Source/WebCore/platform/chromium/ClipboardChromiumMac.cpp
>@@ -27,11 +27,8 @@
> #include "config.h"
> #include "ClipboardChromium.h"
> 
>-#include "ChromiumDataObject.h"
>-
> namespace WebCore {
>     
>-
> // Filename length and character-set limits vary by filesystem, and there's no
> // real way to tell what filesystem is in use. Since HFS+ is by far the most
> // common, we'll abide by its limits, which are 255 Unicode characters (slightly
>@@ -39,7 +36,6 @@ namespace WebCore {
> // really worth dealing with here.)
> static const unsigned MaxHFSFilenameLength = 255;
> 
>-
> static bool isInvalidFileCharacter(UChar c)
> {
>     // HFS+ basically allows anything but '/'. For sanity's sake we'll disallow
>@@ -47,23 +43,18 @@ static bool isInvalidFileCharacter(UChar c)
>     return c < ' ' || c == 0x7F || c == '/';
> }
> 
>-String ClipboardChromium::validateFileName(const String& title, ChromiumDataObject* dataObject)
>+void ClipboardChromium::validateFilename(String& name, String& extension)
> {
>     // Remove any invalid file system characters, especially "/".
>-    String result = title.removeCharacters(isInvalidFileCharacter);
>-    String extension = dataObject->fileExtension().removeCharacters(&isInvalidFileCharacter);
>+    name = name.removeCharacters(&isInvalidFileCharacter);
>+    extension = extension.removeCharacters(&isInvalidFileCharacter);
> 
>     // Remove a ridiculously-long extension.
>     if (extension.length() >= MaxHFSFilenameLength)
>-        extension = "";
>+        extension = String();
>     
>     // Truncate an overly-long filename.
>-    int overflow = result.length() + extension.length() - MaxHFSFilenameLength;
>-    if (overflow > 0) 
>-        result.remove(result.length() - overflow, overflow);
>-    
>-    dataObject->setFileExtension(extension);
>-    return result;
>+    name.truncate(MaxHFSFilenameLength - extension.length() - 1);
> }
> 
> }  // namespace WebCore
>diff --git a/Source/WebCore/platform/chromium/ClipboardChromiumWin.cpp b/Source/WebCore/platform/chromium/ClipboardChromiumWin.cpp
>index d9bbeb5dd3d37a2548914aabcef03ed9afc0bcd7..592fae0a5100fee39278b947db8253decd1c7a32 100644
>--- a/Source/WebCore/platform/chromium/ClipboardChromiumWin.cpp
>+++ b/Source/WebCore/platform/chromium/ClipboardChromiumWin.cpp
>@@ -27,8 +27,6 @@
> #include "config.h"
> #include "ClipboardChromium.h"
> 
>-#include "ChromiumDataObject.h"
>-
> #include <shlwapi.h>
> 
> namespace WebCore {
>@@ -40,17 +38,17 @@ static bool isInvalidFileCharacter(UChar c)
>     return (PathGetCharType(c) & (GCT_LFNCHAR | GCT_SHORTCHAR)) == 0;
> }
> 
>-String ClipboardChromium::validateFileName(const String& title, ChromiumDataObject* dataObject)
>+void ClipboardChromium::validateFilename(String& name, String& extension)
> {
>     // Remove any invalid file system characters.
>-    String result = title.removeCharacters(&isInvalidFileCharacter);
>-    if (result.length() + dataObject->fileExtension().length() + 1 >= MAX_PATH) {
>-        if (dataObject->fileExtension().length() + 1 >= MAX_PATH)
>-            dataObject->setFileExtension("");
>-        if (result.length() + dataObject->fileExtension().length() + 1 >= MAX_PATH)
>-            result = result.substring(0, MAX_PATH - dataObject->fileExtension().length() - 1);
>-    }
>-    return result;
>+    name = name.removeCharacters(&isInvalidFileCharacter);
>+    extension = extension.removeCharacters(&isInvalidFileCharacter);
>+
>+    if (extension.length() + 1 >= MAX_PATH)
>+        extension = String();
>+
>+    // Subtract 2 additional characters for dot and terminating null.
>+    name.truncate(MAX_PATH - extension.length() - 2);
> }
> 
> }  // namespace WebCore
>diff --git a/Source/WebKit/chromium/WebKit.gypi b/Source/WebKit/chromium/WebKit.gypi
>index 702599051101a4e9f3c0859e790189e89703d5fe..be8795d91d0b4c8900286bd2e32eb8f5ae0f0ea6 100644
>--- a/Source/WebKit/chromium/WebKit.gypi
>+++ b/Source/WebKit/chromium/WebKit.gypi
>@@ -145,6 +145,11 @@
>                     'tests/ScrollAnimatorNoneTest.cpp',
>                 ],
>             }],
>+            ['OS!="linux"', {
>+                'webkit_unittest_files': [
>+                    'tests/ClipboardChromiumTest.cpp',
>+                ],
>+            }],
>             ['toolkit_uses_gtk == 1', {
>                 'webkit_unittest_files': [
>                     'tests/WebInputEventFactoryTestGtk.cpp',
>diff --git a/Source/WebKit/chromium/tests/ClipboardChromiumTest.cpp b/Source/WebKit/chromium/tests/ClipboardChromiumTest.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..2956acab98eac27807ce488c77d87a19a967af49
>--- /dev/null
>+++ b/Source/WebKit/chromium/tests/ClipboardChromiumTest.cpp
>@@ -0,0 +1,98 @@
>+/*
>+ * Copyright (C) 2010 Google Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions are
>+ * met:
>+ *
>+ *     * Redistributions of source code must retain the above copyright
>+ * notice, this list of conditions and the following disclaimer.
>+ *     * Redistributions in binary form must reproduce the above
>+ * copyright notice, this list of conditions and the following disclaimer
>+ * in the documentation and/or other materials provided with the
>+ * distribution.
>+ *     * Neither the name of Google Inc. nor the names of its
>+ * contributors may be used to endorse or promote products derived from
>+ * this software without specific prior written permission.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+
>+#include "ClipboardChromium.h"
>+
>+#include <gtest/gtest.h>
>+
>+using namespace WebCore;
>+
>+namespace {
>+
>+#if OS(DARWIN)
>+const char invalidCharacters[] =
>+    "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
>+    "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
>+    "\x7f/";
>+#elif OS(WINDOWS)
>+const char invalidCharacters[] = "\x00/\\:*?\"<>|";
>+#endif
>+const char longString[] =
>+    "0,1,1,2,3,5,8,13,21,34,55,89,144,233,377,610,987,1597,2584,4181,6765,10946,17711,28657,46368,"
>+    "75025,121393,196418,317811,514229,832040,1346269,2178309,3524578,5702887,9227465,14930352";
>+
>+TEST(ClipboardChromiumTest, Normal)
>+{
>+    String name = "name";
>+    String extension = "ext";
>+    ClipboardChromium::validateFilename(name, extension);
>+    EXPECT_EQ("name", name);
>+    EXPECT_EQ("ext", extension);
>+}
>+
>+TEST(ClipboardChromiumTest, InvalidCharacters)
>+{
>+    String name = makeString("na", String(invalidCharacters, arraysize(invalidCharacters)), "me");
>+    String extension = makeString("e", String(invalidCharacters, arraysize(invalidCharacters)), "xt");
>+    ClipboardChromium::validateFilename(name, extension);
>+    EXPECT_EQ("name", name);
>+    EXPECT_EQ("ext", extension);
>+}
>+
>+TEST(ClipboardChromiumTest, ExtensionTooLong)
>+{
>+    String name;
>+    String extension = makeString(longString, longString);
>+    ClipboardChromium::validateFilename(name, extension);
>+    EXPECT_EQ(String(), extension);
>+}
>+
>+TEST(ClipboardChromiumTest, NamePlusExtensionTooLong)
>+{
>+    String name = makeString(longString, longString);
>+    String extension = longString;
>+    ClipboardChromium::validateFilename(name, extension);
>+#if OS(DARWIN)
>+    EXPECT_EQ("0,1,1,2,3,5,8,13,21,34,55,89,144,233,377,610,987,1597,2584,4181,6765,109", name);
>+#elif OS(WINDOWS)
>+    EXPECT_EQ("0,1,1,2,3,5,8,13,21,34,55,89,144,233,377,610,987,1597,2584,4181,6765,10946,1", name);
>+#endif
>+    EXPECT_EQ(longString, extension);
>+#if OS(DARWIN)
>+    EXPECT_EQ(254, name.length() + extension.length());
>+#elif OS(WINDOWS)
>+    EXPECT_EQ(258, name.length() + extension.length());
>+#endif
>+}
>+
>+} // anonymous namespace
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 76996: 123905 | 123994 | 123995 | 124008