Revert 215407 "Base: FileEnumerator should not follow reparse po..."
> Base: FileEnumerator should not follow reparse points.
>
> Following reparse points by default is dangerous because it can easily result
> in infinite loops.
>
> BUG=260360
> TEST=base_unittests
>
> Review URL: https://chromiumcodereview.appspot.com/20610004
TBR=rvargas@chromium.org
Review URL: https://codereview.chromium.org/21898008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215443 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 31af585e79a683d62ecf16338e8d78e8da4c31a6
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index 3434f15..e523920 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -116,36 +116,6 @@
}
return true;
}
-
-// Manages a reparse point for a test.
-class ReparsePoint {
- public:
- // Creates a reparse point from |source| (an empty directory) to |target|.
- ReparsePoint(const FilePath& source, const FilePath& target) {
- dir_.Set(
- ::CreateFile(source.value().c_str(),
- FILE_ALL_ACCESS,
- FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
- NULL,
- OPEN_EXISTING,
- FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory.
- NULL));
- created_ = dir_.IsValid() && SetReparsePoint(dir_, target);
- }
-
- ~ReparsePoint() {
- if (created_)
- DeleteReparsePoint(dir_);
- }
-
- bool IsValid() { return created_; }
-
- private:
- base::win::ScopedHandle dir_;
- bool created_;
- DISALLOW_COPY_AND_ASSIGN(ReparsePoint);
-};
-
#endif
#if defined(OS_POSIX)
@@ -461,61 +431,86 @@
FilePath to_sub_a = base_b.Append(FPL("to_sub_a"));
ASSERT_TRUE(file_util::CreateDirectory(to_sub_a));
+ base::win::ScopedHandle reparse_to_sub_a(
+ ::CreateFile(to_sub_a.value().c_str(),
+ FILE_ALL_ACCESS,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ NULL,
+ OPEN_EXISTING,
+ FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory.
+ NULL));
+ ASSERT_TRUE(reparse_to_sub_a.IsValid());
+ ASSERT_TRUE(SetReparsePoint(reparse_to_sub_a, sub_a));
+
+ FilePath to_base_b = base_b.Append(FPL("to_base_b"));
+ ASSERT_TRUE(file_util::CreateDirectory(to_base_b));
+ base::win::ScopedHandle reparse_to_base_b(
+ ::CreateFile(to_base_b.value().c_str(),
+ FILE_ALL_ACCESS,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ NULL,
+ OPEN_EXISTING,
+ FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory.
+ NULL));
+ ASSERT_TRUE(reparse_to_base_b.IsValid());
+ ASSERT_TRUE(SetReparsePoint(reparse_to_base_b, base_b));
+
+ FilePath to_sub_long = base_b.Append(FPL("to_sub_long"));
+ ASSERT_TRUE(file_util::CreateDirectory(to_sub_long));
+ base::win::ScopedHandle reparse_to_sub_long(
+ ::CreateFile(to_sub_long.value().c_str(),
+ FILE_ALL_ACCESS,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ NULL,
+ OPEN_EXISTING,
+ FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory.
+ NULL));
+ ASSERT_TRUE(reparse_to_sub_long.IsValid());
+ ASSERT_TRUE(SetReparsePoint(reparse_to_sub_long, sub_long));
+
+ // Normalize a junction free path: base_a\sub_a\file.txt .
FilePath normalized_path;
- {
- ReparsePoint reparse_to_sub_a(to_sub_a, sub_a);
- ASSERT_TRUE(reparse_to_sub_a.IsValid());
+ ASSERT_TRUE(file_util::NormalizeFilePath(file_txt, &normalized_path));
+ ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
- FilePath to_base_b = base_b.Append(FPL("to_base_b"));
- ASSERT_TRUE(file_util::CreateDirectory(to_base_b));
- ReparsePoint reparse_to_base_b(to_base_b, base_b);
- ASSERT_TRUE(reparse_to_base_b.IsValid());
+ // Check that the path base_b\to_sub_a\file.txt can be normalized to exclude
+ // the junction to_sub_a.
+ ASSERT_TRUE(file_util::NormalizeFilePath(to_sub_a.Append(FPL("file.txt")),
+ &normalized_path));
+ ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
- FilePath to_sub_long = base_b.Append(FPL("to_sub_long"));
- ASSERT_TRUE(file_util::CreateDirectory(to_sub_long));
- ReparsePoint reparse_to_sub_long(to_sub_long, sub_long);
- ASSERT_TRUE(reparse_to_sub_long.IsValid());
+ // Check that the path base_b\to_base_b\to_base_b\to_sub_a\file.txt can be
+ // normalized to exclude junctions to_base_b and to_sub_a .
+ ASSERT_TRUE(file_util::NormalizeFilePath(base_b.Append(FPL("to_base_b"))
+ .Append(FPL("to_base_b"))
+ .Append(FPL("to_sub_a"))
+ .Append(FPL("file.txt")),
+ &normalized_path));
+ ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
- // Normalize a junction free path: base_a\sub_a\file.txt .
- ASSERT_TRUE(file_util::NormalizeFilePath(file_txt, &normalized_path));
- ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
-
- // Check that the path base_b\to_sub_a\file.txt can be normalized to exclude
- // the junction to_sub_a.
- ASSERT_TRUE(file_util::NormalizeFilePath(to_sub_a.Append(FPL("file.txt")),
- &normalized_path));
- ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
-
- // Check that the path base_b\to_base_b\to_base_b\to_sub_a\file.txt can be
- // normalized to exclude junctions to_base_b and to_sub_a .
- ASSERT_TRUE(file_util::NormalizeFilePath(base_b.Append(FPL("to_base_b"))
- .Append(FPL("to_base_b"))
- .Append(FPL("to_sub_a"))
- .Append(FPL("file.txt")),
- &normalized_path));
- ASSERT_STREQ(file_txt.value().c_str(), normalized_path.value().c_str());
-
- // A long enough path will cause NormalizeFilePath() to fail. Make a long
- // path using to_base_b many times, and check that paths long enough to fail
- // do not cause a crash.
- FilePath long_path = base_b;
- const int kLengthLimit = MAX_PATH + 200;
- while (long_path.value().length() <= kLengthLimit) {
- long_path = long_path.Append(FPL("to_base_b"));
- }
- long_path = long_path.Append(FPL("to_sub_a"))
- .Append(FPL("file.txt"));
-
- ASSERT_FALSE(file_util::NormalizeFilePath(long_path, &normalized_path));
-
- // Normalizing the junction to deep.txt should fail, because the expanded
- // path to deep.txt is longer than MAX_PATH.
- ASSERT_FALSE(file_util::NormalizeFilePath(to_sub_long.Append(deep_txt),
- &normalized_path));
-
- // Delete the reparse points, and see that NormalizeFilePath() fails
- // to traverse them.
+ // A long enough path will cause NormalizeFilePath() to fail. Make a long
+ // path using to_base_b many times, and check that paths long enough to fail
+ // do not cause a crash.
+ FilePath long_path = base_b;
+ const int kLengthLimit = MAX_PATH + 200;
+ while (long_path.value().length() <= kLengthLimit) {
+ long_path = long_path.Append(FPL("to_base_b"));
}
+ long_path = long_path.Append(FPL("to_sub_a"))
+ .Append(FPL("file.txt"));
+
+ ASSERT_FALSE(file_util::NormalizeFilePath(long_path, &normalized_path));
+
+ // Normalizing the junction to deep.txt should fail, because the expanded
+ // path to deep.txt is longer than MAX_PATH.
+ ASSERT_FALSE(file_util::NormalizeFilePath(to_sub_long.Append(deep_txt),
+ &normalized_path));
+
+ // Delete the reparse points, and see that NormalizeFilePath() fails
+ // to traverse them.
+ ASSERT_TRUE(DeleteReparsePoint(reparse_to_sub_a));
+ ASSERT_TRUE(DeleteReparsePoint(reparse_to_base_b));
+ ASSERT_TRUE(DeleteReparsePoint(reparse_to_sub_long));
ASSERT_FALSE(file_util::NormalizeFilePath(to_sub_a.Append(FPL("file.txt")),
&normalized_path));
@@ -1826,35 +1821,36 @@
TEST_F(FileUtilTest, FileEnumeratorTest) {
// Test an empty directory.
FileEnumerator f0(temp_dir_.path(), true, FILES_AND_DIRECTORIES);
- EXPECT_EQ(f0.Next().value(), FPL(""));
- EXPECT_EQ(f0.Next().value(), FPL(""));
+ EXPECT_EQ(f0.Next().value(), FILE_PATH_LITERAL(""));
+ EXPECT_EQ(f0.Next().value(), FILE_PATH_LITERAL(""));
// Test an empty directory, non-recursively, including "..".
FileEnumerator f0_dotdot(temp_dir_.path(), false,
FILES_AND_DIRECTORIES | FileEnumerator::INCLUDE_DOT_DOT);
- EXPECT_EQ(temp_dir_.path().Append(FPL("..")).value(),
+ EXPECT_EQ(temp_dir_.path().Append(FILE_PATH_LITERAL("..")).value(),
f0_dotdot.Next().value());
- EXPECT_EQ(FPL(""), f0_dotdot.Next().value());
+ EXPECT_EQ(FILE_PATH_LITERAL(""),
+ f0_dotdot.Next().value());
// create the directories
- FilePath dir1 = temp_dir_.path().Append(FPL("dir1"));
+ FilePath dir1 = temp_dir_.path().Append(FILE_PATH_LITERAL("dir1"));
EXPECT_TRUE(file_util::CreateDirectory(dir1));
- FilePath dir2 = temp_dir_.path().Append(FPL("dir2"));
+ FilePath dir2 = temp_dir_.path().Append(FILE_PATH_LITERAL("dir2"));
EXPECT_TRUE(file_util::CreateDirectory(dir2));
- FilePath dir2inner = dir2.Append(FPL("inner"));
+ FilePath dir2inner = dir2.Append(FILE_PATH_LITERAL("inner"));
EXPECT_TRUE(file_util::CreateDirectory(dir2inner));
// create the files
- FilePath dir2file = dir2.Append(FPL("dir2file.txt"));
+ FilePath dir2file = dir2.Append(FILE_PATH_LITERAL("dir2file.txt"));
CreateTextFile(dir2file, std::wstring());
- FilePath dir2innerfile = dir2inner.Append(FPL("innerfile.txt"));
+ FilePath dir2innerfile = dir2inner.Append(FILE_PATH_LITERAL("innerfile.txt"));
CreateTextFile(dir2innerfile, std::wstring());
- FilePath file1 = temp_dir_.path().Append(FPL("file1.txt"));
+ FilePath file1 = temp_dir_.path().Append(FILE_PATH_LITERAL("file1.txt"));
CreateTextFile(file1, std::wstring());
FilePath file2_rel = dir2.Append(FilePath::kParentDirectory)
- .Append(FPL("file2.txt"));
+ .Append(FILE_PATH_LITERAL("file2.txt"));
CreateTextFile(file2_rel, std::wstring());
- FilePath file2_abs = temp_dir_.path().Append(FPL("file2.txt"));
+ FilePath file2_abs = temp_dir_.path().Append(FILE_PATH_LITERAL("file2.txt"));
// Only enumerate files.
FileEnumerator f1(temp_dir_.path(), true, FileEnumerator::FILES);
@@ -1888,7 +1884,8 @@
FindResultCollector c2_dotdot(f2_dotdot);
EXPECT_TRUE(c2_dotdot.HasFile(dir1));
EXPECT_TRUE(c2_dotdot.HasFile(dir2));
- EXPECT_TRUE(c2_dotdot.HasFile(temp_dir_.path().Append(FPL(".."))));
+ EXPECT_TRUE(c2_dotdot.HasFile(
+ temp_dir_.path().Append(FILE_PATH_LITERAL(".."))));
EXPECT_EQ(c2_dotdot.size(), 3);
// Enumerate files and directories.
@@ -1913,7 +1910,8 @@
EXPECT_EQ(c4.size(), 4);
// Enumerate with a pattern.
- FileEnumerator f5(temp_dir_.path(), true, FILES_AND_DIRECTORIES, FPL("dir*"));
+ FileEnumerator f5(temp_dir_.path(), true, FILES_AND_DIRECTORIES,
+ FILE_PATH_LITERAL("dir*"));
FindResultCollector c5(f5);
EXPECT_TRUE(c5.HasFile(dir1));
EXPECT_TRUE(c5.HasFile(dir2));
@@ -1922,48 +1920,10 @@
EXPECT_TRUE(c5.HasFile(dir2innerfile));
EXPECT_EQ(c5.size(), 5);
-#if defined(OS_WIN)
- {
- // Make dir1 point to dir2.
- ReparsePoint reparse_point(dir1, dir2);
- EXPECT_TRUE(reparse_point.IsValid());
-
- // Enumerate the reparse point.
- FileEnumerator f6(dir1, true, FILES_AND_DIRECTORIES);
- FindResultCollector c6(f6);
- FilePath inner2 = dir1.Append(FPL("inner"));
- EXPECT_TRUE(c6.HasFile(inner2));
- EXPECT_TRUE(c6.HasFile(inner2.Append(FPL("innerfile.txt"))));
- EXPECT_TRUE(c6.HasFile(dir1.Append(FPL("dir2file.txt"))));
- EXPECT_EQ(c6.size(), 3);
-
- // No changes for non recursive operation.
- FileEnumerator f7(temp_dir_.path(), false, FILES_AND_DIRECTORIES);
- FindResultCollector c7(f7);
- EXPECT_TRUE(c7.HasFile(dir2));
- EXPECT_TRUE(c7.HasFile(dir2));
- EXPECT_TRUE(c7.HasFile(file1));
- EXPECT_TRUE(c7.HasFile(file2_abs));
- EXPECT_EQ(c7.size(), 4);
-
- // Should not enumerate inside dir1 when using recursion.
- FileEnumerator f8(temp_dir_.path(), true, FILES_AND_DIRECTORIES);
- FindResultCollector c8(f8);
- EXPECT_TRUE(c8.HasFile(dir1));
- EXPECT_TRUE(c8.HasFile(dir2));
- EXPECT_TRUE(c8.HasFile(file1));
- EXPECT_TRUE(c8.HasFile(file2_abs));
- EXPECT_TRUE(c8.HasFile(dir2file));
- EXPECT_TRUE(c8.HasFile(dir2inner));
- EXPECT_TRUE(c8.HasFile(dir2innerfile));
- EXPECT_EQ(c8.size(), 7);
- }
-#endif
-
// Make sure the destructor closes the find handle while in the middle of a
// query to allow TearDown to delete the directory.
- FileEnumerator f9(temp_dir_.path(), true, FILES_AND_DIRECTORIES);
- EXPECT_FALSE(f9.Next().value().empty()); // Should have found something
+ FileEnumerator f6(temp_dir_.path(), true, FILES_AND_DIRECTORIES);
+ EXPECT_FALSE(f6.Next().value().empty()); // Should have found something
// (we don't care what).
}