We should not use FilePath::DirName() for sandboxed virtual paths

As well as FilePath::BaseName(). When a virtual path starts with drive-letter-like string like 'a:b' FilePath::DirName() returns 'a:' on Win32.

This is a follow-up patch for https://codereview.chromium.org/12253006/

BUG=176253
TEST=content_browsertests:FileSystemLayoutTest.OpRestrictedChars, content_unittests:FileSystemUtilTest.VirtualPathDirName

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

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

5 files changed, 84 insertions, 6 deletions

diff --git a/webkit/fileapi/file_system_util.cc b/webkit/fileapi/file_system_util.cc
index 1447c99..82aa01e 100644
--- a/webkit/fileapi/file_system_util.cc
+++ b/webkit/fileapi/file_system_util.cc
@@ -48,6 +48,40 @@ base::FilePath VirtualPath::BaseName(const base::FilePath& virtual_path) {
   return base::FilePath(path);
 }
 
+base::FilePath VirtualPath::DirName(const base::FilePath& virtual_path) {
+  typedef base::FilePath::StringType StringType;
+  StringType  path = virtual_path.value();
+
+  // The logic below is taken from that of base::FilePath::DirName, except
+  // that this version never cares about '//' or drive-letters even on win32.
+
+  // Strip trailing separators.
+  while (path.size() > 1 && base::FilePath::IsSeparator(path[path.size() - 1]))
+    path.resize(path.size() - 1);
+
+  StringType::size_type last_separator =
+      path.find_last_of(base::FilePath::kSeparators);
+  if (last_separator == StringType::npos) {
+    // path_ is in the current directory.
+    return base::FilePath(base::FilePath::kCurrentDirectory);
+  }
+  if (last_separator == 0) {
+    // path_ is in the root directory.
+    return base::FilePath(path.substr(0, 1));
+  }
+  // path_ is somewhere else, trim the basename.
+  path.resize(last_separator);
+
+  // Strip trailing separators.
+  while (path.size() > 1 && base::FilePath::IsSeparator(path[path.size() - 1]))
+    path.resize(path.size() - 1);
+
+  if (path.empty())
+    return base::FilePath(base::FilePath::kCurrentDirectory);
+
+  return base::FilePath(path);
+}
+
 void VirtualPath::GetComponents(
     const base::FilePath& path,
     std::vector<base::FilePath::StringType>* components) {
diff --git a/webkit/fileapi/file_system_util.h b/webkit/fileapi/file_system_util.h
index f77cbe0..89ee3c36 100644
--- a/webkit/fileapi/file_system_util.h
+++ b/webkit/fileapi/file_system_util.h
@@ -36,6 +36,10 @@ class WEBKIT_STORAGE_EXPORT VirtualPath {
   // character.
   static base::FilePath BaseName(const base::FilePath& virtual_path);
 
+  // Use this instead of base::FilePath::DirName when operating on virtual
+  // paths.
+  static base::FilePath DirName(const base::FilePath& virtual_path);
+
   // Likewise, use this instead of base::FilePath::GetComponents when
   // operating on virtual paths.
   // Note that this assumes very clean input, with no leading slash, and
diff --git a/webkit/fileapi/file_system_util_unittest.cc b/webkit/fileapi/file_system_util_unittest.cc
index 40849e5..04c0f1d 100644
--- a/webkit/fileapi/file_system_util_unittest.cc
+++ b/webkit/fileapi/file_system_util_unittest.cc
@@ -54,6 +54,46 @@ TEST_F(FileSystemUtilTest, VirtualPathBaseName) {
   }
 }
 
+TEST_F(FileSystemUtilTest, VirtualPathDirName) {
+  struct test_data {
+    const base::FilePath::StringType path;
+    const base::FilePath::StringType dir_name;
+  } test_cases[] = {
+    { FILE_PATH_LITERAL("foo/bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo/b:bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL(""), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("/"), FILE_PATH_LITERAL("/") },
+    { FILE_PATH_LITERAL("foo//////bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo/bar/"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo/bar/////"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("/bar/////"), FILE_PATH_LITERAL("/") },
+    { FILE_PATH_LITERAL("bar/////"), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("bar/"), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("/bar"), FILE_PATH_LITERAL("/") },
+    { FILE_PATH_LITERAL("////bar"), FILE_PATH_LITERAL("/") },
+    { FILE_PATH_LITERAL("bar"), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("c:bar"), FILE_PATH_LITERAL(".") },
+#ifdef FILE_PATH_USES_WIN_SEPARATORS
+    { FILE_PATH_LITERAL("foo\\bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo\\b:bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("\\"), FILE_PATH_LITERAL("\\") },
+    { FILE_PATH_LITERAL("foo\\\\\\\\\\\\bar"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo\\bar\\"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("foo\\bar\\\\\\\\\\"), FILE_PATH_LITERAL("foo") },
+    { FILE_PATH_LITERAL("\\bar\\\\\\\\\\"), FILE_PATH_LITERAL("\\") },
+    { FILE_PATH_LITERAL("bar\\\\\\\\\\"), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("bar\\"), FILE_PATH_LITERAL(".") },
+    { FILE_PATH_LITERAL("\\bar"), FILE_PATH_LITERAL("\\") },
+    { FILE_PATH_LITERAL("\\\\\\\\bar"), FILE_PATH_LITERAL("\\") },
+#endif
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    base::FilePath input = base::FilePath(test_cases[i].path);
+    base::FilePath dir_name = VirtualPath::DirName(input);
+    EXPECT_EQ(test_cases[i].dir_name, dir_name.value());
+  }
+}
+
 TEST_F(FileSystemUtilTest, GetNormalizedFilePath) {
   struct test_data {
     const base::FilePath::StringType path;
diff --git a/webkit/fileapi/obfuscated_file_util.cc b/webkit/fileapi/obfuscated_file_util.cc
index b2a8462..7031172 100644
--- a/webkit/fileapi/obfuscated_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_util.cc
@@ -274,7 +274,7 @@ PlatformFileError ObfuscatedFileUtil::CreateOrOpen(
         base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_OPEN_ALWAYS)))
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     FileId parent_id;
-    if (!db->GetFileWithPath(url.path().DirName(),
+    if (!db->GetFileWithPath(VirtualPath::DirName(url.path()),
                              &parent_id))
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     FileInfo file_info;
@@ -367,7 +367,7 @@ PlatformFileError ObfuscatedFileUtil::EnsureFileExists(
     return base::PLATFORM_FILE_OK;
   }
   FileId parent_id;
-  if (!db->GetFileWithPath(url.path().DirName(), &parent_id))
+  if (!db->GetFileWithPath(VirtualPath::DirName(url.path()), &parent_id))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   FileInfo file_info;
@@ -621,7 +621,7 @@ PlatformFileError ObfuscatedFileUtil::CopyOrMoveFile(
   }
   if (!overwrite) {
     FileId dest_parent_id;
-    if (!db->GetFileWithPath(dest_url.path().DirName(),
+    if (!db->GetFileWithPath(VirtualPath::DirName(dest_url.path()),
                              &dest_parent_id)) {
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     }
@@ -746,7 +746,7 @@ PlatformFileError ObfuscatedFileUtil::CopyInForeignFile(
   }
   if (!overwrite) {
     FileId dest_parent_id;
-    if (!db->GetFileWithPath(dest_url.path().DirName(),
+    if (!db->GetFileWithPath(VirtualPath::DirName(dest_url.path()),
                              &dest_parent_id)) {
       return base::PLATFORM_FILE_ERROR_NOT_FOUND;
     }
@@ -958,7 +958,7 @@ bool ObfuscatedFileUtil::DeleteDirectoryForOriginAndType(
       return false;
   }
 
-  base::FilePath origin_path = origin_type_path.DirName();
+  base::FilePath origin_path = VirtualPath::DirName(origin_type_path);
   DCHECK_EQ(origin_path.value(),
             GetDirectoryForOrigin(origin, false, NULL).value());
 
diff --git a/webkit/fileapi/obfuscated_file_util_unittest.cc b/webkit/fileapi/obfuscated_file_util_unittest.cc
index 8bb8e91..07e33c9 100644
--- a/webkit/fileapi/obfuscated_file_util_unittest.cc
+++ b/webkit/fileapi/obfuscated_file_util_unittest.cc
@@ -105,7 +105,7 @@ FileSystemURL FileSystemURLAppendUTF8(
 
 FileSystemURL FileSystemURLDirName(const FileSystemURL& url) {
   return FileSystemURL::CreateForTest(
-      url.origin(), url.mount_type(), url.virtual_path().DirName());
+      url.origin(), url.mount_type(), VirtualPath::DirName(url.virtual_path()));
 }
 
 }  // namespace (anonymous)