g_file_copy(): Clean up logic for info query

Previously, we called g_file_query_info() *again* on the source at the
very end of the copy.  This has the lame semantics that if the source
happened to be deleted, we would fail to apply attributes to the
destination.  This could even be a security flaw.

This commit changes things so that we query info from the source
*stream* after opening - i.e. on Unix we use the proper fstat() and
friends.  That way we operate more atomically.

https://bugzilla.gnome.org/show_bug.cgi?id=699959

diff --git a/gio/gfile.c b/gio/gfile.c
index 961d40f..57d3bc3 100644 (file)
--- a/gio/gfile.c
+++ b/gio/gfile.c
@@ -2510,7 +2510,7 @@ copy_symlink (GFile           *destination,
   return TRUE;
 }
 
-static GInputStream *
+static GFileInputStream *
 open_source_for_copy (GFile           *source,
                       GFile           *destination,
                       GFileCopyFlags   flags,
@@ -2518,14 +2518,14 @@ open_source_for_copy (GFile           *source,
                       GError         **error)
 {
   GError *my_error;
-  GInputStream *in;
+  GFileInputStream *ret;
   GFileInfo *info;
   GFileType file_type;
 
   my_error = NULL;
-  in = (GInputStream *)g_file_read (source, cancellable, &my_error);
-  if (in != NULL)
-    return in;
+  ret = g_file_read (source, cancellable, &my_error);
+  if (ret != NULL)
+    return ret;
 
   /* There was an error opening the source, try to set a good error for it: */
   if (my_error->domain == G_IO_ERROR && my_error->code == G_IO_ERROR_IS_DIRECTORY)
@@ -2588,26 +2588,48 @@ open_source_for_copy (GFile           *source,
 
 static gboolean
 should_copy (GFileAttributeInfo *info,
-             gboolean            as_move,
+             gboolean            copy_all_attributes,
              gboolean            skip_perms)
 {
   if (skip_perms && strcmp(info->name, "unix::mode") == 0)
         return FALSE;
 
-  if (as_move)
+  if (copy_all_attributes)
     return info->flags & G_FILE_ATTRIBUTE_INFO_COPY_WHEN_MOVED;
   return info->flags & G_FILE_ATTRIBUTE_INFO_COPY_WITH_FILE;
 }
 
-static char *
-build_attribute_list_for_copy (GFileAttributeInfoList *attributes,
-                               GFileAttributeInfoList *namespaces,
-                               gboolean                as_move,
-                               gboolean                skip_perms)
+static gboolean
+build_attribute_list_for_copy (GFile                  *file,
+                               GFileCopyFlags          flags,
+                               char                  **out_attributes,
+                               GCancellable           *cancellable,
+                               GError                **error)
 {
+  gboolean ret = FALSE;
+  GFileAttributeInfoList *attributes = NULL, *namespaces = NULL;
   GString *s;
   gboolean first;
   int i;
+  gboolean copy_all_attributes;
+  gboolean skip_perms;
+
+  copy_all_attributes = flags & G_FILE_COPY_ALL_METADATA;
+  skip_perms = (flags & G_FILE_COPY_TARGET_DEFAULT_PERMS) != 0;
+
+  /* Ignore errors here, if the target supports no attributes there is
+   * nothing to copy.  We still honor the cancellable though.
+   */
+  attributes = g_file_query_settable_attributes (file, cancellable, NULL);
+  if (g_cancellable_set_error_if_cancelled (cancellable, error))
+    goto out;
+
+  namespaces = g_file_query_writable_namespaces (file, cancellable, NULL);
+  if (g_cancellable_set_error_if_cancelled (cancellable, error))
+    goto out;
+
+  if (attributes == NULL && namespaces == NULL)
+    goto out;
 
   first = TRUE;
   s = g_string_new ("");
@@ -2616,7 +2638,7 @@ build_attribute_list_for_copy (GFileAttributeInfoList *attributes,
     {
       for (i = 0; i < attributes->n_infos; i++)
         {
-          if (should_copy (&attributes->infos[i], as_move, skip_perms))
+          if (should_copy (&attributes->infos[i], copy_all_attributes, skip_perms))
             {
               if (first)
                 first = FALSE;
@@ -2632,7 +2654,7 @@ build_attribute_list_for_copy (GFileAttributeInfoList *attributes,
     {
       for (i = 0; i < namespaces->n_infos; i++)
         {
-          if (should_copy (&namespaces->infos[i], as_move, FALSE))
+          if (should_copy (&namespaces->infos[i], copy_all_attributes, FALSE))
             {
               if (first)
                 first = FALSE;
@@ -2645,7 +2667,18 @@ build_attribute_list_for_copy (GFileAttributeInfoList *attributes,
         }
     }
 
-  return g_string_free (s, FALSE);
+  ret = TRUE;
+  *out_attributes = g_string_free (s, FALSE);
+  s = NULL;
+ out:
+  if (s)
+    g_string_free (s, TRUE);
+  if (attributes)
+    g_file_attribute_info_list_unref (attributes);
+  if (namespaces)
+    g_file_attribute_info_list_unref (namespaces);
+  
+  return ret;
 }
 
 /**
@@ -2676,28 +2709,16 @@ g_file_copy_attributes (GFile           *source,
                         GCancellable    *cancellable,
                         GError         **error)
 {
-  GFileAttributeInfoList *attributes, *namespaces;
   char *attrs_to_read;
   gboolean res;
   GFileInfo *info;
-  gboolean as_move;
   gboolean source_nofollow_symlinks;
-  gboolean skip_perms;
-
-  as_move = flags & G_FILE_COPY_ALL_METADATA;
-  source_nofollow_symlinks = flags & G_FILE_COPY_NOFOLLOW_SYMLINKS;
-  skip_perms = (flags & G_FILE_COPY_TARGET_DEFAULT_PERMS) != 0;
-
-  /* Ignore errors here, if the target supports no attributes there is
-   * nothing to copy
-   */
-  attributes = g_file_query_settable_attributes (destination, cancellable, NULL);
-  namespaces = g_file_query_writable_namespaces (destination, cancellable, NULL);
 
-  if (attributes == NULL && namespaces == NULL)
-    return TRUE;
+  if (!build_attribute_list_for_copy (destination, flags, &attrs_to_read,
+                                      cancellable, error))
+    return FALSE;
 
-  attrs_to_read = build_attribute_list_for_copy (attributes, namespaces, as_move, skip_perms);
+  source_nofollow_symlinks = flags & G_FILE_COPY_NOFOLLOW_SYMLINKS;
 
   /* Ignore errors here, if we can't read some info (e.g. if it doesn't exist)
    * we just don't copy it.
@@ -2720,9 +2741,6 @@ g_file_copy_attributes (GFile           *source,
       g_object_unref (info);
     }
 
-  g_file_attribute_info_list_unref (attributes);
-  g_file_attribute_info_list_unref (namespaces);
-
   return res;
 }
 
@@ -3012,10 +3030,13 @@ file_copy_fallback (GFile                  *source,
                     GError                **error)
 {
   gboolean ret = FALSE;
+  GFileInputStream *file_in = NULL;
   GInputStream *in = NULL;
   GOutputStream *out = NULL;
   GFileInfo *info = NULL;
   const char *target;
+  char *attrs_to_read;
+  gboolean do_set_attributes = FALSE;
 
   /* need to know the file type */
   info = g_file_query_info (source,
@@ -3052,10 +3073,32 @@ file_copy_fallback (GFile                  *source,
 
   /* Everything else should just fall back on a regular copy. */
 
-  in = open_source_for_copy (source, destination, flags, cancellable, error);
-  if (!in)
+  file_in = open_source_for_copy (source, destination, flags, cancellable, error);
+  if (!file_in)
+    goto out;
+  in = G_INPUT_STREAM (file_in);
+
+  if (!build_attribute_list_for_copy (destination, flags, &attrs_to_read,
+                                      cancellable, error))
     goto out;
 
+  if (attrs_to_read != NULL)
+    {
+      /* Ok, ditch the previous lightweight info (on Unix we just
+       * called lstat()); at this point we gather all the information
+       * we need about the source from the opened file descriptor.
+       */
+      g_object_unref (info);
+
+      info = g_file_input_stream_query_info (file_in, attrs_to_read,
+                                             cancellable, error);
+      g_free (attrs_to_read);
+      if (!info)
+        goto out;
+
+      do_set_attributes = TRUE;
+    }
+
   if (flags & G_FILE_COPY_OVERWRITE)
     {
       out = (GOutputStream *)g_file_replace (destination,
@@ -3151,10 +3194,15 @@ file_copy_fallback (GFile                  *source,
     }
 
   /* Ignore errors here. Failure to copy metadata is not a hard error */
-  if (ret)
-    (void) g_file_copy_attributes (source, destination,
-                                   flags, cancellable, NULL);
-
+  /* TODO: set these attributes /before/ we do the rename() on Unix */
+  if (ret && do_set_attributes)
+    {
+      g_file_set_attributes_from_info (destination,
+                                       info,
+                                       G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
+                                       cancellable,
+                                       error);
+    }
 
   g_clear_object (&info);