Add wxFileName::CopyAttributesFrom() and use it in wxTemp{F,}File

Copy the target attributes that are easy to copy and make sense to
preserve under Windows too instead of only doing it under Unix in
wxTemp{F,}File and do it in a new wxFileName::CopyAttributesFrom()
function instead of duplicating the same code in both classes.

The new function can be potentially useful on its own, so make it public
and document it.

Add a unit test verifying that the attributes are preserved after
overwriting the file with wxTemp{F,}File.

include/wx/filename.h

@@ -266,6 +266,9 @@ public:
         // values
     bool SetPermissions(int permissions);
 
+        // copy the supported attributes of the given file to this one
+    bool CopyAttributesFrom(const wxFileName& source) const;
+
     // Returns the native path for a file URL
     static wxFileName URLToFileName(const wxString& url);
 

interface/wx/filename.h

@@ -1405,6 +1405,37 @@ public:
     */
     bool SetPermissions(int permissions);
 
+    /**
+        Copies the attributes of the given file to this one.
+
+        This function is useful when replacing an existing file by writing the
+        new contents to a temporary file and then renaming it over the old
+        one, as it allows the replacement to keep the attributes of the file
+        being replaced, which would be lost otherwise.
+
+        Under Unix this copies the file permissions (including setuid and
+        setgid bits). Under MSW it copies the hidden, system and
+        not-content-indexed attributes and the creation time (access and
+        modification times will presumably be modified by the program anyhow
+        and so are not copied). Under other platforms this function currently
+        does nothing but still returns @true.
+
+        Note that this function does @e not copy everything and, in
+        particular, under MSW it doesn't copy the read-only attribute, as this
+        would prevent the destination file from being modified or removed
+        later. It also currently doesn't copy the compressed and encrypted
+        attributes nor the access control lists.
+
+        @param source
+            The file to copy the attributes from. It must exist.
+
+        @return @true if all supported attributes were copied, @false if any of
+            them couldn't be (some of them may still have been copied).
+
+        @since 3.3.4
+    */
+    bool CopyAttributesFrom(const wxFileName& source) const;
+
     /**
         Converts URL into a well-formed filename.
         The URL must use the @c file protocol.

src/common/ffile.cpp

@@ -326,27 +326,27 @@ bool wxTempFFile::Open(const wxString& strName)
         return false;
     }
 
-#ifdef __UNIX__
-    // the temp file should have the same permissions as the original one
-    mode_t mode;
-
-    wxStructStat st;
-    if ( wxStat(m_strName, &st) == 0 )
+    // The temp file should have the same attributes as the original one.
+    if ( wxFileExists(m_strName) )
     {
-        mode = st.st_mode;
+        if ( !wxFileName(m_strTemp).CopyAttributesFrom(m_strName) )
+        {
+            wxLogError(_("Error preserving all attributes of '%s'"), m_strName);
+        }
     }
+#ifdef __UNIX__
     else
     {
-        // file probably didn't exist, just give it the default mode _using_
+        // The file didn't exist, so just give it the default mode _using_
         // user's umask (new files creation should respect umask)
         mode_t mask = umask(0777);
-        mode = 0666 & ~mask;
+        mode_t mode = 0666 & ~mask;
         umask(mask);
-    }
 
-    if ( chmod( (const char*) m_strTemp.fn_str(), mode) == -1 )
-    {
-        wxLogSysError(_("Failed to set temporary file permissions"));
+        if ( chmod( (const char*) m_strTemp.fn_str(), mode) == -1 )
+        {
+            wxLogSysError(_("Failed to set temporary file permissions"));
+        }
     }
 #endif // Unix
 

src/common/file.cpp

@@ -546,27 +546,27 @@ bool wxTempFile::Open(const wxString& strName)
         return false;
     }
 
-#ifdef __UNIX__
-    // the temp file should have the same permissions as the original one
-    mode_t mode;
-
-    wxStructStat st;
-    if ( wxStat(m_strName, &st) == 0 )
+    // The temp file should have the same attributes as the original one.
+    if ( wxFileExists(m_strName) )
     {
-        mode = st.st_mode;
+        if ( !wxFileName(m_strTemp).CopyAttributesFrom(m_strName) )
+        {
+            wxLogError(_("Error preserving all attributes of '%s'"), m_strName);
+        }
     }
+#ifdef __UNIX__
     else
     {
-        // file probably didn't exist, just give it the default mode _using_
+        // The file didn't exist, so just give it the default mode _using_
         // user's umask (new files creation should respect umask)
         mode_t mask = umask(0777);
-        mode = 0666 & ~mask;
+        mode_t mode = 0666 & ~mask;
         umask(mask);
-    }
 
-    if ( chmod( (const char*) m_strTemp.fn_str(), mode) == -1 )
-    {
-        wxLogSysError(_("Failed to set temporary file permissions"));
+        if ( chmod( (const char*) m_strTemp.fn_str(), mode) == -1 )
+        {
+            wxLogSysError(_("Failed to set temporary file permissions"));
+        }
     }
 #endif // Unix
 

src/common/filename.cpp

@@ -2743,6 +2743,109 @@ bool wxFileName::SetPermissions(int permissions)
     return wxChmod(GetFullPath(), permissions) == 0;
 }
 
+#if defined(__WINDOWS__)
+
+namespace
+{
+
+bool CopyFileAttributes(const wxString& pathSrc, const wxString& pathDst)
+{
+    const DWORD attrsSrc = ::GetFileAttributes(pathSrc.t_str());
+    if ( attrsSrc == INVALID_FILE_ATTRIBUTES )
+    {
+        wxLogSysError(_("Failed to get attributes of '%s'"), pathSrc);
+        return false;
+    }
+
+    const DWORD attrsDst = ::GetFileAttributes(pathDst.t_str());
+    if ( attrsDst == INVALID_FILE_ATTRIBUTES )
+    {
+        wxLogSysError(_("Failed to get attributes of '%s'"), pathDst);
+        return false;
+    }
+
+    // Only these attributes can be usefully and successfully copied: notably
+    // FILE_ATTRIBUTE_READONLY is not copied because it would prevent the
+    // destination file from being written to or removed later, while the
+    // compressed and encrypted ones simply can't be set using this function.
+    constexpr DWORD ATTRS_TO_COPY = FILE_ATTRIBUTE_HIDDEN |
+                                    FILE_ATTRIBUTE_SYSTEM |
+                                    FILE_ATTRIBUTE_NOT_CONTENT_INDEXED;
+
+    // Note that this clears the attributes not set in the source file too.
+    const DWORD attrsNew = (attrsDst & ~ATTRS_TO_COPY) |
+                           (attrsSrc & ATTRS_TO_COPY);
+    if ( attrsNew != attrsDst &&
+            !::SetFileAttributes(pathDst.t_str(), attrsNew) )
+    {
+        wxLogSysError(_("Failed to set attributes of '%s'"), pathDst);
+        return false;
+    }
+
+    return true;
+}
+
+#if wxUSE_DATETIME
+bool CopyFileTimes(const wxFileName& src, const wxFileName& dst)
+{
+    // Preserve just the creation time, the others are going to be modified
+    // anyhow soon by the caller.
+    wxDateTime dtCreate;
+
+    return src.GetTimes(nullptr, nullptr, &dtCreate) &&
+                dst.SetTimes(nullptr, nullptr, &dtCreate);
+}
+#endif // wxUSE_DATETIME
+
+} // anonymous namespace
+
+#endif // __WINDOWS__
+
+bool wxFileName::CopyAttributesFrom(const wxFileName& source) const
+{
+    const wxString pathSrc = source.GetFullPath();
+    const wxString pathDst = GetFullPath();
+
+#if defined(__WINDOWS__)
+    bool ok = true;
+
+    if ( !CopyFileAttributes(pathSrc, pathDst) )
+        ok = false;
+
+#if wxUSE_DATETIME
+    if ( !CopyFileTimes(source, *this) )
+        ok = false;
+#endif // wxUSE_DATETIME
+
+    return ok;
+#elif defined(__UNIX_LIKE__)
+    // Under Unix we copy just the file mode: we can't set creation time.
+    wxStructStat st;
+    if ( wxStat(pathSrc, &st) != 0 )
+    {
+        wxLogSysError(_("Failed to get mode of '%s'"), pathSrc);
+        return false;
+    }
+
+    if ( wxChmod(pathDst, st.st_mode) != 0 )
+    {
+        wxLogSysError(_("Failed to set mode of '%s'"), pathDst);
+        return false;
+    }
+
+    return true;
+#else // other platform
+    wxUnusedVar(pathSrc);
+    wxUnusedVar(pathDst);
+
+    // Returning false from here would result in an error message being logged
+    // by the caller, which would be incomprehensible and there won't be
+    // anything that could possibly be done about it, so just pretend that we
+    // succeeded.
+    return true;
+#endif // platforms
+}
+
 // Returns the native path for a file URL
 wxFileName wxFileName::URLToFileName(const wxString& url)
 {

tests/file/filetest.cpp

@@ -17,6 +17,7 @@
 
 #include "wx/ffile.h"
 #include "wx/file.h"
+#include "wx/filename.h"
 
 #include "testfile.h"
 
@@ -160,6 +161,60 @@ TEMPLATE_TEST_CASE("wxTempFile", "[file][temp]", wxTempFile)
     CheckFileContents(name, dataNew);
 }
 
+// Check that replacing an existing file preserves its attributes: this is
+// what wxFileName::CopyAttributesFrom() is used for in wxTempFile::Open().
+#if wxUSE_FFILE
+TEMPLATE_TEST_CASE("wxTempFile::Attributes", "[file][temp]",
+                   wxTempFile, wxTempFFile)
+#else
+TEMPLATE_TEST_CASE("wxTempFile::Attributes", "[file][temp]", wxTempFile)
+#endif
+{
+    constexpr const char* name = "wxtemp_attr_test";
+
+    // Ensure that it will be removed at the end of the test in any case.
+    TempFile tf(name);
+
+    {
+        wxFile f(name, wxFile::write);
+        REQUIRE( f.IsOpened() );
+        CHECK( f.Write("old") );
+    }
+
+    wxFileName fn(name);
+
+    // Give the file to be replaced some distinctive attributes.
+#if wxUSE_DATETIME
+    const wxDateTime dtOld(1, wxDateTime::Jan, 2000);
+    REQUIRE( fn.SetTimes(nullptr, nullptr, &dtOld) );
+#endif // wxUSE_DATETIME
+
+#ifndef __WINDOWS__
+    REQUIRE( fn.SetPermissions(wxS_IRUSR | wxS_IWUSR) );
+#endif // !__WINDOWS__
+
+    TestType tmpFile;
+    REQUIRE( tmpFile.Open(name) );
+    CHECK( tmpFile.Write("new") );
+    CHECK( tmpFile.Commit() );
+
+    CheckFileContents(name, "new");
+
+#ifdef __WINDOWS__
+#if wxUSE_DATETIME
+    // Under MSW the creation time of the replaced file must be preserved.
+    wxDateTime dtCreate;
+    REQUIRE( fn.GetTimes(nullptr, nullptr, &dtCreate) );
+    CHECK( dtCreate == dtOld );
+#endif // wxUSE_DATETIME
+#else // !__WINDOWS__
+    // Elsewhere its permissions must be.
+    wxStructStat st;
+    REQUIRE( wxStat(name, &st) == 0 );
+    CHECK( (st.st_mode & 0777) == 0600 );
+#endif // __WINDOWS__/!__WINDOWS__
+}
+
 #ifdef __LINUX__
 
 // Check that GetSize() works correctly for special files.