pull: Update key loading function to match error style

This code wasn't written with idiomatic GError usage; it's not standard
to construct an error up front and continually append to its
message.  The exit from a function is usually `return TRUE`,
with error conditions before that.

Updating it to match style reveals what I think is a bug;
we were silently ignoring failure to parse key files.

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 454eaf8ac5254ccf57663a2ab63812f2073e3b5c..a5e3a7293610225b22922b13941d3c18b780c37c 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1488,14 +1488,10 @@ _load_public_keys (OstreeSign *sign,
                    const gchar *remote_name,
                    GError **error)
 {
-
   g_autofree gchar *pk_ascii = NULL;
   g_autofree gchar *pk_file = NULL;
   gboolean loaded_from_file = TRUE;
   gboolean loaded_inlined = TRUE;
-  g_autoptr (GError) verification_error = NULL;
-
-  glnx_throw (&verification_error, "no public keys loaded");
 
   ostree_repo_get_remote_option (repo,
                                  remote_name,
@@ -1536,10 +1532,8 @@ _load_public_keys (OstreeSign *sign,
         loaded_from_file = TRUE;
       else
         {
-          g_debug("Unable to load public keys for '%s' from file '%s': %s",
-                  ostree_sign_get_name(sign), pk_file, local_error->message);
-          /* Save error message for better reason detection later if needed */
-          glnx_prefix_error (&verification_error, "%s", local_error->message);
+          return glnx_throw (error, "Failed loading '%s' keys from '%s",
+                             ostree_sign_get_name (sign), pk_file);
         }
     }
 
@@ -1556,19 +1550,16 @@ _load_public_keys (OstreeSign *sign,
 
       if (!loaded_inlined)
         {
-          g_debug("Unable to load public key '%s' for '%s': %s",
-                  pk_ascii, ostree_sign_get_name (sign), local_error->message);
-
-          /* Save error message for better reason detection later if needed */
-          glnx_prefix_error (&verification_error, "%s", local_error->message);
+          return glnx_throw (error, "Failed loading '%s' keys from inline `verification-key`",
+                             ostree_sign_get_name (sign));
         }
     }
 
   /* Return true if able to load from any source */
-  if (loaded_from_file || loaded_inlined)
-    return TRUE;
+  if (!(loaded_from_file || loaded_inlined))
+    return glnx_throw (error, "No keys found");
 
-  return glnx_throw (error, "%s", verification_error->message);
+  return TRUE;
 }
 
 static gboolean

diff --git a/tests/test-signed-pull.sh b/tests/test-signed-pull.sh
index f222db4fb3ce1fe4fdcac64fbfb0a937f497e3c3..2c677d46fb424982899c97370ea43af709323e9e 100755 (executable)
--- a/tests/test-signed-pull.sh
+++ b/tests/test-signed-pull.sh
@@ -93,6 +93,7 @@ echo "ok pull failure with incorrect keys file option"
 
 # Test with correct dummy key
 ${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-key "${DUMMYSIGN}"
+${CMD_PREFIX} ostree --repo=repo config unset 'remote "origin"'.verification-file
 test_signed_pull "dummy" ""
 
 if ! has_libsodium; then