Bug 560564 – Replacing a symlink with its linked file truncates the

2009-02-18  Alexander Larsson  <alexl@redhat.com>

	Bug 560564 – Replacing a symlink with its linked file truncates the original file

        * gioenums.h:
	Add G_FILE_CREATE_REPLACE_DESTINATION

        * glocalfileoutputstream.c:
        (handle_overwrite_open):
        (_g_local_file_output_stream_replace):
	Handle G_FILE_CREATE_REPLACE_DESTINATION when overwriting files.

        * gfile.c:
        (file_copy_fallback):
	Pass G_FILE_CREATE_REPLACE_DESTINATION to g_file_replace when copying
	with overwrite.


svn path=/trunk/; revision=7880

4 files changed, 91 insertions, 23 deletions

diff --git a/gio/ChangeLog b/gio/ChangeLog
index 9725a7b34..affd6af30 100644
--- a/gio/ChangeLog
+++ b/gio/ChangeLog
@@ -1,3 +1,20 @@
+2009-02-18  Alexander Larsson  <alexl@redhat.com>
+
+	Bug 560564 – Replacing a symlink with its linked file truncates the original file
+
+        * gioenums.h:
+	Add G_FILE_CREATE_REPLACE_DESTINATION
+
+        * glocalfileoutputstream.c:
+        (handle_overwrite_open):
+        (_g_local_file_output_stream_replace):
+	Handle G_FILE_CREATE_REPLACE_DESTINATION when overwriting files.
+
+        * gfile.c:
+        (file_copy_fallback):
+	Pass G_FILE_CREATE_REPLACE_DESTINATION to g_file_replace when copying
+	with overwrite.
+
 2009-02-17  Ryan Lortie  <desrt@desrt.ca>
 
 	* gfileinfo.c: unref the destination's attribute matcher before
diff --git a/gio/gfile.c b/gio/gfile.c
index 6ae42090c..cc4768131 100644
--- a/gio/gfile.c
+++ b/gio/gfile.c
@@ -2342,7 +2342,7 @@ file_copy_fallback (GFile                  *source,
       out = (GOutputStream *)g_file_replace (destination,
 					     NULL,
 					     flags & G_FILE_COPY_BACKUP,
-                                             0,
+                                             G_FILE_CREATE_REPLACE_DESTINATION,
 					     cancellable, error);
     }
   else
diff --git a/gio/gioenums.h b/gio/gioenums.h
index 2b69ccf56..c29b4820d 100644
--- a/gio/gioenums.h
+++ b/gio/gioenums.h
@@ -156,12 +156,21 @@ typedef enum {
  * @G_FILE_CREATE_NONE: No flags set.
  * @G_FILE_CREATE_PRIVATE: Create a file that can only be
  *    accessed by the current user.
+ * @G_FILE_CREATE_REPLACE_DESTINATION: Replace the destination
+ *    as if it didn't exist before. Don't try to keep any old
+ *    permissions, replace instead of following links. This
+ *    is generally useful if you're doing a "copy over" 
+ *    rather than a "save new version of" replace operation.
+ *    You can think of it as "unlink destination" before
+ *    writing to it, although the implementation may not
+ *    be exactly like that.
  *
  * Flags used when an operation may create a file.
  */
 typedef enum {
   G_FILE_CREATE_NONE    = 0,
-  G_FILE_CREATE_PRIVATE = (1 << 0)
+  G_FILE_CREATE_PRIVATE = (1 << 0),
+  G_FILE_CREATE_REPLACE_DESTINATION = (1 << 1)
 } GFileCreateFlags;
 
 
diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
index 2d5ff3aa0..8ca328428 100644
--- a/gio/glocalfileoutputstream.c
+++ b/gio/glocalfileoutputstream.c
@@ -644,6 +644,7 @@ handle_overwrite_open (const char    *filename,
 		       const char    *etag,
 		       gboolean       create_backup,
 		       char         **temp_filename,
+		       GFileCreateFlags flags,
 		       GCancellable  *cancellable,
 		       GError       **error)
 {
@@ -653,6 +654,12 @@ handle_overwrite_open (const char    *filename,
   gboolean is_symlink;
   int open_flags;
   int res;
+  int mode;
+
+  if (flags & G_FILE_CREATE_PRIVATE)
+    mode = 0600;
+  else
+    mode = 0666;
 
   /* We only need read access to the original file if we are creating a backup.
    * We also add O_CREATE to avoid a race if the file was just removed */
@@ -665,16 +672,16 @@ handle_overwrite_open (const char    *filename,
    * when finding out if the file we opened was a symlink */
 #ifdef O_NOFOLLOW
   is_symlink = FALSE;
-  fd = g_open (filename, open_flags | O_NOFOLLOW, 0666);
+  fd = g_open (filename, open_flags | O_NOFOLLOW, mode);
   if (fd == -1 && errno == ELOOP)
     {
       /* Could be a symlink, or it could be a regular ELOOP error,
        * but then the next open will fail too. */
       is_symlink = TRUE;
-      fd = g_open (filename, open_flags, 0666);
+      fd = g_open (filename, open_flags, mode);
     }
 #else
-  fd = g_open (filename, open_flags, 0666);
+  fd = g_open (filename, open_flags, mode);
   /* This is racy, but we do it as soon as possible to minimize the race */
   is_symlink = g_file_test (filename, G_FILE_TEST_IS_SYMLINK);
 #endif
@@ -751,7 +758,8 @@ handle_overwrite_open (const char    *filename,
    * to a backup file and rewrite the contents of the file.
    */
   
-  if (!(original_stat.st_nlink > 1) && !is_symlink)
+  if ((flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
+      (!(original_stat.st_nlink > 1) && !is_symlink))
     {
       char *dirname, *tmp_filename;
       int tmpfd;
@@ -767,16 +775,18 @@ handle_overwrite_open (const char    *filename,
 	  goto fallback_strategy;
 	}
       
-      /* try to keep permissions */
+      /* try to keep permissions (unless replacing) */
 
-      if (
+      if ( ! (flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
+	   (
 #ifdef HAVE_FCHOWN
-	  fchown (tmpfd, original_stat.st_uid, original_stat.st_gid) == -1 ||
+	    fchown (tmpfd, original_stat.st_uid, original_stat.st_gid) == -1 ||
 #endif
 #ifdef HAVE_FCHMOD
-	  fchmod (tmpfd, original_stat.st_mode) == -1 ||
+	    fchmod (tmpfd, original_stat.st_mode) == -1 ||
 #endif
-	  0
+	    0
+	    )
 	  )
 	{
 	  struct stat tmp_statbuf;
@@ -899,26 +909,58 @@ handle_overwrite_open (const char    *filename,
 	}
     }
 
-  /* Truncate the file at the start */
+  if (flags & G_FILE_CREATE_REPLACE_DESTINATION)
+    {
+      close (fd);
+      
+      if (g_unlink (filename) != 0)
+	{
+	  int errsv = errno;
+	  
+	  g_set_error (error, G_IO_ERROR,
+		       g_io_error_from_errno (errsv),
+		       _("Error removing old file: %s"),
+		       g_strerror (errsv));
+	  goto err_out2;
+	}
+      
+      fd = g_open (filename, O_WRONLY | O_CREAT | O_BINARY, mode);
+      if (fd == -1)
+	{
+	  int errsv = errno;
+	  char *display_name = g_filename_display_name (filename);
+	  g_set_error (error, G_IO_ERROR,
+		       g_io_error_from_errno (errsv),
+		       _("Error opening file '%s': %s"),
+		       display_name, g_strerror (errsv));
+	  g_free (display_name);
+	  goto err_out2;
+	}
+    }
+  else
+    {
+      /* Truncate the file at the start */
 #ifdef G_OS_WIN32
-  if (g_win32_ftruncate (fd, 0) == -1)
+      if (g_win32_ftruncate (fd, 0) == -1)
 #else
-  if (ftruncate (fd, 0) == -1)
+	if (ftruncate (fd, 0) == -1)
 #endif
-    {
-      int errsv = errno;
-
-      g_set_error (error, G_IO_ERROR,
-		   g_io_error_from_errno (errsv),
-		   _("Error truncating file: %s"),
-		   g_strerror (errsv));
-      goto err_out;
+	  {
+	    int errsv = errno;
+	    
+	    g_set_error (error, G_IO_ERROR,
+			 g_io_error_from_errno (errsv),
+			 _("Error truncating file: %s"),
+			 g_strerror (errsv));
+	    goto err_out;
+	  }
     }
     
   return fd;
 
  err_out:
   close (fd);
+ err_out2:
   return -1;
 }
 
@@ -952,7 +994,7 @@ _g_local_file_output_stream_replace (const char        *filename,
     {
       /* The file already exists */
       fd = handle_overwrite_open (filename, etag, create_backup, &temp_file,
-				  cancellable, error);
+				  flags, cancellable, error);
       if (fd == -1)
 	return NULL;
     }