Merge branch 'jc/receive-deny-current-branch-fix' into maint
authorJunio C Hamano <gitster@pobox.com>
Wed, 21 Nov 2018 13:57:51 +0000 (22:57 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 21 Nov 2018 13:57:51 +0000 (22:57 +0900)
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

1  2 
builtin/receive-pack.c
t/t5516-fetch-push.sh
diff --combined builtin/receive-pack.c
index c17ce94e12ee34c5b822b0e09fcd6d7264e759ad,d28a35c9920b84a3c527f333fd5aa3998f1a3630..1e9d0dfb6d857db81b6c7bde55f2be45a594cb91
@@@ -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;
                        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)) {
                                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;
                }
        }
                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 ||
                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 539c25aadafdcf6aa9fbcce0988631702d3fb02d,1a67f479769d71db2b9c90f5345f3a0202bdcfa3..bb0a36535c1a2f40d6d4cead5703af0f46b27301
@@@ -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
        ) &&
        (
        )
  '
  
 -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' '