Don't populate WebDropData with file URLs when dragging files.

This is the OS X patch. There will be separate patches for Windows and Linux.

BUG=42685
TEST=unit_tests --gtest_filter=WebDropTargetTest.*

Review URL: http://codereview.chromium.org/2095011

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

14 files changed, 125 insertions, 74 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index ed04eb0..ffa131e 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -2132,15 +2132,11 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
     // Create and add the new bookmark nodes.
     size_t urlCount = [urls count];
     for (size_t i = 0; i < urlCount; ++i) {
-      // URLs come in several forms (see NSPasteboard+Utils.mm).
       GURL gurl;
       const char* string = [[urls objectAtIndex:i] UTF8String];
       if (string)
         gurl = GURL(string);
-      if (!gurl.is_valid() && string) {
-        gurl = GURL([[NSString stringWithFormat:@"file://%s", string]
-                     UTF8String]);
-      }
+      // We only expect to receive valid URLs.
       DCHECK(gurl.is_valid());
       if (gurl.is_valid()) {
         bookmarkModel_->AddURL(destParent,
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 395b416..5ea549c 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -951,7 +951,7 @@ TEST_F(BookmarkBarControllerTest, DropBookmarks) {
     "http://qwantz.com",
     "http://xkcd.com",
     "javascript:alert('lolwut')",
-    "/tmp/local-file.txt"  // As if dragged from the desktop.
+    "file://localhost/tmp/local-file.txt"  // As if dragged from the desktop.
   };
   std::wstring titles[] = {
     std::wstring(L"Philosophoraptor"),
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index 2f8f70f..a8f696e 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -1209,15 +1209,11 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   // Create and add the new bookmark nodes.
   size_t urlCount = [urls count];
   for (size_t i = 0; i < urlCount; ++i) {
-    // URLs come in several forms (see NSPasteboard+Utils.mm).
     GURL gurl;
     const char* string = [[urls objectAtIndex:i] UTF8String];
     if (string)
       gurl = GURL(string);
-    if (!gurl.is_valid() && string) {
-      gurl = GURL([[NSString stringWithFormat:@"file://%s", string]
-                   UTF8String]);
-    }
+    // We only expect to receive valid URLs.
     DCHECK(gurl.is_valid());
     if (gurl.is_valid()) {
       bookmarkModel->AddURL(destParent,
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view.mm b/chrome/browser/cocoa/bookmark_bar_folder_view.mm
index f3e1791..d7fd23f 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_view.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_view.mm
@@ -158,7 +158,7 @@
 
   NSArray* urls = nil;
   NSArray* titles = nil;
-  [pboard getURLs:&urls andTitles:&titles];
+  [pboard getURLs:&urls andTitles:&titles convertingFilenames:YES];
 
   return [[self controller] addURLs:urls
                          withTitles:titles
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
index a1e2b1e..3fb4d11 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
@@ -111,7 +111,9 @@ namespace {
   return YES;
 }
 
-- (void)getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles {
+- (void)getURLs:(NSArray**)outUrls
+    andTitles:(NSArray**)outTitles
+    convertingFilenames:(BOOL)convertFilenames {
 }
 
 - (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info {
diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm
index f4c9f15..6923081 100644
--- a/chrome/browser/cocoa/bookmark_bar_view.mm
+++ b/chrome/browser/cocoa/bookmark_bar_view.mm
@@ -42,7 +42,7 @@
                         name:kBrowserThemeDidChangeNotification
                       object:nil];
 
-  DCHECK(controller_ && "Expected this to be hooked up via Interface Builder");
+  DCHECK(controller_) << "Expected this to be hooked up via Interface Builder";
   NSArray* types = [NSArray arrayWithObjects:
                     NSStringPboardType,
                     NSHTMLPboardType,
@@ -200,7 +200,7 @@
 
   NSArray* urls = nil;
   NSArray* titles = nil;
-  [pboard getURLs:&urls andTitles:&titles];
+  [pboard getURLs:&urls andTitles:&titles convertingFilenames:YES];
 
   return [controller_ addURLs:urls
                    withTitles:titles
diff --git a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
index 88168ff..1b43c59 100644
--- a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
@@ -111,7 +111,9 @@ namespace {
   return YES;
 }
 
-- (void)getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles {
+- (void)getURLs:(NSArray**)outUrls
+    andTitles:(NSArray**)outTitles
+    convertingFilenames:(BOOL)convertFilenames {
 }
 
 - (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info {
diff --git a/chrome/browser/cocoa/url_drop_target.mm b/chrome/browser/cocoa/url_drop_target.mm
index dfcd99b..8e3f19b 100644
--- a/chrome/browser/cocoa/url_drop_target.mm
+++ b/chrome/browser/cocoa/url_drop_target.mm
@@ -62,7 +62,7 @@
   if ([pboard containsURLData]) {
     NSArray* urls = nil;
     NSArray* titles;  // discarded
-    [pboard getURLs:&urls andTitles:&titles];
+    [pboard getURLs:&urls andTitles:&titles convertingFilenames:YES];
 
     if ([urls count]) {
       // Tell the window controller about the dropped URL(s).
diff --git a/chrome/browser/cocoa/web_drop_target.h b/chrome/browser/cocoa/web_drop_target.h
index 63c7288..671b51f 100644
--- a/chrome/browser/cocoa/web_drop_target.h
+++ b/chrome/browser/cocoa/web_drop_target.h
@@ -4,6 +4,9 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/string16.h"
+
+class GURL;
 class RenderViewHost;
 class TabContents;
 class WebDropData;
@@ -53,11 +56,15 @@ typedef RenderViewHost* RenderViewHostIdentifier;
 
 // Public use only for unit tests.
 @interface WebDropTarget(Testing)
-// Populate the URL portion of |data|. There may be more than one, but we only
-// handle dropping the first. |data| must not be |NULL|. Returns |YES| if URL
-// data was obtained from the pasteboard, |NO| otherwise.
-- (BOOL)populateURLAndTitle:(WebDropData*)data
-             fromPasteboard:(NSPasteboard*)pboard;
+// Populate the |url| and |title| with URL data in |pboard|. There may be more
+// than one, but we only handle dropping the first. |url| must not be |NULL|;
+// |title| is an optional parameter. Returns |YES| if URL data was obtained from
+// the pasteboard, |NO| otherwise. If |convertFilenames| is |YES|, the function
+// will also attempt to convert filenames in |pboard| to file URLs.
+- (BOOL)populateURL:(GURL*)url
+    andTitle:(string16*)title
+    fromPasteboard:(NSPasteboard*)pboard
+    convertingFilenames:(BOOL)convertFilenames;
 // Given |data|, which should not be nil, fill it in using the contents of the
 // given pasteboard.
 - (void)populateWebDropData:(WebDropData*)data
diff --git a/chrome/browser/cocoa/web_drop_target.mm b/chrome/browser/cocoa/web_drop_target.mm
index 6ee1874..68c0b79 100644
--- a/chrome/browser/cocoa/web_drop_target.mm
+++ b/chrome/browser/cocoa/web_drop_target.mm
@@ -159,9 +159,12 @@ using WebKit::WebDragOperationsMask;
   if ([self onlyAllowsNavigation]) {
     NSPasteboard* pboard = [info draggingPasteboard];
     if ([pboard containsURLData]) {
-      WebDropData data;
-      [self populateURLAndTitle:&data fromPasteboard:pboard];
-      tabContents_->OpenURL(data.url, GURL(), CURRENT_TAB,
+      GURL url;
+      [self populateURL:&url
+          andTitle:NULL
+          fromPasteboard:pboard
+          convertingFilenames:YES];
+      tabContents_->OpenURL(url, GURL(), CURRENT_TAB,
                             PageTransition::AUTO_BOOKMARK);
       return YES;
     }
@@ -189,42 +192,41 @@ using WebKit::WebDragOperationsMask;
   return YES;
 }
 
-// Populate the URL portion of |data|. There may be more than one, but we only
-// handle dropping the first. |data| must not be |NULL|. Returns |YES| if URL
-// data was obtained from the pasteboard, |NO| otherwise.
-- (BOOL)populateURLAndTitle:(WebDropData*)data
-             fromPasteboard:(NSPasteboard*)pboard {
-  DCHECK(data);
+// Populate the |url| and |title| with URL data in |pboard|. There may be more
+// than one, but we only handle dropping the first. |url| must not be |NULL|;
+// |title| is an optional parameter. Returns |YES| if URL data was obtained from
+// the pasteboard, |NO| otherwise. If |convertFilenames| is |YES|, the function
+// will also attempt to convert filenames in |pboard| to file URLs.
+- (BOOL)populateURL:(GURL*)url
+    andTitle:(string16*)title
+    fromPasteboard:(NSPasteboard*)pboard
+    convertingFilenames:(BOOL)convertFilenames {
+  DCHECK(url);
+  DCHECK(title);
 
   // Bail out early if there's no URL data.
   if (![pboard containsURLData])
     return NO;
 
-  // |-getURLs:andTitles:| will already validate URIs so we don't need to again.
-  // However, if the URI is a local file, it won't be prefixed with file://,
-  // which is what GURL expects. We can detect that case because the resulting
-  // URI will have no valid scheme, and we'll assume it's a local file. The
-  // arrays returned are both of NSString's.
+  // |-getURLs:andTitles:convertingFilenames:| will already validate URIs so we
+  // don't need to again. The arrays returned are both of NSString's.
   NSArray* urls = nil;
   NSArray* titles = nil;
-  [pboard getURLs:&urls andTitles:&titles];
+  [pboard getURLs:&urls andTitles:&titles convertingFilenames:convertFilenames];
   DCHECK_EQ([urls count], [titles count]);
   // It's possible that no URLs were actually provided!
   if (![urls count])
     return NO;
   NSString* urlString = [urls objectAtIndex:0];
   if ([urlString length]) {
-    NSURL* url = [NSURL URLWithString:urlString];
-    if (![url scheme])
-      urlString = [[NSURL fileURLWithPath:urlString] absoluteString];
     // Check again just to make sure to not assign NULL into a std::string,
     // which throws an exception.
     const char* utf8Url = [urlString UTF8String];
     if (utf8Url) {
-      data->url = GURL(utf8Url);
+      *url = GURL(utf8Url);
       // Extra paranoia check.
-      if ([titles count])
-        data->url_title = base::SysNSStringToUTF16([titles objectAtIndex:0]);
+      if (title && [titles count])
+        *title = base::SysNSStringToUTF16([titles objectAtIndex:0]);
     }
   }
   return YES;
@@ -238,8 +240,12 @@ using WebKit::WebDragOperationsMask;
   DCHECK(pboard);
   NSArray* types = [pboard types];
 
-  // Get URL if possible.
-  [self populateURLAndTitle:data fromPasteboard:pboard];
+  // Get URL if possible. To avoid exposing file system paths to web content,
+  // filenames in the drag are not converted to file URLs.
+  [self populateURL:&data->url
+      andTitle:&data->url_title
+      fromPasteboard:pboard
+      convertingFilenames:NO];
 
   // Get plain text.
   if ([types containsObject:NSStringPboardType]) {
diff --git a/chrome/browser/cocoa/web_drop_target_unittest.mm b/chrome/browser/cocoa/web_drop_target_unittest.mm
index 1df01441..99405bb 100644
--- a/chrome/browser/cocoa/web_drop_target_unittest.mm
+++ b/chrome/browser/cocoa/web_drop_target_unittest.mm
@@ -4,6 +4,7 @@
 
 #include "base/scoped_nsautorelease_pool.h"
 #include "base/sys_string_conversions.h"
+#include "base/utf_string_conversions.h"
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
 #import "chrome/browser/cocoa/web_drop_target.h"
 #include "chrome/browser/renderer_host/test/test_render_view_host.h"
@@ -59,26 +60,30 @@ TEST_F(WebDropTargetTest, Flip) {
   NSPoint screenPoint =
       [drop_target_ flipWindowPointToScreen:windowPoint
                                view:[window contentView]];
-  EXPECT_EQ(viewPoint.x, 0);
-  EXPECT_EQ(viewPoint.y, 600);
-  EXPECT_EQ(screenPoint.x, 0);
+  EXPECT_EQ(0, viewPoint.x);
+  EXPECT_EQ(600, viewPoint.y);
+  EXPECT_EQ(0, screenPoint.x);
   // We can't put a value on the screen size since everyone will have a
   // different one.
-  EXPECT_NE(screenPoint.y, 0);
+  EXPECT_NE(0, screenPoint.y);
 }
 
 TEST_F(WebDropTargetTest, URL) {
   NSPasteboard* pboard = nil;
   NSString* url = nil;
   NSString* title = nil;
-  WebDropData data;
+  GURL result_url;
+  string16 result_title;
 
   // Put a URL on the pasteboard and check it.
   pboard = [NSPasteboard pasteboardWithUniqueName];
   url = @"http://www.google.com/";
   PutURLOnPasteboard(url, pboard);
-  EXPECT_TRUE([drop_target_ populateURLAndTitle:&data fromPasteboard:pboard]);
-  EXPECT_EQ(data.url.spec(), base::SysNSStringToUTF8(url));
+  EXPECT_TRUE([drop_target_ populateURL:&result_url
+                               andTitle:&result_title
+                         fromPasteboard:pboard
+                    convertingFilenames:NO]);
+  EXPECT_EQ(base::SysNSStringToUTF8(url), result_url.spec());
   [pboard releaseGlobally];
 
   // Put a 'url ' and 'urln' on the pasteboard and check it.
@@ -86,9 +91,12 @@ TEST_F(WebDropTargetTest, URL) {
   url = @"http://www.google.com/";
   title = @"Title of Awesomeness!",
   PutCoreURLAndTitleOnPasteboard(url, title, pboard);
-  EXPECT_TRUE([drop_target_ populateURLAndTitle:&data fromPasteboard:pboard]);
-  EXPECT_EQ(data.url.spec(), base::SysNSStringToUTF8(url));
-  EXPECT_EQ(data.url_title, base::SysNSStringToUTF16(title));
+  EXPECT_TRUE([drop_target_ populateURL:&result_url
+                               andTitle:&result_title
+                         fromPasteboard:pboard
+                    convertingFilenames:NO]);
+  EXPECT_EQ(base::SysNSStringToUTF8(url), result_url.spec());
+  EXPECT_EQ(base::SysNSStringToUTF16(title), result_title);
   [pboard releaseGlobally];
 
   // Also check that it passes file:// via 'url '/'urln' properly.
@@ -96,9 +104,12 @@ TEST_F(WebDropTargetTest, URL) {
   url = @"file:///tmp/dont_delete_me.txt";
   title = @"very important";
   PutCoreURLAndTitleOnPasteboard(url, title, pboard);
-  EXPECT_TRUE([drop_target_ populateURLAndTitle:&data fromPasteboard:pboard]);
-  EXPECT_EQ(data.url.spec(), base::SysNSStringToUTF8(url));
-  EXPECT_EQ(data.url_title, base::SysNSStringToUTF16(title));
+  EXPECT_TRUE([drop_target_ populateURL:&result_url
+                               andTitle:&result_title
+                         fromPasteboard:pboard
+                    convertingFilenames:NO]);
+  EXPECT_EQ(base::SysNSStringToUTF8(url), result_url.spec());
+  EXPECT_EQ(base::SysNSStringToUTF16(title), result_title);
   [pboard releaseGlobally];
 
   // And javascript:.
@@ -106,9 +117,30 @@ TEST_F(WebDropTargetTest, URL) {
   url = @"javascript:open('http://www.youtube.com/')";
   title = @"kill some time";
   PutCoreURLAndTitleOnPasteboard(url, title, pboard);
-  EXPECT_TRUE([drop_target_ populateURLAndTitle:&data fromPasteboard:pboard]);
-  EXPECT_EQ(data.url.spec(), base::SysNSStringToUTF8(url));
-  EXPECT_EQ(data.url_title, base::SysNSStringToUTF16(title));
+  EXPECT_TRUE([drop_target_ populateURL:&result_url
+                               andTitle:&result_title
+                         fromPasteboard:pboard
+                    convertingFilenames:NO]);
+  EXPECT_EQ(base::SysNSStringToUTF8(url), result_url.spec());
+  EXPECT_EQ(base::SysNSStringToUTF16(title), result_title);
+  [pboard releaseGlobally];
+
+  pboard = [NSPasteboard pasteboardWithUniqueName];
+  url = @"/bin/sh";
+  [pboard declareTypes:[NSArray arrayWithObject:NSFilenamesPboardType]
+                 owner:nil];
+  [pboard setPropertyList:[NSArray arrayWithObject:url]
+                  forType:NSFilenamesPboardType];
+  EXPECT_FALSE([drop_target_ populateURL:&result_url
+                                andTitle:&result_title
+                          fromPasteboard:pboard
+                    convertingFilenames:NO]);
+  EXPECT_TRUE([drop_target_ populateURL:&result_url
+                               andTitle:&result_title
+                         fromPasteboard:pboard
+                    convertingFilenames:YES]);
+  EXPECT_EQ("file://localhost/bin/sh", result_url.spec());
+  EXPECT_EQ("sh", UTF16ToUTF8(result_title));
   [pboard releaseGlobally];
 }
 
@@ -126,8 +158,8 @@ TEST_F(WebDropTargetTest, Data) {
   [pboard setString:textString forType:NSStringPboardType];
   [drop_target_ populateWebDropData:&data fromPasteboard:pboard];
   EXPECT_EQ(data.url.spec(), "http://www.google.com/");
-  EXPECT_EQ(data.plain_text, base::SysNSStringToUTF16(textString));
-  EXPECT_EQ(data.text_html, base::SysNSStringToUTF16(htmlString));
+  EXPECT_EQ(base::SysNSStringToUTF16(textString), data.plain_text);
+  EXPECT_EQ(base::SysNSStringToUTF16(htmlString), data.text_html);
 
   [pboard releaseGlobally];
 }
diff --git a/third_party/mozilla/NSPasteboard+Utils.h b/third_party/mozilla/NSPasteboard+Utils.h
index 5c2f1e1..c75de6a 100644
--- a/third_party/mozilla/NSPasteboard+Utils.h
+++ b/third_party/mozilla/NSPasteboard+Utils.h
@@ -51,7 +51,9 @@ extern NSString* const kWebURLsWithTitlesPboardType;
 - (void) setDataForURL:(NSString*)url title:(NSString*)title;
 
 - (void) setURLs:(NSArray*)inUrls withTitles:(NSArray*)inTitles;
-- (void) getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles;
+- (void) getURLs:(NSArray**)outUrls
+    andTitles:(NSArray**)outTitles
+    convertingFilenames:(BOOL)convertFilenames;
 - (BOOL) containsURLData;
 
 @end
diff --git a/third_party/mozilla/NSPasteboard+Utils.mm b/third_party/mozilla/NSPasteboard+Utils.mm
index bfd87de..8ac1ba5 100644
--- a/third_party/mozilla/NSPasteboard+Utils.mm
+++ b/third_party/mozilla/NSPasteboard+Utils.mm
@@ -166,13 +166,15 @@ NSString* const kWebURLsWithTitlesPboardType  = @"WebURLsWithTitlesPboardType";
   }
 }
 
-//
-// Get the set of URLs and their corresponding titles from the clipboards
+// Get the set of URLs and their corresponding titles from the pasteboard.
 // If there are no URLs in a format we understand on the pasteboard empty
 // arrays will be returned. The two arrays will always be the same size.
-// The arrays returned are on the auto release pool.
-//
-- (void) getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles
+// The arrays returned are on the auto release pool. If |convertFilenames|
+// is YES, then the function will attempt to convert filenames in the drag
+// to file URLs.
+- (void) getURLs:(NSArray**)outUrls
+    andTitles:(NSArray**)outTitles
+    convertingFilenames:(BOOL)convertFilenames
 {
   NSArray* types = [self types];
   NSURL* urlFromNSURL = nil;  // Used below in getting an URL from the NSURLPboardType.
@@ -206,10 +208,13 @@ NSString* const kWebURLsWithTitlesPboardType  = @"WebURLsWithTitlesPboardType";
         }
       }
       
-      // Use the filename if not a .webloc or .url file, or if either of the
-      // functions returns nil.
       if (!urlString) {
-        urlString = file;
+        if (!convertFilenames) {
+          continue;
+        }
+        // Use the filename if not a .webloc or .url file, or if either of the
+        // functions returns nil.
+        urlString = [[NSURL fileURLWithPath:file] absoluteString];
         title     = [file lastPathComponent];
       }
 
diff --git a/third_party/mozilla/README.chromium b/third_party/mozilla/README.chromium
index 658fd72..fc5779c 100644
--- a/third_party/mozilla/README.chromium
+++ b/third_party/mozilla/README.chromium
@@ -12,7 +12,10 @@ Description:
     NSWorkspace+Utils.h/m
 
 Local modifications:
-  NSURL+Utils.m was modified to use non-deprecated Cocoa APIs to allow
+- NSURL+Utils.m was modified to use non-deprecated Cocoa APIs to allow
   compilation on future versions of Mac OS X.
-  NSString+Utils.m was renamed to NSString+Utils.mm and modified to use GURL
+- NSString+Utils.m was renamed to NSString+Utils.mm and modified to use GURL
   for validation in -[NSString isValidURI].
+- NSPasteboard+Utils.mm was modified to add an argument to
+  -[NSPasteboard getURLs:andTitles:] to determine whether or not filenames in
+  the drag should be converted to file URLs.