[gnome-software: 1/4] gs-download-utils: Don’t use ETag when writing local file

[Philip Withnall <pwithnall src gnome org>]

commit edbeab87ce5aa48bd9fe49dd22acfcce2b8b0fc0
Author: Philip Withnall <pwithnall endlessos org>
Date:   Thu Mar 10 13:20:58 2022 +0000

    gs-download-utils: Don’t use ETag when writing local file
    
    The ETag we care about for the download is the ETag returned by the
    server, not the ETag of the local file. Passing the server’s ETag to the
    local file operations may result in the local file not being written to,
    with a “The file was externally modified” error.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Fixes: #1677

 lib/gs-download-utils.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)
---
diff --git a/lib/gs-download-utils.c b/lib/gs-download-utils.c
index f3a64479b..a6d930587 100644
--- a/lib/gs-download-utils.c
+++ b/lib/gs-download-utils.c
@@ -134,6 +134,9 @@ static void download_progress (GTask *task);
  * existing content of the output stream (if it’s a file, for example) will not
  * be overwritten.
  *
+ * Note that @last_etag must be the ETag value returned by the server last time
+ * the file was downloaded, not the local file ETag generated by GLib.
+ *
  * If specified, @progress_callback will be called zero or more times until
  * @callback is called, providing progress updates on the download.
  *
@@ -644,9 +647,20 @@ gs_download_file_async (SoupSession                *soup_session,
        /* Query the old ETag if the file already exists. */
        data->last_etag = gs_utils_get_file_etag (output_file, cancellable);
 
-       /* Create the output file. */
+       /* Create the output file.
+        *
+        * Note that `data->last_etag` is *not* passed in here, as the ETag from
+        * the server and the file modification ETag that GLib uses are
+        * different things. For g_file_replace_async(), GLib always uses an
+        * ETag it generates internally based on the file mtime (see
+        * _g_local_file_info_create_etag()), which will never match what the
+        * server returns in its ETag header.
+        *
+        * This is fine, as we are using the ETag to avoid an unnecessary HTTP
+        * download if possible. We don’t care about tracking changes to the
+        * file on disk. */
        g_file_replace_async (output_file,
-                             data->last_etag,
+                             NULL,  /* ETag */
                              FALSE,  /* make_backup */
                              G_FILE_CREATE_PRIVATE | G_FILE_CREATE_REPLACE_DESTINATION,
                              io_priority,
@@ -699,7 +713,7 @@ download_file_cb (GObject      *source_object,
                return;
        }
 
-       /* Update the ETag. */
+       /* Update the stored HTTP ETag. */
        gs_utils_set_file_etag (data->output_file, new_etag, cancellable);
 
        g_task_return_boolean (task, TRUE);