lib/pull: Fix regression with pull-local for nonexistent refs

I was reading the pull code for the last release, and spotted
a bug in commit f923c2e1eaebe0c781f07d34ae1a03f94357bccd - in
the case where the ref doesn't exist, we don't set an error,
tripping an assertion in the main code.

The previous code wanted the ref to always exist, so just flip back the boolean
for "ignore noent". I moved the `g_strchomp()` just into the HTTP path - if a
local repo is corrupted in this way it's something to fix in that repo.

Closes: #1238
Approved by: pwithnall

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 8a699ca0805d6a3187573b887a07f75eaab05b25..d5062e0194e7fc2a406eb20373ba7b554a0b56f6 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -844,7 +844,7 @@ fetch_ref_contents (OtPullData                 *pull_data,
     {
 #ifdef OSTREE_ENABLE_EXPERIMENTAL_API
       if (!ostree_repo_resolve_collection_ref (pull_data->remote_repo_local,
-                                               ref, TRUE  /* ignore enoent */,
+                                               ref, FALSE,
                                                OSTREE_REPO_RESOLVE_REV_EXT_NONE,
                                                &ret_contents, cancellable, error))
         return FALSE;
@@ -855,7 +855,7 @@ fetch_ref_contents (OtPullData                 *pull_data,
   else if (pull_data->remote_repo_local != NULL)
     {
       if (!ostree_repo_resolve_rev_ext (pull_data->remote_repo_local,
-                                        ref->ref_name, TRUE  /* ignore enoent */,
+                                        ref->ref_name, FALSE,
                                         OSTREE_REPO_RESOLVE_REV_EXT_NONE,
                                         &ret_contents, error))
         return FALSE;
@@ -874,14 +874,13 @@ fetch_ref_contents (OtPullData                 *pull_data,
                                                   filename, &ret_contents,
                                                   cancellable, error))
         return FALSE;
+
+      g_strchomp (ret_contents);
     }
 
-  /* Validate and return. */
-  if (ret_contents != NULL)
-    g_strchomp (ret_contents);
+  g_assert (ret_contents);
 
-  if (ret_contents == NULL ||
-      !ostree_validate_checksum_string (ret_contents, error))
+  if (!ostree_validate_checksum_string (ret_contents, error))
     return glnx_prefix_error (error, "Fetching checksum for ref (%s, %s)",
                               ref->collection_id ? ref->collection_id : "(empty)",
                               ref->ref_name);

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 7d4b57f9b809a3f4aea27386102791a41292b96f..2afc0aca9f182bc238c55dc463092884a2af8467 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -35,7 +35,7 @@ function verify_initial_contents() {
     assert_file_has_content baz/cow '^moo$'
 }
 
-echo "1..31"
+echo "1..32"
 
 # Try both syntaxes
 repo_init --no-gpg-verify
@@ -238,6 +238,14 @@ ${CMD_PREFIX} ostree --repo=mirrorrepo-local rev-parse localbranch
 ${CMD_PREFIX} ostree --repo=mirrorrepo-local fsck
 echo "ok pull-local mirror errors with mixed refs"
 
+rm -f otherrepo/summary
+if ${CMD_PREFIX} ostree --repo=mirrorrepo-local pull-local otherrepo nosuchbranch 2>err.txt; then
+    fatal "pulled nonexistent branch"
+fi
+# So true
+assert_file_has_content_literal err.txt "error: Refspec 'nosuchbranch' not found"
+echo "ok pull-local nonexistent branch"
+
 cd ${test_tmpdir}
 ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit -b main -s "Metadata string" --add-detached-metadata-string=SIGNATURE=HANCOCK --tree=ref=main
 ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u