From 3234295324fe63f488cc68632066a784d4d11f53 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 4 Jul 2017 06:55:51 -0400
Subject: [PATCH] pull: Cleanly error when doing local pulls of remote-prefixed
 refs

In the storage PR I was trying to do a `pull-local` of the whole
`/ostree/repo` on the system, which ended up triggering a `g_critical()`
in the collections code, since we tried to parse a remote-prefixed ref
`fedora:fedora/26/x86_64/atomic-host` as a ref.

I'm not sure offhand what our behavior in this case *should* be.  I
think git only clones local refs, but I need to check.

This corner case arises only with `pull-local`.  But in any case,
while we were previously saying this is programmer error, since it's
so easy to pass various unchecked input into the pull machinery,
make invalid refs an explicit error.

Closes: #992
Approved by: jlebon
---
 src/libostree/ostree-repo-pull.c |  2 ++
 tests/pull-test.sh               | 22 +++++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 7346b299..b51220bb 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -3561,6 +3561,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
             }
           else
             {
+              if (!ostree_validate_rev (branch, error))
+                goto out;
               char *commitid = commitid_strviter ? g_strdup (*commitid_strviter) : NULL;
               g_hash_table_insert (requested_refs_to_fetch,
                                    ostree_collection_ref_new (NULL, branch), commitid);
diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 1da0fa4c..aaba2a7b 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..25"
+echo "1..26"
 
 # Try both syntaxes
 repo_init --no-gpg-verify
@@ -161,6 +161,26 @@ ${CMD_PREFIX} ostree --repo=mirrorrepo-local fsck
 $OSTREE show main >/dev/null
 echo "ok pull local mirror"
 
+cd ${test_tmpdir}
+# This is more of a known issue; test that we give a clean error right now
+rm otherrepo -rf
+ostree_repo_init otherrepo --mode=archive
+rm checkout-origin-main -rf
+${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo checkout main checkout-origin-main
+${CMD_PREFIX} ostree --repo=otherrepo commit -b localbranch --tree=dir=checkout-origin-main
+${CMD_PREFIX} ostree --repo=otherrepo remote add --set=gpg-verify=false origin file://$(pwd)/ostree-srv/gnomerepo
+${CMD_PREFIX} ostree --repo=otherrepo pull origin main
+rm mirrorrepo-local -rf
+ostree_repo_init mirrorrepo-local --mode=archive
+if ${CMD_PREFIX} ostree --repo=mirrorrepo-local pull-local otherrepo 2>err.txt; then
+    fatal "pull with mixed refs succeeded?"
+fi
+assert_file_has_content err.txt "error: Invalid ref name origin:main"
+${CMD_PREFIX} ostree --repo=mirrorrepo-local pull-local otherrepo localbranch
+${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"
+
 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