Use a prefix check to determine if a well-known MIME type is an image.

The original code used a different way to check that the extension is an
image extension (and not something else, like .exe), but this depended
on being able to directly access the blink::Image class. When the name
calculation was moved into the browser, blink::IsSupportedImageType was
used instead to ensure that a user doesn't drag out an image and get
something surprising like an executable instead.

However, it turns out that blink::IsSupportedImageType doesn't consider
SVG an image type--since it's actually an XML file. To get around this,
just simply check that the MIME type begins with image/. This is safe
enough, since the filename calculation only takes well-known MIME
types into consideration.

Bug: 767775
Change-Id: I5c71d8ae2daa9a413beab9c3c1cb393760383323
Reviewed-on: https://chromium-review.googlesource.com/721615
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509317}

content/public/common/drop_data.cc

@@ -7,7 +7,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "net/base/filename_util.h"
 #include "net/base/mime_util.h"
-#include "third_party/WebKit/common/mime_util/mime_util.h"
 
 namespace content {
 
@@ -65,7 +64,8 @@ base::Optional<base::FilePath> DropData::GetSafeFilenameForImageFileContents()
   std::string mime_type;
   if (net::GetWellKnownMimeTypeFromExtension(file_contents_filename_extension,
                                              &mime_type) &&
-      blink::IsSupportedImageMimeType(mime_type)) {
+      base::StartsWith(mime_type, "image/",
+                       base::CompareCase::INSENSITIVE_ASCII)) {
     return file_name.ReplaceExtension(file_contents_filename_extension);
   }
   return base::nullopt;

content/public/common/drop_data_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/common/drop_data.h"
+
+#include "base/files/file_path.h"
+#include "base/optional.h"
+#include "base/strings/stringprintf.h"
+#include "build/build_config.h"
+#include "net/base/mime_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_WIN)
+#include "base/strings/utf_string_conversions.h"
+#define CONVERT_IF_NEEDED(x) base::UTF8ToUTF16((x))
+#else
+#define CONVERT_IF_NEEDED(x) x
+#endif
+
+namespace content {
+
+TEST(DropDataTest, GetSafeFilenameForImageFileContents) {
+  static constexpr struct {
+    const char* const extension;
+    const bool is_well_known;
+    const bool should_generate_filename;
+  } kTestCases[] = {
+      // Extensions with a well-known but non-image MIME type should not result
+      // in the generation of a filename.
+      {"exe", true, false},
+      {"html", true, false},
+      {"js", true, false},
+      // Extensions that do not have a well-known MIME type should not result in
+      // the generation of a filename.
+      {"should-not-be-known-extension", false, false},
+      // Extensions with a well-known MIME type should result in the generation
+      // of a filename.
+      {"bmp", true, true},
+      {"gif", true, true},
+      {"ico", true, true},
+      {"jpg", true, true},
+      {"png", true, true},
+      {"svg", true, true},
+      {"tif", true, true},
+      {"xbm", true, true},
+      {"webp", true, true},
+  };
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(test_case.extension);
+    std::string ignored;
+    ASSERT_EQ(test_case.is_well_known,
+              net::GetWellKnownMimeTypeFromExtension(
+                  CONVERT_IF_NEEDED(test_case.extension), &ignored));
+
+    DropData drop_data;
+    drop_data.file_contents_source_url =
+        GURL(base::StringPrintf("https://example.com/testresource"));
+    drop_data.file_contents_filename_extension =
+        CONVERT_IF_NEEDED(test_case.extension);
+    base::Optional<base::FilePath> generated_name =
+        drop_data.GetSafeFilenameForImageFileContents();
+    ASSERT_EQ(test_case.should_generate_filename, generated_name.has_value());
+
+    if (test_case.should_generate_filename) {
+      // base::FilePath::Extension() returns the preceeding dot, so drop it
+      // before doing the comparison.
+      EXPECT_EQ(CONVERT_IF_NEEDED(test_case.extension),
+                generated_name->Extension().substr(1));
+    }
+  }
+}
+
+}  // namespace content
+
+#undef CONVERT_IF_NEEDED

content/test/BUILD.gn

@@ -1512,6 +1512,7 @@ test("content_unittests") {
     "../network/restricted_cookie_manager_impl_unittest.cc",
     "../network/throttling/throttling_controller_unittest.cc",
     "../network/url_loader_unittest.cc",
+    "../public/common/drop_data_unittest.cc",
     "../public/common/simple_url_loader_unittest.cc",
     "../public/common/url_utils_unittest.cc",
     "../public/test/referrer_unittest.cc",