From ab69f8ef7e9fbcb2e8a7bb4cc7fad717e1ecb6c8 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 3 Jun 2022 12:37:08 +0200
Subject: [PATCH] dir, system-helper: Don't ignore errors when getting a
 remote's URL

Of the 27 instances where ostree_repo_remote_get_url() is used, these
are the only ones where the return value is ignored.  This triggers
Coverity.

It might not always be strictly necessary to handle the errors, but
doing so can only help with debugging.  However, in the case of
flatpak_dir_get_remote_disabled() this clarifies the subtle difference
between an empty URL (ie., ""), and a NULL URL caused by a corrupt
configuration file or a missing "url" key.
---
 common/flatpak-dir.c                  | 39 ++++++++++++++++++++-------
 system-helper/flatpak-system-helper.c | 13 ++++-----
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 26d3d857..d8042182 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -11281,13 +11281,19 @@ flatpak_dir_install_bundle (FlatpakDir         *self,
       g_autofree char *group = g_strdup_printf ("remote \"%s\"", remote);
       g_autofree char *old_url = NULL;
       g_autoptr(GKeyFile) new_config = NULL;
+      g_autoptr(GError) local_error = NULL;
 
       /* The pull succeeded, and this is an update. So, we need to update the repo config
          if anything changed */
-      ostree_repo_remote_get_url (self->repo,
-                                  remote,
-                                  &old_url,
-                                  NULL);
+      if (!ostree_repo_remote_get_url (self->repo,
+                                       remote,
+                                       &old_url,
+                                       &local_error))
+        {
+          g_debug ("Unable to get the URL for remote %s: %s", remote, local_error->message);
+          g_clear_error (&local_error);
+        }
+
       if (origin != NULL &&
           (old_url == NULL || strcmp (old_url, origin) != 0))
         {
@@ -15296,15 +15302,25 @@ flatpak_dir_get_remote_disabled (FlatpakDir *self,
 {
   GKeyFile *config = flatpak_dir_get_repo_config (self);
   g_autofree char *group = get_group (remote_name);
-  g_autofree char *url = NULL;
 
   if (config &&
       g_key_file_get_boolean (config, group, "xa.disable", NULL))
     return TRUE;
 
-  if (self->repo &&
-      ostree_repo_remote_get_url (self->repo, remote_name, &url, NULL) && *url == 0)
-    return TRUE; /* Empty URL => disabled */
+  if (self->repo)
+    {
+      g_autoptr(GError) error = NULL;
+      g_autofree char *url = NULL;
+
+      if (!ostree_repo_remote_get_url (self->repo, remote_name, &url, &error))
+        {
+          g_debug ("Unable to get the URL for remote %s: %s", remote_name, error->message);
+          return FALSE;
+        }
+
+      if (*url == 0)
+        return TRUE; /* Empty URL => disabled */
+    }
 
   return FALSE;
 }
@@ -15840,6 +15856,7 @@ flatpak_dir_remove_remote (FlatpakDir   *self,
   GHashTableIter hash_iter;
   gpointer key;
   g_autofree char *url = NULL;
+  g_autoptr(GError) local_error = NULL;
 
   if (flatpak_dir_use_system_helper (self, NULL))
     {
@@ -15916,7 +15933,11 @@ flatpak_dir_remove_remote (FlatpakDir   *self,
                                      cancellable, error))
     return FALSE;
 
-  ostree_repo_remote_get_url (self->repo, remote_name, &url, NULL);
+  if (!ostree_repo_remote_get_url (self->repo, remote_name, &url, &local_error))
+    {
+      g_debug ("Unable to get the URL for remote %s: %s", remote_name, local_error->message);
+      g_clear_error (&local_error);
+    }
 
   if (!ostree_repo_remote_change (self->repo, NULL,
                                   OSTREE_REPO_REMOTE_CHANGE_DELETE,
diff --git a/system-helper/flatpak-system-helper.c b/system-helper/flatpak-system-helper.c
index 622f7175..4b87b144 100644
--- a/system-helper/flatpak-system-helper.c
+++ b/system-helper/flatpak-system-helper.c
@@ -495,15 +495,12 @@ handle_deploy (FlatpakSystemHelper   *object,
       g_autofree char *upstream_url = NULL;
       g_autoptr(FlatpakImageSource) system_image_source = NULL;
 
-      ostree_repo_remote_get_url (flatpak_dir_get_repo (system),
-                                  arg_origin,
-                                  &upstream_url,
-                                  NULL);
-
-      if (upstream_url == NULL)
+      if (!ostree_repo_remote_get_url (flatpak_dir_get_repo (system),
+                                       arg_origin,
+                                       &upstream_url,
+                                       &error))
         {
-          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_FAILED,
-                                                 "Remote %s is disabled", arg_origin);
+          flatpak_invocation_return_error (invocation, error, "Remote %s is disabled", arg_origin);
           return G_DBUS_METHOD_INVOCATION_HANDLED;
         }