Add the methods to change and get a posix file permission to file_util.

base/file_util_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #include <shlobj.h>
@@ skipping 93 matching lines @@
   data.ReparseTag = 0xa0000003;
   if (!DeviceIoControl(source, FSCTL_DELETE_REPARSE_POINT, &data, 8, NULL, 0,
                        &returned, NULL)) {
     return false;
   }
   return true;
 }
 #endif
 
 #if defined(OS_POSIX)
-// Provide a simple way to change the permissions bits on |path| in tests.
-// ASSERT failures will return, but not stop the test.  Caller should wrap
-// calls to this function in ASSERT_NO_FATAL_FAILURE().
-void ChangePosixFilePermissions(const FilePath& path,
-                                mode_t mode_bits_to_set,
-                                mode_t mode_bits_to_clear) {
-  ASSERT_FALSE(mode_bits_to_set & mode_bits_to_clear)
+bool ChangePosixFilePermissions(const FilePath& path,
+                                int mode_bits_to_set,
+                                int mode_bits_to_clear) {
+  DCHECK((mode_bits_to_set & ~file_util::FILE_PERMISSION_MASK) == 0);
+  DCHECK((mode_bits_to_clear & ~file_util::FILE_PERMISSION_MASK) == 0);

[satorux1, 2012/07/10 00:06:54]

the two lines look unnecessary, as we have a DCHECK in SetPosixFilePermissions()
anway

[yoshiki, 2012/07/10 01:31:17]

Done.

+  DCHECK(!(mode_bits_to_set & mode_bits_to_clear))
       << "Can't set and clear the same bits.";
 
-  struct stat stat_buf;
-  ASSERT_EQ(0, stat(path.value().c_str(), &stat_buf));
+  int mode;

[satorux1, 2012/07/10 00:06:54]

mode = 0; just in case.

[yoshiki, 2012/07/10 01:31:17]

Done.

+  if (!file_util::GetPosixFilePermissions(path, &mode))
+    return false;
 
-  mode_t updated_mode_bits = stat_buf.st_mode;
-  updated_mode_bits |= mode_bits_to_set;
-  updated_mode_bits &= ~mode_bits_to_clear;
+  mode |= mode_bits_to_set;
+  mode &= ~mode_bits_to_clear;
 
-  ASSERT_EQ(0, chmod(path.value().c_str(), updated_mode_bits));
+  if (file_util::SetPosixFilePermissions(path, mode))

[satorux1, 2012/07/10 00:06:54]

Is this correct? Shouldn't we have ! here?

if (!file_util::SetPosixFilePermissions(path, mode))

[yoshiki, 2012/07/10 01:31:17]

Oops... Done.

On 2012/07/10 00:06:54, satorux1 wrote:

+    return false;
+
+  return true;
 }
 #endif  // defined(OS_POSIX)
 
 const wchar_t bogus_content[] = L"I'm cannon fodder.";
 
 const file_util::FileEnumerator::FileType FILES_AND_DIRECTORIES =
     static_cast<file_util::FileEnumerator::FileType>(
         file_util::FileEnumerator::FILES |
         file_util::FileEnumerator::DIRECTORIES);
 
@@ skipping 519 matching lines @@
   ASSERT_TRUE(file_util::CreateSymbolicLink(link_to, link_from))
     << "Failed to create directory symlink.";
 
   // Test failures.
   ASSERT_FALSE(file_util::CreateSymbolicLink(link_to, link_to));
   ASSERT_FALSE(file_util::ReadSymbolicLink(link_to, &result));
   FilePath missing = temp_dir_.path().Append(FPL("missing"));
   ASSERT_FALSE(file_util::ReadSymbolicLink(missing, &result));
 }
 
-
 // The following test of NormalizeFilePath() require that we create a symlink.
 // This can not be done on Windows before Vista.  On Vista, creating a symlink
 // requires privilege "SeCreateSymbolicLinkPrivilege".
 // TODO(skerner): Investigate the possibility of giving base_unittests the
 // privileges required to create a symlink.
 TEST_F(FileUtilTest, NormalizeFilePathSymlinks) {
   FilePath normalized_path;
 
   // Link one file to another.
   FilePath link_from = temp_dir_.path().Append(FPL("from_file"));
@@ skipping 96 matching lines @@
   // Make sure the symbolic link is exist
   EXPECT_TRUE(file_util::IsLink(file_link));
   EXPECT_FALSE(file_util::PathExists(file_link));
 
   // Delete the symbolic link
   EXPECT_TRUE(file_util::Delete(file_link, false));
 
   // Make sure the symbolic link is deleted
   EXPECT_FALSE(file_util::IsLink(file_link));
 }
+
+TEST_F(FileUtilTest, ChangeFilePermissionsAndRead) {
+  // Create a file path

[satorux1, 2012/07/10 00:06:54]

period is missing.

[yoshiki, 2012/07/10 01:31:17]

Done.

+  FilePath file_name = temp_dir_.path().Append(FPL("Test Readable File.txt"));
+  EXPECT_FALSE(file_util::PathExists(file_name));
+
+  char buffer[32] = "hello";

[satorux1, 2012/07/10 00:06:54]

remove 32. even better, remove this variable.

[yoshiki, 2012/07/10 01:31:17]

Removed 32. But not remove the variable because it is used later.

+  std::string data(buffer);

[satorux1, 2012/07/10 00:06:54]

const std::string kData = "hello";

[yoshiki, 2012/07/10 01:31:17]

Made it const and changed name, keeping the initialization from |buffer| since
it is not removed.

On 2012/07/10 00:06:54, satorux1 wrote:

[satorux1, 2012/07/10 06:57:17]

Reusing 'buffer' at a later time for writing sounds a bit unclean. Maybe:

const std::string kData("hello");
...
std::string buffer(kData.length());
EXPECT_EQ(static_cast<int>(kData.length()),
          file_util::ReadFile(file_name, string_as_array(buffer),
buffer.size());

However it's not a big deal so it's up to you.

[yoshiki, 2012/07/10 18:51:35]

I think direct manipulation to buffer of std::string is not common. I change the
code to alloc char array buffer dynamically.

On 2012/07/10 06:57:17, satorux1 wrote:

+
+  // Write file
+  EXPECT_EQ(static_cast<int>(data.length()),
+            file_util::WriteFile(file_name, data.c_str(), data.length()));
+  EXPECT_TRUE(file_util::PathExists(file_name));
+
+  // Meke sure the file is readable
+  int32 mode;

[satorux1, 2012/07/10 00:06:54]

int mode = 0;

[yoshiki, 2012/07/10 01:31:17]

Done.

+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_READ_BY_USER,
+            mode & file_util::FILE_PERMISSION_READ_BY_USER);

[satorux1, 2012/07/10 00:06:54]

EXPECT_TRUE(mode & file_util::FILE_PERMISSION_READ_BY_USER)

[yoshiki, 2012/07/10 01:31:17]

Done.

+
+  // Get rid of the read permission
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(file_name, 0u));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(0, mode & file_util::FILE_PERMISSION_READ_BY_USER);

[satorux1, 2012/07/10 00:06:54]

EXPECT_FALSE(mode & file_util::FILE_PERMISSION_READ_BY_USER)

[yoshiki, 2012/07/10 01:31:17]

Done.

+  // Make sure the file can't be read
+  EXPECT_EQ(-1, file_util::ReadFile(file_name, buffer, sizeof(buffer)));

[satorux1, 2012/07/10 00:06:54]

kData.data(), kData.size()

[yoshiki, 2012/07/10 01:31:17]

string::data() returns a pointer of const char*, so it is unsafe to write to the
pointer. 

On 2012/07/10 00:06:54, satorux1 wrote:

[satorux1, 2012/07/10 06:57:17]

oops, you are right.

On 2012/07/10 01:31:17, yoshiki wrote:

+
+  // Give the read permission
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(
+      file_name,
+      file_util::FILE_PERMISSION_READ_BY_USER));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_READ_BY_USER,
+            mode & file_util::FILE_PERMISSION_READ_BY_USER);
+  // Make sure the file can be read
+  EXPECT_EQ(static_cast<int>(data.length()),
+            file_util::ReadFile(file_name, buffer, sizeof(buffer)));
+
+  // Delete the file
+  EXPECT_TRUE(file_util::Delete(file_name, false));
+  EXPECT_FALSE(file_util::PathExists(file_name));
+}
+
+TEST_F(FileUtilTest, ChangeFilePermissionsAndWrite) {
+  // Create a file path
+  FilePath file_name = temp_dir_.path().Append(FPL("Test Readable File.txt"));
+  EXPECT_FALSE(file_util::PathExists(file_name));
+
+  char buffer[32] = "hello";
+  std::string data(buffer);
+
+  // Write file
+  EXPECT_EQ(static_cast<int>(data.length()),
+            file_util::WriteFile(file_name, data.c_str(), data.length()));
+  EXPECT_TRUE(file_util::PathExists(file_name));
+
+  // Meke sure the file is writable
+  int mode;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_WRITE_BY_USER,
+            mode & file_util::FILE_PERMISSION_WRITE_BY_USER);
+  EXPECT_TRUE(file_util::PathIsWritable(file_name));
+
+  // Get rid of the write permission
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(file_name, 0u));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(0, mode & file_util::FILE_PERMISSION_WRITE_BY_USER);
+  // Make sure the file can't be write
+  EXPECT_EQ(-1,
+            file_util::WriteFile(file_name, data.c_str(), data.length()));
+  EXPECT_FALSE(file_util::PathIsWritable(file_name));
+
+  // Give read permission
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(
+      file_name,
+      file_util::FILE_PERMISSION_WRITE_BY_USER));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(file_name, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_WRITE_BY_USER,
+            mode & file_util::FILE_PERMISSION_WRITE_BY_USER);
+  // Make sure the file can be write
+  EXPECT_EQ(static_cast<int>(data.length()),
+            file_util::WriteFile(file_name, data.c_str(), data.length()));
+  EXPECT_TRUE(file_util::PathIsWritable(file_name));
+
+  // Delete the file
+  EXPECT_TRUE(file_util::Delete(file_name, false));
+  EXPECT_FALSE(file_util::PathExists(file_name));
+}
+
+TEST_F(FileUtilTest, ChangeDirectoryPermissionsAndEnumerate) {
+  // Create a directory path
+  FilePath subdir_path =
+      temp_dir_.path().Append(FPL("PermissionTest1"));
+  file_util::CreateDirectory(subdir_path);
+  ASSERT_TRUE(file_util::PathExists(subdir_path));
+
+  // Create a dummy file to enumerate
+  FilePath file_name = subdir_path.Append(FPL("Test Readable File.txt"));
+  EXPECT_FALSE(file_util::PathExists(file_name));
+  char buffer[32] = "hello";
+  std::string data(buffer);
+  EXPECT_EQ(static_cast<int>(data.length()),
+            file_util::WriteFile(file_name, data.c_str(), data.length()));
+  EXPECT_TRUE(file_util::PathExists(file_name));
+
+  // Meke sure the file is the all permissions
+  int mode;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(subdir_path, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_USER_MASK,
+            mode & file_util::FILE_PERMISSION_USER_MASK);
+
+  // Get rid of the permissions from the directory
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(subdir_path, 0u));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(subdir_path, &mode));
+  EXPECT_EQ(0, mode & file_util::FILE_PERMISSION_USER_MASK);
+
+  // Make sure the file in the directory can't be enumerated.
+  file_util::FileEnumerator f1(subdir_path, true,
+                               file_util::FileEnumerator::FILES);
+  EXPECT_TRUE(file_util::PathExists(subdir_path));
+  FindResultCollector c1(f1);
+  EXPECT_EQ(c1.size(), 0);
+  EXPECT_FALSE(file_util::GetPosixFilePermissions(file_name, &mode));
+
+  // Give the permissions to the directory
+  EXPECT_TRUE(file_util::SetPosixFilePermissions(
+      subdir_path,
+      file_util::FILE_PERMISSION_USER_MASK));
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(subdir_path, &mode));
+  EXPECT_EQ(file_util::FILE_PERMISSION_USER_MASK,
+            mode & file_util::FILE_PERMISSION_USER_MASK);
+
+  // Make sure the file in the directory can be enumerated.
+  file_util::FileEnumerator f2(subdir_path, true,
+                               file_util::FileEnumerator::FILES);
+  FindResultCollector c2(f2);
+  EXPECT_TRUE(c2.HasFile(file_name));
+  EXPECT_EQ(c2.size(), 1);
+
+  // Delete the file
+  EXPECT_TRUE(file_util::Delete(subdir_path, true));
+  EXPECT_FALSE(file_util::PathExists(subdir_path));
+}
+
 #endif  // defined(OS_POSIX)
 
 #if defined(OS_WIN)
 // Tests that the Delete function works for wild cards, especially
 // with the recursion flag.  Also coincidentally tests PathExists.
 // TODO(erikkay): see if anyone's actually using this feature of the API
 TEST_F(FileUtilTest, DeleteWildCard) {
   // Create a file and a directory
   FilePath file_name = temp_dir_.path().Append(FPL("Test DeleteWildCard.txt"));
   CreateTextFile(file_name, bogus_content);
@@ skipping 1193 matching lines @@
     uid_ = stat_buf.st_uid;
     ok_gids_.insert(stat_buf.st_gid);
     bad_gids_.insert(stat_buf.st_gid + 1);
 
     ASSERT_EQ(uid_, getuid());  // This process should be the owner.
 
     // To ensure that umask settings do not cause the initial state
     // of permissions to be different from what we expect, explicitly
     // set permissions on the directories we create.
     // Make all files and directories non-world-writable.
-    mode_t enabled_permissions =
-        S_IRWXU |  // User can read, write, traverse
-        S_IRWXG;   // Group can read, write, traverse
-    mode_t disabled_permissions =
-        S_IRWXO;   // Other users can't read, write, traverse.
+
+    // Users and group can read, write, traverse
+    int enabled_permissions =
+        file_util::FILE_PERMISSION_USER_MASK |
+        file_util::FILE_PERMISSION_GROUP_MASK;
+    // Other users can't read, write, traverse
+    int disabled_permissions =
+        file_util::FILE_PERMISSION_OTHERS_MASK;
 
     ASSERT_NO_FATAL_FAILURE(
         ChangePosixFilePermissions(
             base_dir_, enabled_permissions, disabled_permissions));
     ASSERT_NO_FATAL_FAILURE(
         ChangePosixFilePermissions(
             sub_dir_, enabled_permissions, disabled_permissions));
   }
 
   FilePath base_dir_;
@@ skipping 303 matching lines @@
       file_util::VerifyPathControlledByUser(
           base_dir_, text_file_, uid_, ok_gids_));
   EXPECT_TRUE(
       file_util::VerifyPathControlledByUser(
           sub_dir_, text_file_, uid_, ok_gids_));
 }
 
 #endif  // defined(OS_POSIX)
 
 }  // namespace