[glib: 1/3] glocalfileoutputstream: Fix ETag check when replacing through a symlink

[Emmanuele Bassi <ebassi src gnome org>]

commit b1ebb7252277f13acc3854b0eb972d2514599e5c
Author: Philip Withnall <pwithnall endlessos org>
Date:   Mon Jun 7 12:42:16 2021 +0100

    glocalfileoutputstream: Fix ETag check when replacing through a symlink
    
    Since commit 87e19535fe, the ETag check when writing out a file through
    a symlink (following the symlink) has been incorrectly using the ETag
    value of the symlink, rather than the target file. This is incorrect
    because the ETag should represent the file content, not its metadata or
    links to it.
    
    Fix that, and add a unit test.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Fixes: #2417

 gio/glocalfileoutputstream.c |  31 ++++++++++-
 gio/tests/file.c             | 120 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 1 deletion(-)
---
diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
index 78d3e85a6..71a992668 100644
--- a/gio/glocalfileoutputstream.c
+++ b/gio/glocalfileoutputstream.c
@@ -975,7 +975,36 @@ handle_overwrite_open (const char    *filename,
   
   if (etag != NULL)
     {
-      current_etag = _g_local_file_info_create_etag (&original_stat);
+      GLocalFileStat etag_stat;
+      GLocalFileStat *etag_stat_pointer;
+
+      /* The ETag is calculated on the details of the target file, for symlinks,
+       * so we might need to stat() again. */
+      if (is_symlink)
+        {
+          res = g_local_file_stat (filename,
+                                   G_LOCAL_FILE_STAT_FIELD_MTIME,
+                                   G_LOCAL_FILE_STAT_FIELD_ALL, &etag_stat);
+          errsv = errno;
+
+          if (res != 0)
+            {
+              char *display_name = g_filename_display_name (filename);
+              g_set_error (error, G_IO_ERROR,
+                           g_io_error_from_errno (errsv),
+                           _("Error when getting information for file “%s”: %s"),
+                           display_name, g_strerror (errsv));
+              g_free (display_name);
+              goto error;
+            }
+
+          etag_stat_pointer = &etag_stat;
+        }
+      else
+        etag_stat_pointer = &original_stat;
+
+      /* Compare the ETags */
+      current_etag = _g_local_file_info_create_etag (etag_stat_pointer);
       if (strcmp (etag, current_etag) != 0)
        {
          g_set_error_literal (error,
diff --git a/gio/tests/file.c b/gio/tests/file.c
index 7e99601fa..b2e1f3d46 100644
--- a/gio/tests/file.c
+++ b/gio/tests/file.c
@@ -932,6 +932,125 @@ test_replace_symlink (void)
 #endif
 }
 
+static void
+test_replace_symlink_using_etag (void)
+{
+#ifdef G_OS_UNIX
+  gchar *tmpdir_path = NULL;
+  GFile *tmpdir = NULL, *source_file = NULL, *target_file = NULL;
+  GFileOutputStream *stream = NULL;
+  const gchar *old_contents = "this is a test message which should be written to target and then 
overwritten";
+  gchar *old_etag = NULL;
+  const gchar *new_contents = "this is an updated message";
+  gsize n_written;
+  gchar *contents = NULL;
+  gsize length = 0;
+  GFileInfo *info = NULL;
+  GError *local_error = NULL;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2417";);
+  g_test_summary ("Test that ETag checks work when replacing a file through a symlink");
+
+  /* Create a fresh, empty working directory. */
+  tmpdir_path = g_dir_make_tmp ("g_file_replace_symlink_using_etag_XXXXXX", &local_error);
+  g_assert_no_error (local_error);
+  tmpdir = g_file_new_for_path (tmpdir_path);
+
+  g_test_message ("Using temporary directory %s", tmpdir_path);
+  g_free (tmpdir_path);
+
+  /* Create symlink `source` which points to `target`. */
+  source_file = g_file_get_child (tmpdir, "source");
+  target_file = g_file_get_child (tmpdir, "target");
+  g_file_make_symbolic_link (source_file, "target", NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  /* Sleep for at least 1s to ensure the mtimes of `source` and `target` differ,
+   * as that’s what _g_local_file_info_create_etag() uses to create the ETag,
+   * and one failure mode we’re testing for is that the ETags of `source` and
+   * `target` are conflated. */
+  sleep (1);
+
+  /* Create `target` with some arbitrary content. */
+  stream = g_file_create (target_file, G_FILE_CREATE_NONE, NULL, &local_error);
+  g_assert_no_error (local_error);
+  g_output_stream_write_all (G_OUTPUT_STREAM (stream), old_contents, strlen (old_contents),
+                             &n_written, NULL, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpint (n_written, ==, strlen (old_contents));
+
+  g_output_stream_close (G_OUTPUT_STREAM (stream), NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  old_etag = g_file_output_stream_get_etag (stream);
+  g_assert_nonnull (old_etag);
+  g_assert_cmpstr (old_etag, !=, "");
+
+  g_clear_object (&stream);
+
+  /* Sleep again to ensure the ETag changes again. */
+  sleep (1);
+
+  /* Write out a new copy of the `target`, checking its ETag first. This should
+   * replace `target` by following the symlink. */
+  stream = g_file_replace (source_file, old_etag, FALSE  /* no backup */,
+                           G_FILE_CREATE_NONE, NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  g_output_stream_write_all (G_OUTPUT_STREAM (stream), new_contents, strlen (new_contents),
+                             &n_written, NULL, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpint (n_written, ==, strlen (new_contents));
+
+  g_output_stream_close (G_OUTPUT_STREAM (stream), NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  g_clear_object (&stream);
+
+  /* At this point, there should be a regular file, `target`, containing
+   * @new_contents; and a symlink `source` which points to `target`. */
+  g_assert_cmpint (g_file_query_file_type (source_file, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, NULL), ==, 
G_FILE_TYPE_SYMBOLIC_LINK);
+  g_assert_cmpint (g_file_query_file_type (target_file, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, NULL), ==, 
G_FILE_TYPE_REGULAR);
+
+  /* Check the content of `target`. */
+  g_file_load_contents (target_file,
+                        NULL,
+                        &contents,
+                        &length,
+                        NULL,
+                        &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpstr (contents, ==, new_contents);
+  g_assert_cmpuint (length, ==, strlen (new_contents));
+  g_free (contents);
+
+  /* And check its ETag value has changed. */
+  info = g_file_query_info (target_file, G_FILE_ATTRIBUTE_ETAG_VALUE,
+                            G_FILE_QUERY_INFO_NONE, NULL, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpstr (g_file_info_get_etag (info), !=, old_etag);
+
+  g_clear_object (&info);
+  g_free (old_etag);
+
+  /* Tidy up. */
+  g_file_delete (target_file, NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  g_file_delete (source_file, NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  g_file_delete (tmpdir, NULL, &local_error);
+  g_assert_no_error (local_error);
+
+  g_clear_object (&target_file);
+  g_clear_object (&source_file);
+  g_clear_object (&tmpdir);
+#else  /* if !G_OS_UNIX */
+  g_test_skip ("Symlink replacement tests can only be run on Unix")
+#endif
+}
+
 /* FIXME: These tests have only been checked on Linux. Most of them are probably
  * applicable on Windows, too, but that has not been tested yet.
  * See https://gitlab.gnome.org/GNOME/glib/-/issues/2325 */
@@ -2906,6 +3025,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/file/replace-load", test_replace_load);
   g_test_add_func ("/file/replace-cancel", test_replace_cancel);
   g_test_add_func ("/file/replace-symlink", test_replace_symlink);
+  g_test_add_func ("/file/replace-symlink/using-etag", test_replace_symlink_using_etag);
   g_test_add_data_func ("/file/replace/write-only", GUINT_TO_POINTER (FALSE), test_replace);
   g_test_add_data_func ("/file/replace/read-write", GUINT_TO_POINTER (TRUE), test_replace);
   g_test_add_func ("/file/async-delete", test_async_delete);