Be explicit about target type in platform_util::OpenItem()

chrome/browser/chromeos/file_manager/open_util.cc

Index: chrome/browser/chromeos/file_manager/open_util.cc
diff --git a/chrome/browser/chromeos/file_manager/open_util.cc b/chrome/browser/chromeos/file_manager/open_util.cc
index dad853242d6b4dd7ad68aacec4cdd91ebf68d9c0..03c9508a641feec7643ecb403e86691a0b9eb2a7 100644
--- a/chrome/browser/chromeos/file_manager/open_util.cc
+++ b/chrome/browser/chromeos/file_manager/open_util.cc
@@ -127,10 +127,10 @@ void OpenFileWithMimeType(Profile* profile,
 // Opens the file specified by |url| by finding and executing a file task for
 // the file. In case of success, calls |callback| with true. Otherwise the
 // returned value is false.
-void OpenFile(Profile* profile,
-              const base::FilePath& path,
-              const GURL& url,
-              const base::Callback<void(bool)>& callback) {
+void OpenFileInternal(Profile* profile,
+                      const base::FilePath& path,
+                      const GURL& url,
+                      const base::Callback<void(bool)>& callback) {
   extensions::app_file_handler_util::GetMimeTypeForLocalPath(
       profile,
       path,
@@ -138,7 +138,7 @@ void OpenFile(Profile* profile,
 }
 
 // Called when execution of ContinueOpenItem() is completed.
-void OnContinueOpenItemCompleted(Profile* profile,
+void OnContinueOpenFileCompleted(Profile* profile,
                                  const base::FilePath& file_path,
                                  bool result) {
   if (!result) {
@@ -154,23 +154,30 @@ void OnContinueOpenItemCompleted(Profile* profile,
   }
 }
 
-// Used to implement OpenItem().
-void ContinueOpenItem(Profile* profile,
+void ContinueOpenFile(Profile* profile,
                       const base::FilePath& file_path,
                       const GURL& url,
-                      base::File::Error error) {
+                      base::File::Error folder_check_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (error == base::File::FILE_OK) {
-    // A directory exists at |url|. Open it with the file manager.
-    OpenFileManagerWithInternalActionId(profile, url, "open");
-  } else {
-    // |url| should be a file. Open it.
-    OpenFile(profile,
-             file_path,
-             url,
-             base::Bind(&OnContinueOpenItemCompleted, profile, file_path));
+  if (folder_check_result == base::File::FILE_OK) {
+    OnContinueOpenFileCompleted(profile, file_path, false);
+    return;
   }
+  OpenFileInternal(
+      profile,
+      file_path,
+      url,
+      base::Bind(&OnContinueOpenFileCompleted, profile, file_path));
+}
+
+void ContinueOpenFolder(Profile* profile,
+                        const GURL& url,
+                        base::File::Error folder_check_result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (folder_check_result != base::File::FILE_OK)
+    return;
+  OpenFileManagerWithInternalActionId(profile, url, "open");

[hashimoto, 2014/09/26 06:12:43]

Sorry, I should have noticed this before.
Does this really prevent inappropriate files from being opened?

I think the directory at the path can be replaced with a file with the same name
at a timing between CheckIfDirectoryExists() and the path gets actually opened.
Maybe we should leave Chrome OS code unchanged as it doesn't allow executable
files running after all.
Though, it's still possible to open an arbitrary file to launch the associated
app. (should we also be worried about this problem?)

[hashimoto, 2014/09/26 06:37:20]

hirono@ said OpenFileManagerWithInternalActionId() never launches apps other
than Files.app itself. (thanks hirono@!)
So it seems what we should do is to drop CheckIfDirectoryExists() and just call
OpenFileManagerWithInternalActionId() directly from OpenFolder().
Also, OpenFile() can be changed to skip directory existence check too.

[asanka, 2014/09/26 19:35:41]

Thanks! Done.

 }
 
 // Converts the |given_path| passed from external callers to the form that the
@@ -210,7 +217,21 @@ void OpenRemovableDrive(Profile* profile, const base::FilePath& file_path) {
   OpenFileManagerWithInternalActionId(profile, url, "auto-open");
 }
 
-void OpenItem(Profile* profile, const base::FilePath& file_path) {
+void OpenFile(Profile* profile, const base::FilePath& file_path) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  base::FilePath converted_path;
+  GURL url;
+  if (!ConvertPath(profile, file_path, &converted_path, &url))
+    return;
+
+  CheckIfDirectoryExists(
+      GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
+      url,
+      base::Bind(&ContinueOpenFile, profile, converted_path, url));
+}
+
+void OpenFolder(Profile* profile, const base::FilePath& file_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   base::FilePath converted_path;
@@ -221,7 +242,7 @@ void OpenItem(Profile* profile, const base::FilePath& file_path) {
   CheckIfDirectoryExists(
       GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
       url,
-      base::Bind(&ContinueOpenItem, profile, converted_path, url));
+      base::Bind(&ContinueOpenFolder, profile, url));
 }
 
 void ShowItemInFolder(Profile* profile, const base::FilePath& file_path) {