File permission bug of C::B

Biplab June 23rd, 2008

A nasty bug was discovered by one of our user couple of months ago. The bug was affecting our last stable release as well as our latest nightly.

The bug was due to the way a file was being saved by C::B to avoid corrupting the existing file. C::B used to write the new contents to a temporary file before deleting the old one and then renaming the temp file to new file. But this created a bug as the temporary file may/may not get the file permission attributes of the original.

Following patch has been committed into latest svn trunk which fixes this issue.

--- /trunk/src/sdk/filemanager.cpp	2008/06/22 11:05:11	5103
+++ trunk/src/sdk/filemanager.cpp	2008/06/22 15:57:09	5104
@@ -58,7 +58,7 @@
     len = file.Length();
 
     data = new char[len+4];
-	char *dp = data + len;
+    char *dp = data + len;
     *dp++ = '\0';
     *dp++ = '\0';
     *dp++ = '\0';
@@ -100,7 +100,7 @@
 
     data = buffer.Data();
     len = buffer.Length();
-	buffer.Append("\0\0\0\0", 4);
+    buffer.Append("\0\0\0\0", 4);
     Ready();
 }
 
@@ -274,19 +274,40 @@
     wxString tempName(name + _T(".cbTemp"));
     do
     {
-        wxFile f(tempName, wxFile::write);
-        if(!f.IsOpened())
+        if( !wxCopyFile(name, tempName) )
+        {
             return false;
+        }
 
+        wxFile f(name, wxFile::write);
+        if ( !f.IsOpened() )
+        {
+            return false;
+        }
         if(f.Write(data, len) != len)
         {
             f.Close();
-            wxRemoveFile(tempName);
+            // Keep the backup file as the original file has been destroyed
+            //wxRemoveFile(tempName);
             return false;
         }
+
+        f.Close();
+
     }while(false);
 
-    return ReplaceFile(name, tempName);
+    if (Manager::IsAppShuttingDown())
+    {
+        // app shut down, forget delayed deletion
+        wxRemoveFile(tempName);
+    }
+    else
+    {
+        // issue a delayed deletion of the back'd up (old) file
+        delayedDeleteThread.Queue(new DelayedDelete(tempName));
+    }
+
+    return true;
 }
 
 bool FileManager::Save(const wxString& name, const wxString& data, wxFontEncoding encoding, bool bom)
@@ -309,19 +330,39 @@
     wxString tempName(name + _T(".cbTemp"));
     do
     {
-        wxFile f(tempName, wxFile::write);
-        if(!f.IsOpened())
+        if (!wxCopyFile(name, tempName))
+        {
             return false;
+        }
 
+        wxFile f(name, wxFile::write);
+        if ( !f.IsOpened() )
+        {
+            return false;
+        }
         if(WriteWxStringToFile(f, data, encoding, bom) == false)
         {
             f.Close();
-            wxRemoveFile(tempName);
+            // Keep the backup file as the original file has been destroyed
+            //wxRemoveFile(tempName);
             return false;
         }
+
+        f.Close();
+
     }while(false);
 
-    return ReplaceFile(name, tempName);
+    if (Manager::IsAppShuttingDown())
+    {
+        // app shut down, forget delayed deletion
+        wxRemoveFile(tempName);
+    }
+    else
+    {
+        // issue a delayed deletion of the back'd up (old) file
+        delayedDeleteThread.Queue(new DelayedDelete(tempName));
+    }
+    return true;
 }
 
 bool FileManager::ReplaceFile(const wxString& old_file, const wxString& new_file)


The patch is required for our last release (version 8.02).

Thanks for the bug report.

Trackback URI | Comments RSS

Leave a Reply