File permission bug of C::B

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++ = '';
     *dp++ = '';
     *dp++ = '';
@@ -100,7 +100,7 @@
 
     data = buffer.Data();
     len = buffer.Length();
-	buffer.Append("", 4);
+    buffer.Append("", 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.

Leave a Reply

Your email address will not be published. Required fields are marked *

*

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong> <pre lang="" line="" escaped="" highlight="">