Add tests to document base::CopyFile() behavior.

- Always overwrites the destination, if it is a file.
- Does not touch destination permissions when overwriting.
- On POSIX, preserves and follows destinations that are symlinks.
- On POSIX, file permissions for the destination is platform dependent.

Then document these in the header, since these details may be important
to the caller.

Change-Id: Ibd47fa95aac29a35d798e5c19d1da1a64df63754
Reviewed-on: https://chromium-review.googlesource.com/686001
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505836}

base/files/file_util.h

@@ -95,11 +95,26 @@ BASE_EXPORT bool ReplaceFile(const FilePath& from_path,
                              const FilePath& to_path,
                              File::Error* error);
 
-// Copies a single file. Use CopyDirectory to copy directories.
+// Copies a single file. Use CopyDirectory() to copy directories.
 // This function fails if either path contains traversal components ('..').
+// This function also fails if |to_path| is a directory.
 //
-// This function keeps the metadata on Windows. The read only bit on Windows is
-// not kept.
+// On POSIX, if |to_path| is a symlink, CopyFile() will follow the symlink. This
+// may have security implications. Use with care.
+//
+// If |to_path| already exists and is a regular file, it will be overwritten,
+// though its permissions will stay the same.
+//
+// If |to_path| does not exist, it will be created. The new file's permissions
+// varies per platform:
+//
+// - This function keeps the metadata on Windows. The read only bit is not kept.
+// - On Mac and iOS, |to_path| retains |from_path|'s permissions, except user
+//   read/write permissions are always set.
+// - On Linux and Android, |to_path| has user read/write permissions only. i.e.
+//   Always 0600.
+// - On ChromeOS, |to_path| has user read/write permissions and group/others
+//   read permissions. i.e. Always 0644.
 BASE_EXPORT bool CopyFile(const FilePath& from_path, const FilePath& to_path);
 
 // Copies the given path, and optionally all subdirectories and their contents

base/files/file_util_unittest.cc

@@ -633,8 +633,7 @@ TEST_F(FileUtilTest, CreateAndReadSymlinks) {
 
   // If we created the link properly, we should be able to read the contents
   // through it.
-  std::wstring contents = ReadTextFile(link_from);
-  EXPECT_EQ(bogus_content, contents);
+  EXPECT_EQ(bogus_content, ReadTextFile(link_from));
 
   FilePath result;
   ASSERT_TRUE(ReadSymbolicLink(link_from, &result));
@@ -738,6 +737,34 @@ TEST_F(FileUtilTest, DeleteSymlinkToNonExistentFile) {
   EXPECT_FALSE(IsLink(file_link));
 }
 
+TEST_F(FileUtilTest, CopyFileFollowsSymlinks) {
+  FilePath link_from = temp_dir_.GetPath().Append(FPL("from_file"));
+  FilePath link_to = temp_dir_.GetPath().Append(FPL("to_file"));
+  CreateTextFile(link_to, bogus_content);
+
+  ASSERT_TRUE(CreateSymbolicLink(link_to, link_from));
+
+  // If we created the link properly, we should be able to read the contents
+  // through it.
+  EXPECT_EQ(bogus_content, ReadTextFile(link_from));
+
+  FilePath result;
+  ASSERT_TRUE(ReadSymbolicLink(link_from, &result));
+  EXPECT_EQ(link_to.value(), result.value());
+
+  // Create another file and copy it to |link_from|.
+  FilePath src_file = temp_dir_.GetPath().Append(FPL("src.txt"));
+  const std::wstring file_contents(L"Gooooooooooooooooooooogle");
+  CreateTextFile(src_file, file_contents);
+  ASSERT_TRUE(CopyFile(src_file, link_from));
+
+  // Make sure |link_from| is still a symlink, and |link_to| has been written to
+  // by CopyFile().
+  EXPECT_TRUE(IsLink(link_from));
+  EXPECT_EQ(file_contents, ReadTextFile(link_from));
+  EXPECT_EQ(file_contents, ReadTextFile(link_to));
+}
+
 TEST_F(FileUtilTest, ChangeFilePermissionsAndRead) {
   // Create a file path.
   FilePath file_name =
@@ -903,7 +930,6 @@ TEST_F(FileUtilTest, ExecutableExistsInPath) {
   EXPECT_FALSE(ExecutableExistsInPath(scoped_env.GetEnv(), kDneFileName));
 }
 
-
 TEST_F(FileUtilTest, CopyDirectoryPermissions) {
   // Create a directory.
   FilePath dir_name_from =
@@ -1018,6 +1044,79 @@ TEST_F(FileUtilTest, CopyDirectoryPermissionsOverExistingFile) {
   EXPECT_EQ(0777, mode);
 }
 
+TEST_F(FileUtilTest, CopyFileExecutablePermission) {
+  FilePath src = temp_dir_.GetPath().Append(FPL("src.txt"));
+  const std::wstring file_contents(L"Gooooooooooooooooooooogle");
+  CreateTextFile(src, file_contents);
+
+  ASSERT_TRUE(SetPosixFilePermissions(src, 0755));
+  int mode = 0;
+  ASSERT_TRUE(GetPosixFilePermissions(src, &mode));
+  EXPECT_EQ(0755, mode);
+
+  FilePath dst = temp_dir_.GetPath().Append(FPL("dst.txt"));
+  ASSERT_TRUE(CopyFile(src, dst));
+  EXPECT_EQ(file_contents, ReadTextFile(dst));
+
+  ASSERT_TRUE(GetPosixFilePermissions(dst, &mode));
+  int expected_mode;
+#if defined(OS_MACOSX)
+  expected_mode = 0755;
+#elif defined(OS_CHROMEOS)
+  expected_mode = 0644;
+#else
+  expected_mode = 0600;
+#endif
+  EXPECT_EQ(expected_mode, mode);
+  ASSERT_TRUE(DeleteFile(dst, false));
+
+  ASSERT_TRUE(SetPosixFilePermissions(src, 0777));
+  ASSERT_TRUE(GetPosixFilePermissions(src, &mode));
+  EXPECT_EQ(0777, mode);
+
+  ASSERT_TRUE(CopyFile(src, dst));
+  EXPECT_EQ(file_contents, ReadTextFile(dst));
+
+  ASSERT_TRUE(GetPosixFilePermissions(dst, &mode));
+#if defined(OS_MACOSX)
+  expected_mode = 0755;
+#elif defined(OS_CHROMEOS)
+  expected_mode = 0644;
+#else
+  expected_mode = 0600;
+#endif
+  EXPECT_EQ(expected_mode, mode);
+  ASSERT_TRUE(DeleteFile(dst, false));
+
+  ASSERT_TRUE(SetPosixFilePermissions(src, 0400));
+  ASSERT_TRUE(GetPosixFilePermissions(src, &mode));
+  EXPECT_EQ(0400, mode);
+
+  ASSERT_TRUE(CopyFile(src, dst));
+  EXPECT_EQ(file_contents, ReadTextFile(dst));
+
+  ASSERT_TRUE(GetPosixFilePermissions(dst, &mode));
+#if defined(OS_MACOSX)
+  expected_mode = 0600;
+#elif defined(OS_CHROMEOS)
+  expected_mode = 0644;
+#else
+  expected_mode = 0600;
+#endif
+  EXPECT_EQ(expected_mode, mode);
+
+  // This time, do not delete |dst|. Instead set its permissions to 0777.
+  ASSERT_TRUE(SetPosixFilePermissions(dst, 0777));
+  ASSERT_TRUE(GetPosixFilePermissions(dst, &mode));
+  EXPECT_EQ(0777, mode);
+
+  // Overwrite it and check the permissions again.
+  ASSERT_TRUE(CopyFile(src, dst));
+  EXPECT_EQ(file_contents, ReadTextFile(dst));
+  ASSERT_TRUE(GetPosixFilePermissions(dst, &mode));
+  EXPECT_EQ(0777, mode);
+}
+
 #endif  // !defined(OS_FUCHSIA) && defined(OS_POSIX)
 
 #if !defined(OS_FUCHSIA)
@@ -1717,8 +1816,8 @@ TEST_F(FileUtilTest, CopyFile) {
   // Create a directory
   FilePath dir_name_from =
       temp_dir_.GetPath().Append(FILE_PATH_LITERAL("Copy_From_Subdir"));
-  CreateDirectory(dir_name_from);
-  ASSERT_TRUE(PathExists(dir_name_from));
+  ASSERT_TRUE(CreateDirectory(dir_name_from));
+  ASSERT_TRUE(DirectoryExists(dir_name_from));
 
   // Create a file under the directory
   FilePath file_name_from =
@@ -1744,10 +1843,31 @@ TEST_F(FileUtilTest, CopyFile) {
   // Check expected copy results.
   EXPECT_TRUE(PathExists(file_name_from));
   EXPECT_TRUE(PathExists(dest_file));
-  const std::wstring read_contents = ReadTextFile(dest_file);
-  EXPECT_EQ(file_contents, read_contents);
+  EXPECT_EQ(file_contents, ReadTextFile(dest_file));
   EXPECT_FALSE(PathExists(dest_file2_test));
   EXPECT_FALSE(PathExists(dest_file2));
+
+  // Change |file_name_from| contents.
+  const std::wstring new_file_contents(L"Moogle");
+  CreateTextFile(file_name_from, new_file_contents);
+  ASSERT_TRUE(PathExists(file_name_from));
+  EXPECT_EQ(new_file_contents, ReadTextFile(file_name_from));
+
+  // Overwrite |dest_file|.
+  ASSERT_TRUE(CopyFile(file_name_from, dest_file));
+  EXPECT_TRUE(PathExists(dest_file));
+  EXPECT_EQ(new_file_contents, ReadTextFile(dest_file));
+
+  // Create another directory.
+  FilePath dest_dir = temp_dir_.GetPath().Append(FPL("dest_dir"));
+  ASSERT_TRUE(CreateDirectory(dest_dir));
+  EXPECT_TRUE(DirectoryExists(dest_dir));
+  EXPECT_TRUE(IsDirectoryEmpty(dest_dir));
+
+  // Make sure CopyFile() cannot overwrite a directory.
+  ASSERT_FALSE(CopyFile(file_name_from, dest_dir));
+  EXPECT_TRUE(DirectoryExists(dest_dir));
+  EXPECT_TRUE(IsDirectoryEmpty(dest_dir));
 }
 
 // file_util winds up using autoreleased objects on the Mac, so this needs