Don't populate URL data in WebDropData when dragging files.

This is considered a potential security issue as well, since it leaks
filesystem paths.

BUG=332579

Review URL: https://codereview.chromium.org/135633002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244538 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/bookmarks/bookmark_node_data_unittest.cc

@@ -128,7 +128,8 @@ TEST_F(BookmarkNodeDataTest, URL) {
   // Writing should also put the URL and title on the clipboard.
   GURL read_url;
   base::string16 read_title;
-  EXPECT_TRUE(data2.GetURLAndTitle(&read_url, &read_title));
+  EXPECT_TRUE(data2.GetURLAndTitle(
+      ui::OSExchangeData::CONVERT_FILENAMES, &read_url, &read_title));
   EXPECT_EQ(url, read_url);
   EXPECT_EQ(title, read_title);
 }

chrome/browser/bookmarks/bookmark_node_data_views.cc

@@ -56,7 +56,8 @@ bool BookmarkNodeData::Read(const ui::OSExchangeData& data) {
     Element element;
     GURL url;
     base::string16 title;
-    if (data.GetURLAndTitle(&url, &title))
+    if (data.GetURLAndTitle(
+            ui::OSExchangeData::CONVERT_FILENAMES, &url, &title))
       ReadFromTuple(url, title);
   }
 

chrome/browser/ui/views/frame/browser_root_view.cc

@@ -96,7 +96,9 @@ int BrowserRootView::OnPerformDrop(const ui::DropTargetEvent& event) {
   GURL url;
   base::string16 title;
   ui::OSExchangeData mapped_data;
-  if (!event.data().GetURLAndTitle(&url, &title) || !url.is_valid()) {
+  if (!event.data().GetURLAndTitle(
+           ui::OSExchangeData::CONVERT_FILENAMES, &url, &title) ||
+      !url.is_valid()) {
     // The url isn't valid. Use the paste and go url.
     if (GetPasteAndGoURL(event.data(), &url))
       mapped_data.SetURL(url, base::string16());

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -857,7 +857,8 @@ int OmniboxViewViews::OnDrop(const ui::OSExchangeData& data) {
   if (data.HasURL()) {
     GURL url;
     base::string16 title;
-    if (data.GetURLAndTitle(&url, &title)) {
+    if (data.GetURLAndTitle(
+            ui::OSExchangeData::CONVERT_FILENAMES, &url, &title)) {
       base::string16 text(
           StripJavascriptSchemas(base::UTF8ToUTF16(url.spec())));
       if (model()->CanPasteAndGo(text)) {

chrome/browser/ui/views/tabs/tab_strip.cc

@@ -1410,7 +1410,9 @@ void TabStrip::OnDragEntered(const DropTargetEvent& event) {
   base::string16 title;
 
   // Check whether the event data includes supported drop data.
-  if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) {
+  if (event.data().GetURLAndTitle(
+          ui::OSExchangeData::CONVERT_FILENAMES, &url, &title) &&
+      url.is_valid()) {
     drop_info_->url = url;
 
     // For file:// URLs, kick off a MIME type request in case they're dropped.
@@ -1450,7 +1452,9 @@ int TabStrip::OnPerformDrop(const DropTargetEvent& event) {
   GURL url;
   base::string16 title;
   if (!file_supported ||
-      !event.data().GetURLAndTitle(&url, &title) || !url.is_valid())
+      !event.data().GetURLAndTitle(
+           ui::OSExchangeData::CONVERT_FILENAMES, &url, &title) ||
+      !url.is_valid())
     return ui::DragDropTypes::DRAG_NONE;
 
   controller()->PerformDrop(drop_before, drop_index, url);

chrome/browser/ui/views/toolbar/home_button.cc

@@ -165,7 +165,8 @@ int HomeButton::OnDragUpdated(const ui::DropTargetEvent& event) {
 int HomeButton::OnPerformDrop(const ui::DropTargetEvent& event) {
   GURL new_homepage_url;
   base::string16 title;
-  if (event.data().GetURLAndTitle(&new_homepage_url, &title) &&
+  if (event.data().GetURLAndTitle(
+          ui::OSExchangeData::CONVERT_FILENAMES, &new_homepage_url, &title) &&
       new_homepage_url.is_valid()) {
     PrefService* prefs = browser_->profile()->GetPrefs();
     bool old_is_ntp = prefs->GetBoolean(prefs::kHomePageIsNewTabPage);

content/browser/web_contents/web_contents_view_aura.cc

@@ -372,7 +372,8 @@ void PrepareDropData(DropData* drop_data, const ui::OSExchangeData& data) {
 
   GURL url;
   base::string16 url_title;
-  data.GetURLAndTitle(&url, &url_title);
+  data.GetURLAndTitle(
+      ui::OSExchangeData::DO_NOT_CONVERT_FILENAMES, &url, &url_title);
   if (url.is_valid()) {
     drop_data->url = url;
     drop_data->url_title = url_title;

ui/base/dragdrop/os_exchange_data.cc

@@ -62,8 +62,10 @@ bool OSExchangeData::GetString(base::string16* data) const {
   return provider_->GetString(data);
 }
 
-bool OSExchangeData::GetURLAndTitle(GURL* url, base::string16* title) const {
-  return provider_->GetURLAndTitle(url, title);
+bool OSExchangeData::GetURLAndTitle(FilenameToURLPolicy policy,
+                                    GURL* url,
+                                    base::string16* title) const {
+  return provider_->GetURLAndTitle(policy, url, title);
 }
 
 bool OSExchangeData::GetFilename(base::FilePath* path) const {

ui/base/dragdrop/os_exchange_data.h

@@ -68,6 +68,10 @@ class UI_BASE_EXPORT OSExchangeData {
 #endif
   };
 
+  // Controls whether or not filenames should be converted to file: URLs when
+  // getting a URL.
+  enum FilenameToURLPolicy { CONVERT_FILENAMES, DO_NOT_CONVERT_FILENAMES, };
+
   // Encapsulates the info about a file to be downloaded.
   struct UI_BASE_EXPORT DownloadFileInfo {
     DownloadFileInfo(const base::FilePath& filename,
@@ -107,7 +111,9 @@ class UI_BASE_EXPORT OSExchangeData {
                                 const Pickle& data) = 0;
 
     virtual bool GetString(base::string16* data) const = 0;
-    virtual bool GetURLAndTitle(GURL* url, base::string16* title) const = 0;
+    virtual bool GetURLAndTitle(FilenameToURLPolicy policy,
+                                GURL* url,
+                                base::string16* title) const = 0;
     virtual bool GetFilename(base::FilePath* path) const = 0;
     virtual bool GetFilenames(
         std::vector<FileInfo>* file_names) const = 0;
@@ -183,7 +189,9 @@ class UI_BASE_EXPORT OSExchangeData {
   // not exist, the out parameter is not touched. The out parameter cannot be
   // NULL.
   bool GetString(base::string16* data) const;
-  bool GetURLAndTitle(GURL* url, base::string16* title) const;
+  bool GetURLAndTitle(FilenameToURLPolicy policy,
+                      GURL* url,
+                      base::string16* title) const;
   // Return the path of a file, if available.
   bool GetFilename(base::FilePath* path) const;
   bool GetFilenames(

ui/base/dragdrop/os_exchange_data_provider_aura.cc

@@ -73,8 +73,11 @@ bool OSExchangeDataProviderAura::GetString(base::string16* data) const {
   return true;
 }
 
-bool OSExchangeDataProviderAura::GetURLAndTitle(GURL* url,
-                                                base::string16* title) const {
+bool OSExchangeDataProviderAura::GetURLAndTitle(
+    OSExchangeData::FilenameToURLPolicy policy,
+    GURL* url,
+    base::string16* title) const {
+  // TODO(dcheng): implement filename conversion.
   if ((formats_ & OSExchangeData::URL) == 0) {
     title->clear();
     return GetPlainTextURL(url);

ui/base/dragdrop/os_exchange_data_provider_aura.h

@@ -35,7 +35,9 @@ class UI_BASE_EXPORT OSExchangeDataProviderAura
   virtual void SetPickledData(const OSExchangeData::CustomFormat& format,
                               const Pickle& data) OVERRIDE;
   virtual bool GetString(base::string16* data) const OVERRIDE;
-  virtual bool GetURLAndTitle(GURL* url, base::string16* title) const OVERRIDE;
+  virtual bool GetURLAndTitle(OSExchangeData::FilenameToURLPolicy policy,
+                              GURL* url,
+                              base::string16* title) const OVERRIDE;
   virtual bool GetFilename(base::FilePath* path) const OVERRIDE;
   virtual bool GetFilenames(
       std::vector<OSExchangeData::FileInfo>* filenames) const OVERRIDE;

ui/base/dragdrop/os_exchange_data_provider_aurax11.cc

@@ -197,6 +197,7 @@ bool OSExchangeDataProviderAuraX11::GetString(base::string16* result) const {
 }
 
 bool OSExchangeDataProviderAuraX11::GetURLAndTitle(
+    OSExchangeData::FilenameToURLPolicy policy,
     GURL* url,
     base::string16* title) const {
   std::vector< ::Atom> url_atoms = ui::GetURLAtomsFrom(&atom_cache_);
@@ -231,7 +232,8 @@ bool OSExchangeDataProviderAuraX11::GetURLAndTitle(
       for (std::vector<std::string>::const_iterator it = tokens.begin();
            it != tokens.end(); ++it) {
         GURL test_url(*it);
-        if (!test_url.SchemeIsFile()) {
+        if (!test_url.SchemeIsFile() ||
+            policy == OSExchangeData::CONVERT_FILENAMES) {
           *url = test_url;
           *title = base::string16();
           return true;

ui/base/dragdrop/os_exchange_data_provider_aurax11.h

@@ -66,7 +66,9 @@ class UI_BASE_EXPORT OSExchangeDataProviderAuraX11
   virtual void SetPickledData(const OSExchangeData::CustomFormat& format,
                               const Pickle& pickle) OVERRIDE;
   virtual bool GetString(base::string16* data) const OVERRIDE;
-  virtual bool GetURLAndTitle(GURL* url, base::string16* title) const OVERRIDE;
+  virtual bool GetURLAndTitle(OSExchangeData::FilenameToURLPolicy policy,
+                              GURL* url,
+                              base::string16* title) const OVERRIDE;
   virtual bool GetFilename(base::FilePath* path) const OVERRIDE;
   virtual bool GetFilenames(
       std::vector<OSExchangeData::FileInfo>* filenames) const OVERRIDE;

ui/base/dragdrop/os_exchange_data_provider_aurax11_unittest.cc

@@ -46,7 +46,8 @@ TEST_F(OSExchangeDataProviderAuraX11Test, MozillaURL) {
   {
     GURL out_gurl;
     base::string16 out_str;
-    EXPECT_TRUE(provider.GetURLAndTitle(&out_gurl, &out_str));
+    EXPECT_TRUE(provider.GetURLAndTitle(
+        OSExchangeData::DO_NOT_CONVERT_FILENAMES, &out_gurl, &out_str));
     EXPECT_EQ(base::ASCIIToUTF16(kGoogleTitle), out_str);
     EXPECT_EQ(kGoogleURL, out_gurl.spec());
   }
@@ -56,7 +57,8 @@ TEST_F(OSExchangeDataProviderAuraX11Test, MozillaURL) {
   {
     GURL out_gurl;
     base::string16 out_str;
-    EXPECT_TRUE(provider.GetURLAndTitle(&out_gurl, &out_str));
+    EXPECT_TRUE(provider.GetURLAndTitle(
+        OSExchangeData::DO_NOT_CONVERT_FILENAMES, &out_gurl, &out_str));
     EXPECT_EQ(base::string16(), out_str);
     EXPECT_EQ(kGoogleURL, out_gurl.spec());
   }
@@ -91,7 +93,8 @@ TEST_F(OSExchangeDataProviderAuraX11Test, URIListWithBoth) {
   // We should only receive the URL here.
   GURL out_gurl;
   base::string16 out_str;
-  EXPECT_TRUE(provider.GetURLAndTitle(&out_gurl, &out_str));
+  EXPECT_TRUE(provider.GetURLAndTitle(
+      OSExchangeData::DO_NOT_CONVERT_FILENAMES, &out_gurl, &out_str));
   EXPECT_EQ(base::string16(), out_str);
   EXPECT_EQ(kGoogleURL, out_gurl.spec());
 }

ui/base/dragdrop/os_exchange_data_provider_win.cc

@@ -386,10 +386,16 @@ bool OSExchangeDataProviderWin::GetString(base::string16* data) const {
   return ClipboardUtil::GetPlainText(source_object_, data);
 }
 
-bool OSExchangeDataProviderWin::GetURLAndTitle(GURL* url,
-                                               base::string16* title) const {
+bool OSExchangeDataProviderWin::GetURLAndTitle(
+    OSExchangeData::FilenameToURLPolicy policy,
+    GURL* url,
+    base::string16* title) const {
   base::string16 url_str;
-  bool success = ClipboardUtil::GetUrl(source_object_, &url_str, title, true);
+  bool success = ClipboardUtil::GetUrl(
+      source_object_,
+      &url_str,
+      title,
+      policy == OSExchangeData::CONVERT_FILENAMES ? true : false);
   if (success) {
     GURL test_url(url_str);
     if (test_url.is_valid()) {

ui/base/dragdrop/os_exchange_data_provider_win.h

@@ -161,7 +161,9 @@ class UI_BASE_EXPORT OSExchangeDataProviderWin
   virtual void SetHtml(const base::string16& html, const GURL& base_url);
 
   virtual bool GetString(base::string16* data) const;
-  virtual bool GetURLAndTitle(GURL* url, base::string16* title) const;
+  virtual bool GetURLAndTitle(OSExchangeData::FilenameToURLPolicy policy,
+                              GURL* url,
+                              base::string16* title) const;
   virtual bool GetFilename(base::FilePath* path) const;
   virtual bool GetFilenames(
       std::vector<OSExchangeData::FileInfo>* filenames) const;

ui/base/dragdrop/os_exchange_data_unittest.cc

@@ -29,7 +29,8 @@ TEST_F(OSExchangeDataTest, StringDataGetAndSet) {
   std::string url_spec = "http://www.goats.com/";
   GURL url(url_spec);
   base::string16 title;
-  EXPECT_FALSE(data2.GetURLAndTitle(&url, &title));
+  EXPECT_FALSE(
+      data2.GetURLAndTitle(OSExchangeData::CONVERT_FILENAMES, &url, &title));
   // No URLs in |data|, so url should be untouched.
   EXPECT_EQ(url_spec, url.spec());
 }
@@ -47,7 +48,8 @@ TEST_F(OSExchangeDataTest, TestURLExchangeFormats) {
   // URL spec and title should match
   GURL output_url;
   base::string16 output_title;
-  EXPECT_TRUE(data2.GetURLAndTitle(&output_url, &output_title));
+  EXPECT_TRUE(data2.GetURLAndTitle(
+      OSExchangeData::CONVERT_FILENAMES, &output_url, &output_title));
   EXPECT_EQ(url_spec, output_url.spec());
   EXPECT_EQ(url_title, output_title);
   base::string16 output_string;

ui/base/dragdrop/os_exchange_data_win_unittest.cc

@@ -65,7 +65,8 @@ TEST(OSExchangeDataWinTest, StringDataWritingViaCOM) {
   EXPECT_TRUE(data2.HasURL());
   GURL url_from_data;
   std::wstring title;
-  EXPECT_TRUE(data2.GetURLAndTitle(&url_from_data, &title));
+  EXPECT_TRUE(data2.GetURLAndTitle(
+      OSExchangeData::CONVERT_FILENAMES, &url_from_data, &title));
   GURL reference_url(input);
   EXPECT_EQ(reference_url.spec(), url_from_data.spec());
 }
@@ -116,7 +117,8 @@ TEST(OSExchangeDataWinTest, RemoveData) {
   EXPECT_TRUE(data2.HasURL());
   GURL url_from_data;
   std::wstring title;
-  EXPECT_TRUE(data2.GetURLAndTitle(&url_from_data, &title));
+  EXPECT_TRUE(data2.GetURLAndTitle(
+      OSExchangeData::CONVERT_FILENAMES, &url_from_data, &title));
   EXPECT_EQ(GURL(input2).spec(), url_from_data.spec());
 }
 
@@ -348,7 +350,8 @@ TEST(OSExchangeDataWinTest, ProvideURLForPlainTextURL) {
   ASSERT_TRUE(data2.HasURL());
   GURL read_url;
   std::wstring title;
-  EXPECT_TRUE(data2.GetURLAndTitle(&read_url, &title));
+  EXPECT_TRUE(data2.GetURLAndTitle(
+      OSExchangeData::CONVERT_FILENAMES, &read_url, &title));
   EXPECT_EQ(GURL("http://google.com"), read_url);
 }