diff options
| -rw-r--r-- | base/file_path.cc | 26 | ||||
| -rw-r--r-- | base/file_path.h | 5 | ||||
| -rw-r--r-- | base/file_path_unittest.cc | 44 | ||||
| -rw-r--r-- | base/file_util.cc | 27 | ||||
| -rw-r--r-- | base/file_util.h | 7 | ||||
| -rw-r--r-- | base/file_util_unittest.cc | 46 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_protocols.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_protocols_unittest.cc | 6 |
8 files changed, 81 insertions, 90 deletions
diff --git a/base/file_path.cc b/base/file_path.cc index 8738c37..3375b66 100644 --- a/base/file_path.cc +++ b/base/file_path.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "base/file_path.h" -#include "base/file_util.h" #include "base/logging.h" // These includes are just for the *Hack functions, and should be removed @@ -257,31 +256,6 @@ bool FilePath::IsAbsolute() const { return IsPathAbsolute(path_); } -bool FilePath::Contains(const FilePath &other) const { - FilePath parent = FilePath(*this); - FilePath child = FilePath(other); - - if (!file_util::AbsolutePath(&parent) || !file_util::AbsolutePath(&child)) - return false; - -#if defined(OS_WIN) - // file_util::AbsolutePath() does not flatten case on Windows, so we must do - // a case-insensitive compare. - if (!StartsWith(child.value(), parent.value(), false)) -#else - if (!StartsWithASCII(child.value(), parent.value(), true)) -#endif - return false; - - // file_util::AbsolutePath() normalizes '/' to '\' on Windows, so we only need - // to check kSeparators[0]. - if (child.value().length() <= parent.value().length() || - child.value()[parent.value().length()] != kSeparators[0]) - return false; - - return true; -} - #if defined(OS_POSIX) // See file_path.h for a discussion of the encoding of paths on POSIX // platforms. These *Hack() functions are not quite correct, but they're diff --git a/base/file_path.h b/base/file_path.h index d80298d..d82f8f4 100644 --- a/base/file_path.h +++ b/base/file_path.h @@ -195,11 +195,6 @@ class FilePath { // platforms, an absolute path begins with a separator character. bool IsAbsolute() const; - // Returns true if this FilePath represents a parent dir of |other|. Both - // paths are normalized before doing the comparison, but neither |this| nor - // |other| are modified. - bool Contains(const FilePath& other) const; - // Returns a copy of this FilePath that does not end with a trailing // separator. FilePath StripTrailingSeparators() const; diff --git a/base/file_path_unittest.cc b/base/file_path_unittest.cc index ac45e46..b237af6 100644 --- a/base/file_path_unittest.cc +++ b/base/file_path_unittest.cc @@ -364,49 +364,6 @@ TEST_F(FilePathTest, IsAbsolute) { } } -TEST_F(FilePathTest, Contains) { - FilePath data_dir; - ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &data_dir)); - data_dir = data_dir.Append(FILE_PATH_LITERAL("FilePathTest")); - - // Create a fresh, empty copy of this directory. - file_util::Delete(data_dir, true); - file_util::CreateDirectory(data_dir); - - FilePath foo(data_dir.Append(FILE_PATH_LITERAL("foo"))); - FilePath bar(foo.Append(FILE_PATH_LITERAL("bar.txt"))); - FilePath baz(data_dir.Append(FILE_PATH_LITERAL("baz.txt"))); - FilePath foobar(data_dir.Append(FILE_PATH_LITERAL("foobar.txt"))); - - // Annoyingly, the directories must actually exist in order for realpath(), - // which Contains() relies on in posix, to work. - file_util::CreateDirectory(foo); - std::string data("hello"); - file_util::WriteFile(bar.ToWStringHack(), data.c_str(), data.length()); - file_util::WriteFile(baz.ToWStringHack(), data.c_str(), data.length()); - file_util::WriteFile(foobar.ToWStringHack(), data.c_str(), data.length()); - - EXPECT_TRUE(foo.Contains(bar)); - EXPECT_FALSE(foo.Contains(baz)); - EXPECT_FALSE(foo.Contains(foobar)); - EXPECT_FALSE(foo.Contains(foo)); - -// Platform-specific concerns - FilePath foo_caps(data_dir.Append(FILE_PATH_LITERAL("FOO"))); -#if defined(OS_WIN) - EXPECT_TRUE(foo.Contains(foo_caps.Append(FILE_PATH_LITERAL("bar.txt")))); - EXPECT_TRUE(foo.Contains( - FilePath(foo.value() + FILE_PATH_LITERAL("/bar.txt")))); -#elif defined(OS_LINUX) - EXPECT_FALSE(foo.Contains(foo_caps.Append(FILE_PATH_LITERAL("bar.txt")))); -#else - // We can't really do this test on osx since the case-sensitivity of the - // filesystem is configurable. -#endif - - // Note: whether -} - TEST_F(FilePathTest, Extension) { FilePath base_dir(FILE_PATH_LITERAL("base_dir")); @@ -564,4 +521,3 @@ TEST_F(FilePathTest, ReplaceExtension) { ", path: " << path.value() << ", replace: " << cases[i].inputs[1]; } } - diff --git a/base/file_util.cc b/base/file_util.cc index fe7b694..078e3afa 100644 --- a/base/file_util.cc +++ b/base/file_util.cc @@ -276,6 +276,33 @@ bool CloseFile(FILE* file) { return fclose(file) == 0; } +bool ContainsPath(const FilePath &parent, const FilePath& child) { + FilePath abs_parent = FilePath(parent); + FilePath abs_child = FilePath(child); + + if (!file_util::AbsolutePath(&abs_parent) || + !file_util::AbsolutePath(&abs_child)) + return false; + +#if defined(OS_WIN) + // file_util::AbsolutePath() does not flatten case on Windows, so we must do + // a case-insensitive compare. + if (!StartsWith(abs_child.value(), abs_parent.value(), false)) +#else + if (!StartsWithASCII(abs_child.value(), abs_parent.value(), true)) +#endif + return false; + + // file_util::AbsolutePath() normalizes '/' to '\' on Windows, so we only need + // to check kSeparators[0]. + if (abs_child.value().length() <= abs_parent.value().length() || + abs_child.value()[abs_parent.value().length()] != + FilePath::kSeparators[0]) + return false; + + return true; +} + /////////////////////////////////////////////// // MemoryMappedFile diff --git a/base/file_util.h b/base/file_util.h index c064b2c..4154856 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -103,10 +103,9 @@ bool AbsolutePath(FilePath* path); // Deprecated temporary compatibility function. bool AbsolutePath(std::wstring* path); -// Returns true if this FilePath represents a parent dir of |other|. Both -// paths are normalized before doing the comparison, but neither |this| nor -// |other| are modified. -bool PathContains(const FilePath& path, const FilePath& other); +// Returns true if |parent| contains |child|. Both paths are converted to +// absolute paths before doing the comparison. +bool ContainsPath(const FilePath& parent, const FilePath& child); // Deprecated compatibility function. Use FilePath::InsertBeforeExtension. void InsertBeforeExtension(FilePath* path, const FilePath::StringType& suffix); diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index a8855e9..8f5ce19 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -978,4 +978,50 @@ TEST_F(FileUtilTest, PathComponentsTest) { } } +TEST_F(FileUtilTest, Contains) { + FilePath data_dir; + ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &data_dir)); + data_dir = data_dir.Append(FILE_PATH_LITERAL("FilePathTest")); + + // Create a fresh, empty copy of this directory. + ASSERT_TRUE(file_util::Delete(data_dir, true)); + ASSERT_TRUE(file_util::CreateDirectory(data_dir)); + + FilePath foo(data_dir.Append(FILE_PATH_LITERAL("foo"))); + FilePath bar(foo.Append(FILE_PATH_LITERAL("bar.txt"))); + FilePath baz(data_dir.Append(FILE_PATH_LITERAL("baz.txt"))); + FilePath foobar(data_dir.Append(FILE_PATH_LITERAL("foobar.txt"))); + + // Annoyingly, the directories must actually exist in order for realpath(), + // which Contains() relies on in posix, to work. + ASSERT_TRUE(file_util::CreateDirectory(foo)); + std::string data("hello"); + ASSERT_TRUE(file_util::WriteFile(bar.ToWStringHack(), data.c_str(), + data.length())); + ASSERT_TRUE(file_util::WriteFile(baz.ToWStringHack(), data.c_str(), + data.length())); + ASSERT_TRUE(file_util::WriteFile(foobar.ToWStringHack(), data.c_str(), + data.length())); + + EXPECT_TRUE(file_util::ContainsPath(foo, bar)); + EXPECT_FALSE(file_util::ContainsPath(foo, baz)); + EXPECT_FALSE(file_util::ContainsPath(foo, foobar)); + EXPECT_FALSE(file_util::ContainsPath(foo, foo)); + +// Platform-specific concerns + FilePath foo_caps(data_dir.Append(FILE_PATH_LITERAL("FOO"))); +#if defined(OS_WIN) + EXPECT_TRUE(file_util::ContainsPath(foo, + foo_caps.Append(FILE_PATH_LITERAL("bar.txt")))); + EXPECT_TRUE(file_util::ContainsPath(foo, + FilePath(foo.value() + FILE_PATH_LITERAL("/bar.txt")))); +#elif defined(OS_LINUX) + EXPECT_FALSE(file_util::ContainsPath(foo, + foo_caps.Append(FILE_PATH_LITERAL("bar.txt")))); +#else + // We can't really do this test on osx since the case-sensitivity of the + // filesystem is configurable. +#endif +} + } // namespace diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc index d09cdd1..8f6957d 100644 --- a/chrome/browser/extensions/extension_protocols.cc +++ b/chrome/browser/extensions/extension_protocols.cc @@ -40,12 +40,12 @@ FilePath GetPathForExtensionResource(const FilePath& extension_path, if (!net::FileURLToFilePath(file_url, &ret_val)) return FilePath(); - if (!file_util::AbsolutePath(&ret_val)) - return FilePath(); - // Double-check that the path we ended up with is actually inside the - // extension root. - if (!extension_path.Contains(ret_val)) + // extension root. We can do this with a simple prefix match because: + // a) We control the prefix on both sides, and they should match. + // b) GURL normalizes things like "../" and "//" before it gets to us. + if (ret_val.value().find(extension_path.value() + + FilePath::kSeparators[0]) != 0) return FilePath(); return ret_val; diff --git a/chrome/browser/extensions/extension_protocols_unittest.cc b/chrome/browser/extensions/extension_protocols_unittest.cc index 113b480..dee70a8 100644 --- a/chrome/browser/extensions/extension_protocols_unittest.cc +++ b/chrome/browser/extensions/extension_protocols_unittest.cc @@ -15,12 +15,8 @@ TEST(ExtensionProtocolsTest, GetPathForExtensionResource) { GetPathForExtensionResource(extension_path, "/foo/bar.gif").value()); EXPECT_EQ(std::wstring(L"C:\\myextension\\"), GetPathForExtensionResource(extension_path, "/").value()); - // TODO(aa): This one is a bit weird, but is what net::FileURLToFilePath() - // returns for this input. Investigate adding more validation. EXPECT_EQ(std::wstring(L"C:\\myextension\\c:\\foo.gif"), GetPathForExtensionResource(extension_path, "/c:/foo.gif").value()); - EXPECT_EQ(std::wstring(L"C:\\myextension\\foo.gif"), - GetPathForExtensionResource(extension_path, "//foo.gif").value()); EXPECT_EQ(std::wstring(L""), GetPathForExtensionResource(extension_path, "/../foo.gif").value()); #else @@ -29,8 +25,6 @@ TEST(ExtensionProtocolsTest, GetPathForExtensionResource) { GetPathForExtensionResource(extension_path, "/foo/bar.gif").value()); EXPECT_EQ(std::wstring("/myextension/"), GetPathForExtensionResource(extension_path, "/").value()); - EXPECT_EQ(std::wstring("/myextension/foo.gif"), - GetPathForExtensionResource(extension_path, "//foo.gif").value()); EXPECT_EQ(std::wstring(""), GetPathForExtensionResource(extension_path, "/../foo.gif").value()); #endif |