From: Chen Bojun <bojun.cbj@xxxxxxxxxxxxxxx> When pushing a hidden ref, e.g.: $ git push origin HEAD:refs/hidden/foo "receive-pack" will reject our request with an error message like this: ! [remote rejected] HEAD -> refs/hidden/foo (deny updating a hidden ref) The remote side ("git-receive-pack") will not create the hidden ref as expected, but the pack file sent by "git-send-pack" is left inside the remote repository. I.e. the quarantine directory is not purged as it should be. Add a checkpoint before calling "tmp_objdir_migrate()" and after calling the "pre-receive" hook to purge that temporary data in the quarantine area when there is no command ready to run. The reason we do not add the checkpoint before the "pre-receive" hook, but after it, is that the "pre-receive" hook is called with a switch-off "skip_broken" flag, and all commands, even broken ones, should be fed by calling "feed_receive_hook()". Add a new test case and fix some formatting issues in t5516 as well. Helped-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx> Helped-by: Teng Long <dyroneteng@xxxxxxxxx> Signed-off-by: Chen Bojun <bojun.cbj@xxxxxxxxxxxxxxx> --- receive-pack: purge temporary data if no command is ready to run When pushing a hidden ref, e.g.: $ git push origin HEAD:refs/hidden/foo "receive-pack" will reject our request with an error message like this: ! [remote rejected] HEAD -> refs/hidden/foo (deny updating a hidden ref) The remote side ("git-receive-pack") will not create the hidden ref as expected, but the pack file sent by "git-send-pack" is left inside the remote repository. I.e. the quarantine directory is not purged as it should be. Add a checkpoint before calling "tmp_objdir_migrate()" and after calling the "pre-receive" hook to purge that temporary data in the quarantine area when there is no command ready to run. The reason we do not add the checkpoint before the "pre-receive" hook, but after it, is that the "pre-receive" hook is called with a switch-off "skip_broken" flag, and all commands, even broken ones, should be fed by calling "feed_receive_hook()". Add a new test case and fix some formatting issues in t5516 as well. Helped-by: Jiang Xin zhiyou.jx@xxxxxxxxxxxxxxx Helped-by: Teng Long dyroneteng@xxxxxxxxx Signed-off-by: Chen Bojun bojun.cbj@xxxxxxxxxxxxxxx Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1124%2FBerger7%2Fberger7%2Fmaster-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1124/Berger7/berger7/master-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1124 builtin/receive-pack.c | 8 +++ t/t5516-fetch-push.sh | 149 ++++++++++++++++++++--------------------- 2 files changed, 81 insertions(+), 76 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9f4a0b816cf..301dbc4824e 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1971,6 +1971,14 @@ static void execute_commands(struct command *commands, return; } + /* + * If there is no command ready to run, should return directly to destroy + * temporary data in the quarantine area. + */ + for (cmd = commands; cmd && cmd->error_string; cmd = cmd->next); + if (!cmd) + return; + /* * Now we'll start writing out refs, which means the objects need * to be in their final positions so that other processes can see them. diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 2f04cf9a1c7..b71182ae13b 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -21,99 +21,91 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME D=$(pwd) -mk_empty () { +mk_empty() { repo_name="$1" rm -fr "$repo_name" && - mkdir "$repo_name" && - ( - cd "$repo_name" && - git init && - git config receive.denyCurrentBranch warn && - mv .git/hooks .git/hooks-disabled - ) + mkdir "$repo_name" && + ( + cd "$repo_name" && + git init && + git config receive.denyCurrentBranch warn && + mv .git/hooks .git/hooks-disabled + ) } -mk_test () { +mk_test() { repo_name="$1" shift mk_empty "$repo_name" && - ( - for ref in "$@" - do - git push "$repo_name" $the_first_commit:refs/$ref || - exit - done && - cd "$repo_name" && - for ref in "$@" - do - echo "$the_first_commit" >expect && - git show-ref -s --verify refs/$ref >actual && - test_cmp expect actual || - exit - done && - git fsck --full - ) + ( + for ref in "$@"; do + git push "$repo_name" $the_first_commit:refs/$ref || + exit + done && + cd "$repo_name" && + for ref in "$@"; do + echo "$the_first_commit" >expect && + git show-ref -s --verify refs/$ref >actual && + test_cmp expect actual || + exit + done && + git fsck --full + ) } mk_test_with_hooks() { repo_name=$1 mk_test "$@" && - ( - cd "$repo_name" && - mkdir .git/hooks && - cd .git/hooks && - - cat >pre-receive <<-'EOF' && - #!/bin/sh - cat - >>pre-receive.actual - EOF - - cat >update <<-'EOF' && - #!/bin/sh - printf "%s %s %s\n" "$@" >>update.actual - EOF - - cat >post-receive <<-'EOF' && - #!/bin/sh - cat - >>post-receive.actual - EOF - - cat >post-update <<-'EOF' && - #!/bin/sh - for ref in "$@" - do - printf "%s\n" "$ref" >>post-update.actual - done - EOF - - chmod +x pre-receive update post-receive post-update - ) + ( + cd "$repo_name" && + mkdir .git/hooks && + cd .git/hooks && + cat >pre-receive <<-'EOF' && + #!/bin/sh + cat - >>pre-receive.actual + EOF + cat >update <<-'EOF' && + #!/bin/sh + printf "%s %s %s\n" "$@" >>update.actual + EOF + cat >post-receive <<-'EOF' && + #!/bin/sh + cat - >>post-receive.actual + EOF + cat >post-update <<-'EOF' && + #!/bin/sh + for ref in "$@" + do + printf "%s\n" "$ref" >>post-update.actual + done + EOF + chmod +x pre-receive update post-receive post-update + ) } mk_child() { rm -rf "$2" && - git clone "$1" "$2" + git clone "$1" "$2" } -check_push_result () { +check_push_result() { test $# -ge 3 || - BUG "check_push_result requires at least 3 parameters" + BUG "check_push_result requires at least 3 parameters" repo_name="$1" shift ( cd "$repo_name" && - echo "$1" >expect && - shift && - for ref in "$@" - do - git show-ref -s --verify refs/$ref >actual && - test_cmp expect actual || - exit - done && - git fsck --full + echo "$1" >expect && + shift && + for ref in "$@"; do + git show-ref -s --verify refs/$ref >actual && + test_cmp expect actual || + exit + done && + git fsck --full ) } @@ -191,7 +183,7 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' ) ' -grep_wrote () { +grep_wrote() { object_count=$1 file_name=$2 grep 'write_pack_file/wrote.*"value":"'$1'"' $2 @@ -477,8 +469,7 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' ' ' -for head in HEAD @ -do +for head in HEAD @; do test_expect_success "push with $head" ' mk_test testrepo heads/main && @@ -1020,7 +1011,7 @@ test_expect_success 'push into aliased refs (inconsistent)' ' ) ' -test_force_push_tag () { +test_force_push_tag() { tag_type_description=$1 tag_args=$2 @@ -1066,7 +1057,7 @@ test_force_push_tag () { test_force_push_tag "lightweight tag" "-f" test_force_push_tag "annotated tag" "-f -a -m'tag message'" -test_force_fetch_tag () { +test_force_fetch_tag() { tag_type_description=$1 tag_args=$2 @@ -1158,8 +1149,7 @@ test_expect_success 'push --prune refspec' ' ! check_push_result testrepo $the_first_commit tmp/foo tmp/bar ' -for configsection in transfer receive -do +for configsection in transfer receive; do test_expect_success "push to update a ref hidden by $configsection.hiderefs" ' mk_test testrepo heads/main hidden/one hidden/two hidden/three && ( @@ -1250,8 +1240,7 @@ test_expect_success 'fetch exact SHA1 in protocol v2' ' git -C child fetch -v ../testrepo $the_commit:refs/heads/copy ' -for configallowtipsha1inwant in true false -do +for configallowtipsha1inwant in true false; do test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" ' mk_empty testrepo && ( @@ -1809,4 +1798,12 @@ test_expect_success 'refuse fetch to current branch of bare repository worktree' git -C bare.git fetch -u .. HEAD:wt ' +test_expect_success 'refuse to push a hidden ref, and make sure do not pollute the repository' ' + mk_empty testrepo && + git -C testrepo config receive.hiderefs refs/hidden && + git -C testrepo config receive.unpackLimit 1 && + test_must_fail git push testrepo HEAD:refs/hidden/foo && + test_dir_is_empty testrepo/.git/objects/pack +' + test_done base-commit: 297ca895a27a6bbdb7906371d533f72a12ad25b2 -- gitgitgadget