From b8c15ae859de7a353b99c98c6266ee626cd94e7e Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Mon, 2 Oct 2017 11:24:05 -0400
Subject: [PATCH] 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
---
 src/libostree/ostree-repo-pull.c | 13 ++++++-------
 tests/pull-test.sh               | 10 +++++++++-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 8a699ca0..d5062e01 100644
--- 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 7d4b57f9..2afc0aca 100644
--- 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