From cdaf522ce72f975f924903192aa9116d314a72c4 Mon Sep 17 00:00:00 2001
From: James Johnston <johnstonj.public@codenest.com>
Date: Sun, 16 Aug 2015 19:47:11 -0400
Subject: [PATCH] SystemTools: Add honor_umask parameter to SetPermissions.
If honor_umask is set, the umask is queried and applied to the
given permissions when calling SetPermissions.
Change-Id: I302231a2ae7f0c4610eb47fdb256fa02934ecd8c
---
SystemTools.cxx | 26 ++++++--
SystemTools.hxx.in | 16 ++++-
testSystemTools.cxx | 140 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 175 insertions(+), 7 deletions(-)
diff --git a/SystemTools.cxx b/SystemTools.cxx
index 9b40e17e..f12a06c4 100644
--- a/SystemTools.cxx
+++ b/SystemTools.cxx
@@ -68,6 +68,10 @@
#include <sys/stat.h>
#include <time.h>
+#ifdef _MSC_VER
+# define umask _umask // Note this is still umask on Borland
+#endif
+
// support for realpath call
#ifndef _WIN32
#include <sys/time.h>
@@ -4780,21 +4784,35 @@ bool SystemTools::GetPermissions(const kwsys_stl::string& file, mode_t& mode)
return true;
}
-bool SystemTools::SetPermissions(const char* file, mode_t mode)
+bool SystemTools::SetPermissions(const char* file,
+ mode_t mode,
+ bool honor_umask)
{
if ( !file )
{
return false;
}
- return SystemTools::SetPermissions(kwsys_stl::string(file), mode);
+ return SystemTools::SetPermissions(
+ kwsys_stl::string(file), mode, honor_umask);
}
-bool SystemTools::SetPermissions(const kwsys_stl::string& file, mode_t mode)
+bool SystemTools::SetPermissions(const kwsys_stl::string& file,
+ mode_t mode,
+ bool honor_umask)
{
- if ( !SystemTools::FileExists(file) )
+ // TEMPORARY / TODO: After FileExists calls lstat() instead of
+ // access(), change this call to FileExists instead of
+ // TestFileAccess so that we don't follow symlinks.
+ if ( !SystemTools::TestFileAccess(file, TEST_FILE_OK) )
{
return false;
}
+ if (honor_umask)
+ {
+ mode_t currentMask = umask(0);
+ umask(currentMask);
+ mode &= ~currentMask;
+ }
#ifdef _WIN32
if ( _wchmod(SystemTools::ConvertToWindowsExtendedPath(file).c_str(),
mode) < 0 )
diff --git a/SystemTools.hxx.in b/SystemTools.hxx.in
index d181dc02..164c5e04 100644
--- a/SystemTools.hxx.in
+++ b/SystemTools.hxx.in
@@ -758,17 +758,27 @@ public:
*/
static long int CreationTime(const kwsys_stl::string& filename);
+ /**
+ * Visual C++ does not define mode_t (note that Borland does, however).
+ */
#if defined( _MSC_VER )
typedef unsigned short mode_t;
#endif
/**
- * Get and set permissions of the file.
+ * Get and set permissions of the file. If honor_umask is set, the umask
+ * is queried and applied to the given permissions. Returns false if
+ * failure.
+ *
+ * WARNING: A non-thread-safe method is currently used to get the umask
+ * if a honor_umask parameter is set to true.
*/
static bool GetPermissions(const char* file, mode_t& mode);
static bool GetPermissions(const kwsys_stl::string& file, mode_t& mode);
- static bool SetPermissions(const char* file, mode_t mode);
- static bool SetPermissions(const kwsys_stl::string& file, mode_t mode);
+ static bool SetPermissions(
+ const char* file, mode_t mode, bool honor_umask = false);
+ static bool SetPermissions(
+ const kwsys_stl::string& file, mode_t mode, bool honor_umask = false);
/** -----------------------------------------------------------------
* Time Manipulation Routines
diff --git a/testSystemTools.cxx b/testSystemTools.cxx
index ce8ec8b1..bc9ca5e5 100644
--- a/testSystemTools.cxx
+++ b/testSystemTools.cxx
@@ -30,6 +30,17 @@
#include <testSystemTools.h>
#include <string.h> /* strcmp */
+#if defined(_WIN32) && !defined(__CYGWIN__)
+# include <io.h> /* _umask (MSVC) / umask (Borland) */
+# ifdef _MSC_VER
+# define umask _umask // Note this is still umask on Borland
+# endif
+#endif
+#include <sys/stat.h> /* umask (POSIX), _S_I* constants (Windows) */
+// Visual C++ does not define mode_t (note that Borland does, however).
+#if defined( _MSC_VER )
+typedef unsigned short mode_t;
+#endif
//----------------------------------------------------------------------------
static const char* toUnixPaths[][2] =
@@ -170,6 +181,135 @@ static bool CheckFileOperations()
res = false;
}
+ // Reset umask
+#if defined(_WIN32) && !defined(__CYGWIN__)
+ // NOTE: Windows doesn't support toggling _S_IREAD.
+ mode_t fullMask = _S_IWRITE;
+#else
+ // On a normal POSIX platform, we can toggle all permissions.
+ mode_t fullMask = S_IRWXU | S_IRWXG | S_IRWXO;
+#endif
+ mode_t orig_umask = umask(fullMask);
+
+ // Test file permissions without umask
+ mode_t origPerm, thisPerm;
+ if (!kwsys::SystemTools::GetPermissions(testNewFile, origPerm))
+ {
+ kwsys_ios::cerr
+ << "Problem with GetPermissions (1) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if (!kwsys::SystemTools::SetPermissions(testNewFile, 0))
+ {
+ kwsys_ios::cerr
+ << "Problem with SetPermissions (1) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if (!kwsys::SystemTools::GetPermissions(testNewFile, thisPerm))
+ {
+ kwsys_ios::cerr
+ << "Problem with GetPermissions (2) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if ((thisPerm & fullMask) != 0)
+ {
+ kwsys_ios::cerr
+ << "SetPermissions failed to set permissions (1) for: "
+ << testNewFile << ": actual = " << thisPerm << "; expected = "
+ << 0 << kwsys_ios::endl;
+ res = false;
+ }
+
+ // While we're at it, check proper TestFileAccess functionality.
+ if (kwsys::SystemTools::TestFileAccess(testNewFile,
+ kwsys::TEST_FILE_WRITE))
+ {
+ kwsys_ios::cerr
+ << "TestFileAccess incorrectly indicated that this is a writable file:"
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if (!kwsys::SystemTools::TestFileAccess(testNewFile,
+ kwsys::TEST_FILE_OK))
+ {
+ kwsys_ios::cerr
+ << "TestFileAccess incorrectly indicated that this file does not exist:"
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ // Test restoring/setting full permissions.
+ if (!kwsys::SystemTools::SetPermissions(testNewFile, fullMask))
+ {
+ kwsys_ios::cerr
+ << "Problem with SetPermissions (2) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if (!kwsys::SystemTools::GetPermissions(testNewFile, thisPerm))
+ {
+ kwsys_ios::cerr
+ << "Problem with GetPermissions (3) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if ((thisPerm & fullMask) != fullMask)
+ {
+ kwsys_ios::cerr
+ << "SetPermissions failed to set permissions (2) for: "
+ << testNewFile << ": actual = " << thisPerm << "; expected = "
+ << fullMask << kwsys_ios::endl;
+ res = false;
+ }
+
+ // Test setting file permissions while honoring umask
+ if (!kwsys::SystemTools::SetPermissions(testNewFile, fullMask, true))
+ {
+ kwsys_ios::cerr
+ << "Problem with SetPermissions (3) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if (!kwsys::SystemTools::GetPermissions(testNewFile, thisPerm))
+ {
+ kwsys_ios::cerr
+ << "Problem with GetPermissions (4) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ if ((thisPerm & fullMask) != 0)
+ {
+ kwsys_ios::cerr
+ << "SetPermissions failed to honor umask for: "
+ << testNewFile << ": actual = " << thisPerm << "; expected = "
+ << 0 << kwsys_ios::endl;
+ res = false;
+ }
+
+ // Restore umask
+ umask(orig_umask);
+
+ // Restore file permissions
+ if (!kwsys::SystemTools::SetPermissions(testNewFile, origPerm))
+ {
+ kwsys_ios::cerr
+ << "Problem with SetPermissions (4) for: "
+ << testNewFile << kwsys_ios::endl;
+ res = false;
+ }
+
+ // Remove the test file
if (!kwsys::SystemTools::RemoveFile(testNewFile))
{
kwsys_ios::cerr
--
GitLab