From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 21 Nov 2018 13:57:51 +0000 (+0900)
Subject: Merge branch 'jc/receive-deny-current-branch-fix' into maint
X-Git-Tag: v2.19.2~43
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/7d483e9c00516413a9c2dafc24a6ecb40975b02b?hp=-c

Merge branch 'jc/receive-deny-current-branch-fix' into maint

The receive.denyCurrentBranch=updateInstead codepath kicked in even
when the push should have been rejected due to other reasons, such
as it does not fast-forward or the update-hook rejects it, which
has been corrected.

* jc/receive-deny-current-branch-fix:
receive: denyCurrentBranch=updateinstead should not blindly update
---

7d483e9c00516413a9c2dafc24a6ecb40975b02b
diff --combined builtin/receive-pack.c
index c17ce94e12,d28a35c992..1e9d0dfb6d
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@@ -25,7 -25,6 +25,7 @@@
  #include "tmp-objdir.h"
  #include "oidset.h"
  #include "packfile.h"
 +#include "object-store.h"
  #include "protocol.h"
  
  static const char * const receive_pack_usage[] = {
@@@ -630,6 -629,8 +630,6 @@@ static void prepare_push_cert_sha1(stru
  		return;
  
  	if (!already_done) {
 -		struct strbuf gpg_output = STRBUF_INIT;
 -		struct strbuf gpg_status = STRBUF_INIT;
  		int bogs /* beginning_of_gpg_sig */;
  
  		already_done = 1;
@@@ -638,11 -639,22 +638,11 @@@
  			oidclr(&push_cert_oid);
  
  		memset(&sigcheck, '\0', sizeof(sigcheck));
 -		sigcheck.result = 'N';
  
  		bogs = parse_signature(push_cert.buf, push_cert.len);
 -		if (verify_signed_buffer(push_cert.buf, bogs,
 -					 push_cert.buf + bogs, push_cert.len - bogs,
 -					 &gpg_output, &gpg_status) < 0) {
 -			; /* error running gpg */
 -		} else {
 -			sigcheck.payload = push_cert.buf;
 -			sigcheck.gpg_output = gpg_output.buf;
 -			sigcheck.gpg_status = gpg_status.buf;
 -			parse_gpg_output(&sigcheck);
 -		}
 +		check_signature(push_cert.buf, bogs, push_cert.buf + bogs,
 +				push_cert.len - bogs, &sigcheck);
  
 -		strbuf_release(&gpg_output);
 -		strbuf_release(&gpg_status);
  		nonce_status = check_nonce(push_cert.buf, bogs);
  	}
  	if (!is_null_oid(&push_cert_oid)) {
@@@ -893,7 -905,7 +893,7 @@@ static int update_shallow_ref(struct co
  	 * not lose these new roots..
  	 */
  	for (i = 0; i < extra.nr; i++)
 -		register_shallow(&extra.oid[i]);
 +		register_shallow(the_repository, &extra.oid[i]);
  
  	si->shallow_ref[cmd->index] = 0;
  	oid_array_clear(&extra);
@@@ -1025,6 -1037,7 +1025,7 @@@ static const char *update(struct comman
  	const char *ret;
  	struct object_id *old_oid = &cmd->old_oid;
  	struct object_id *new_oid = &cmd->new_oid;
+ 	int do_update_worktree = 0;
  
  	/* only refs/... are allowed */
  	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
@@@ -1050,9 -1063,8 +1051,8 @@@
  				refuse_unconfigured_deny();
  			return "branch is currently checked out";
  		case DENY_UPDATE_INSTEAD:
- 			ret = update_worktree(new_oid->hash);
- 			if (ret)
- 				return ret;
+ 			/* pass -- let other checks intervene first */
+ 			do_update_worktree = 1;
  			break;
  		}
  	}
@@@ -1095,8 -1107,8 +1095,8 @@@
  		struct object *old_object, *new_object;
  		struct commit *old_commit, *new_commit;
  
 -		old_object = parse_object(old_oid);
 -		new_object = parse_object(new_oid);
 +		old_object = parse_object(the_repository, old_oid);
 +		new_object = parse_object(the_repository, new_oid);
  
  		if (!old_object || !new_object ||
  		    old_object->type != OBJ_COMMIT ||
@@@ -1117,9 -1129,15 +1117,15 @@@
  		return "hook declined";
  	}
  
+ 	if (do_update_worktree) {
+ 		ret = update_worktree(new_oid->hash);
+ 		if (ret)
+ 			return ret;
+ 	}
+ 
  	if (is_null_oid(new_oid)) {
  		struct strbuf err = STRBUF_INIT;
 -		if (!parse_object(old_oid)) {
 +		if (!parse_object(the_repository, old_oid)) {
  			old_oid = NULL;
  			if (ref_exists(name)) {
  				rp_warning("Allowing deletion of corrupt ref.");
diff --combined t/t5516-fetch-push.sh
index 539c25aada,1a67f47976..bb0a36535c
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@@ -923,7 -923,7 +923,7 @@@ test_expect_success 'push into aliased 
  	(
  		cd child1 &&
  		git branch foo &&
 -		git symbolic-ref refs/heads/bar refs/heads/foo
 +		git symbolic-ref refs/heads/bar refs/heads/foo &&
  		git config receive.denyCurrentBranch false
  	) &&
  	(
@@@ -945,7 -945,7 +945,7 @@@ test_expect_success 'push into aliased 
  	(
  		cd child1 &&
  		git branch foo &&
 -		git symbolic-ref refs/heads/bar refs/heads/foo
 +		git symbolic-ref refs/heads/bar refs/heads/foo &&
  		git config receive.denyCurrentBranch false
  	) &&
  	(
@@@ -965,51 -965,26 +965,51 @@@
  	)
  '
  
 -test_expect_success 'push requires --force to update lightweight tag' '
 -	mk_test testrepo heads/master &&
 -	mk_child testrepo child1 &&
 -	mk_child testrepo child2 &&
 -	(
 -		cd child1 &&
 -		git tag Tag &&
 -		git push ../child2 Tag &&
 -		git push ../child2 Tag &&
 -		>file1 &&
 -		git add file1 &&
 -		git commit -m "file1" &&
 -		git tag -f Tag &&
 -		test_must_fail git push ../child2 Tag &&
 -		git push --force ../child2 Tag &&
 -		git tag -f Tag &&
 -		test_must_fail git push ../child2 Tag HEAD~ &&
 -		git push --force ../child2 Tag
 -	)
 -'
 +test_force_push_tag () {
 +	tag_type_description=$1
 +	tag_args=$2
 +
 +	test_expect_success 'force pushing required to update lightweight tag' "
 +		mk_test testrepo heads/master &&
 +		mk_child testrepo child1 &&
 +		mk_child testrepo child2 &&
 +		(
 +			cd child1 &&
 +			git tag testTag &&
 +			git push ../child2 testTag &&
 +			>file1 &&
 +			git add file1 &&
 +			git commit -m 'file1' &&
 +			git tag $tag_args testTag &&
 +			test_must_fail git push ../child2 testTag &&
 +			git push --force ../child2 testTag &&
 +			git tag $tag_args testTag HEAD~ &&
 +			test_must_fail git push ../child2 testTag &&
 +			git push --force ../child2 testTag &&
 +
 +			# Clobbering without + in refspec needs --force
 +			git tag -f testTag &&
 +			test_must_fail git push ../child2 'refs/tags/*:refs/tags/*' &&
 +			git push --force ../child2 'refs/tags/*:refs/tags/*' &&
 +
 +			# Clobbering with + in refspec does not need --force
 +			git tag -f testTag HEAD~ &&
 +			git push ../child2 '+refs/tags/*:refs/tags/*' &&
 +
 +			# Clobbering with --no-force still obeys + in refspec
 +			git tag -f testTag &&
 +			git push --no-force ../child2 '+refs/tags/*:refs/tags/*' &&
 +
 +			# Clobbering with/without --force and 'tag <name>' format
 +			git tag -f testTag HEAD~ &&
 +			test_must_fail git push ../child2 tag testTag &&
 +			git push --force ../child2 tag testTag
 +		)
 +	"
 +}
 +
 +test_force_push_tag "lightweight tag" "-f"
 +test_force_push_tag "annotated tag" "-f -a -m'msg'"
  
  test_expect_success 'push --porcelain' '
  	mk_empty testrepo &&
@@@ -1036,7 -1011,7 +1036,7 @@@ test_expect_success 'push --porcelain r
  	mk_empty testrepo &&
  	git push testrepo refs/heads/master:refs/remotes/origin/master &&
  	(cd testrepo &&
 -		git reset --hard origin/master^
 +		git reset --hard origin/master^ &&
  		git config receive.denyCurrentBranch true) &&
  
  	echo >.git/foo  "To testrepo"  &&
@@@ -1050,7 -1025,7 +1050,7 @@@ test_expect_success 'push --porcelain -
  	mk_empty testrepo &&
  	git push testrepo refs/heads/master:refs/remotes/origin/master &&
  	(cd testrepo &&
 -		git reset --hard origin/master
 +		git reset --hard origin/master &&
  		git config receive.denyCurrentBranch true) &&
  
  	echo >.git/foo  "To testrepo"  &&
@@@ -1358,7 -1333,7 +1358,7 @@@ test_expect_success 'push --follow-tag 
  		git commit --allow-empty -m "future commit" &&
  		git tag -m "future" future &&
  		git checkout master &&
 -		git for-each-ref refs/heads/master refs/tags/tag >../expect
 +		git for-each-ref refs/heads/master refs/tags/tag >../expect &&
  		git push --follow-tag ../dst master
  	) &&
  	(
@@@ -1552,7 -1527,13 +1552,13 @@@ test_expect_success 'receive.denyCurren
  		test $(git -C .. rev-parse master) = $(git rev-parse HEAD) &&
  		git diff --quiet &&
  		git diff --cached --quiet
- 	)
+ 	) &&
+ 
+ 	# (6) updateInstead intervened by fast-forward check
+ 	test_must_fail git push void master^:master &&
+ 	test $(git -C void rev-parse HEAD) = $(git rev-parse master) &&
+ 	git -C void diff --quiet &&
+ 	git -C void diff --cached --quiet
  '
  
  test_expect_success 'updateInstead with push-to-checkout hook' '