diff --git a/fetch-pack.c b/fetch-pack.c index 0f158776b0..3f24d0c8a6 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -573,9 +573,14 @@ static void filter_refs(struct fetch_pack_args *args, next = ref->next; if (starts_with(ref->name, "refs/") && - check_refname_format(ref->name, 0)) - ; /* trash */ - else { + check_refname_format(ref->name, 0)) { + /* + * trash or a peeled value; do not even add it to + * unmatched list + */ + free_one_ref(ref); + continue; + } else { while (i < nr_sought) { int cmp = strcmp(ref->name, sought[i]->name); if (cmp < 0) @@ -630,10 +635,7 @@ static void filter_refs(struct fetch_pack_args *args, } oidset_clear(&tip_oids); - for (ref = unmatched; ref; ref = next) { - next = ref->next; - free(ref); - } + free_refs(unmatched); *refs = newlist; } diff --git a/pkt-line.c b/pkt-line.c index ffd7220544..c9ed780d0b 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -350,16 +350,17 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer, return PACKET_READ_EOF; } - if ((options & PACKET_READ_DIE_ON_ERR_PACKET) && - starts_with(buffer, "ERR ")) - die(_("remote error: %s"), buffer + 4); - if ((options & PACKET_READ_CHOMP_NEWLINE) && len && buffer[len-1] == '\n') len--; buffer[len] = 0; packet_trace(buffer, len, 0); + + if ((options & PACKET_READ_DIE_ON_ERR_PACKET) && + starts_with(buffer, "ERR ")) + die(_("remote error: %s"), buffer + 4); + *pktlen = len; return PACKET_READ_NORMAL; } diff --git a/remote.c b/remote.c index 9cc3b07d21..3fe34eae85 100644 --- a/remote.c +++ b/remote.c @@ -820,11 +820,11 @@ struct ref *copy_ref_list(const struct ref *ref) return ret; } -static void free_ref(struct ref *ref) +void free_one_ref(struct ref *ref) { if (!ref) return; - free_ref(ref->peer_ref); + free_one_ref(ref->peer_ref); free(ref->remote_status); free(ref->symref); free(ref); @@ -835,7 +835,7 @@ void free_refs(struct ref *ref) struct ref *next; while (ref) { next = ref->next; - free_ref(ref); + free_one_ref(ref); ref = next; } } diff --git a/remote.h b/remote.h index da53ad570b..f58332a27e 100644 --- a/remote.h +++ b/remote.h @@ -131,8 +131,10 @@ int ref_compare_name(const void *, const void *); int check_ref_type(const struct ref *ref, int flags); /* - * Frees the entire list and peers of elements. + * Free a single ref and its peer, or an entire list of refs and their peers, + * respectively. */ +void free_one_ref(struct ref *ref); void free_refs(struct ref *ref); struct oid_array; diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 49bf4280e8..c81ca360ac 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1241,9 +1241,9 @@ do cd shallow && # Some protocol versions (e.g. 2) support fetching # unadvertised objects, so restrict this test to v0. - test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \ + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \ git fetch ../testrepo/.git $SHA1_3 && - test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \ + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \ git fetch ../testrepo/.git $SHA1_1 && git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true && git fetch ../testrepo/.git $SHA1_1 && @@ -1251,8 +1251,9 @@ do test_must_fail git cat-file commit $SHA1_2 && git fetch ../testrepo/.git $SHA1_2 && git cat-file commit $SHA1_2 && - test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \ - git fetch ../testrepo/.git $SHA1_3 + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \ + git fetch ../testrepo/.git $SHA1_3 2>err && + test_i18ngrep "remote error:.*not our ref.*$SHA1_3\$" err ) ' done @@ -1284,6 +1285,17 @@ test_expect_success 'fetch follows tags by default' ' test_cmp expect actual ' +test_expect_success 'peeled advertisements are not considered ref tips' ' + mk_empty testrepo && + git -C testrepo commit --allow-empty -m one && + git -C testrepo commit --allow-empty -m two && + git -C testrepo tag -m foo mytag HEAD^ && + oid=$(git -C testrepo rev-parse mytag^{commit}) && + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \ + git fetch testrepo $oid 2>err && + test_i18ngrep "Server does not allow request for unadvertised object" err +' + test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' ' mk_test testrepo heads/master && rm -fr src dst && diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh index 4f6e32b04c..a1d3031d40 100755 --- a/t/t5530-upload-pack-error.sh +++ b/t/t5530-upload-pack-error.sh @@ -57,13 +57,25 @@ test_expect_success 'upload-pack fails due to error in rev-list' ' grep "bad tree object" output.err ' -test_expect_success 'upload-pack error message when bad ref requested' ' +test_expect_success 'upload-pack fails due to bad want (no object)' ' printf "0045want %s multi_ack_detailed\n00000009done\n0000" \ "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" >input && test_must_fail git upload-pack . output 2>output.err && - grep -q "not our ref" output.err && - ! grep -q multi_ack_detailed output.err + grep "not our ref" output.err && + grep "ERR" output && + ! grep multi_ack_detailed output.err +' + +test_expect_success 'upload-pack fails due to bad want (not tip)' ' + + oid=$(echo an object we have | git hash-object -w --stdin) && + printf "0045want %s multi_ack_detailed\n00000009done\n0000" \ + "$oid" >input && + test_must_fail git upload-pack . output 2>output.err && + grep "not our ref" output.err && + grep "ERR" output && + ! grep multi_ack_detailed output.err ' test_expect_success 'upload-pack fails due to error in pack-objects enumeration' ' diff --git a/upload-pack.c b/upload-pack.c index d098ef5982..cb603a6d8a 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -592,7 +592,8 @@ error: return 1; } -static void check_non_tip(struct object_array *want_obj) +static void check_non_tip(struct object_array *want_obj, + struct packet_writer *writer) { int i; @@ -611,9 +612,13 @@ error: /* Pick one of them (we know there at least is one) */ for (i = 0; i < want_obj->nr; i++) { struct object *o = want_obj->objects[i].item; - if (!is_our_ref(o)) + if (!is_our_ref(o)) { + packet_writer_error(writer, + "upload-pack: not our ref %s", + oid_to_hex(&o->oid)); die("git upload-pack: not our ref %s", oid_to_hex(&o->oid)); + } } } @@ -936,7 +941,7 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan * by another process that handled the initial request. */ if (has_non_tip) - check_non_tip(want_obj); + check_non_tip(want_obj, &writer); if (!use_sideband && daemon_mode) no_progress = 1;