From e5256c82e569694c64adbbe4c1bef12bbba94f30 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 7 Aug 2020 09:05:58 +0200 Subject: refs: fix interleaving hook calls with reference-transaction hook In order to not repeatedly search for the reference-transaction hook in case it's getting called multiple times, we use a caching mechanism to only call `find_hook()` once. What was missed though is that the return value of `find_hook()` actually comes from a static strbuf, which means it will get overwritten when calling `find_hook()` again. As a result, we may call the wrong hook with parameters of the reference-transaction hook. This scenario was spotted in the wild when executing a git-push(1) with multiple references, where there are interleaving calls to both the update and the reference-transaction hook. While initial calls to the reference-transaction hook work as expected, it will stop working after the next invocation of the update hook. The result is that we now start calling the update hook with parameters and stdin of the reference-transaction hook. This commit fixes the issue by storing a copy of `find_hook()`'s return value in the cache. Signed-off-by: Junio C Hamano --- refs.c | 2 +- t/t1416-ref-transaction-hooks.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index f05295f503..836d49b958 100644 --- a/refs.c +++ b/refs.c @@ -2001,7 +2001,7 @@ static int run_transaction_hook(struct ref_transaction *transaction, if (hook == &hook_not_found) return ret; if (!hook) - hook = find_hook("reference-transaction"); + hook = xstrdup_or_null(find_hook("reference-transaction")); if (!hook) { hook = &hook_not_found; return ret; diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index da58d867a5..d4d19194bf 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -106,4 +106,30 @@ test_expect_success 'hook gets all queued updates in aborted state' ' test_cmp expect actual ' +test_expect_success 'interleaving hook calls succeed' ' + test_when_finished "rm -r target-repo.git" && + + git init --bare target-repo.git && + + write_script target-repo.git/hooks/reference-transaction <<-\EOF && + echo $0 "$@" >>actual + EOF + + write_script target-repo.git/hooks/update <<-\EOF && + echo $0 "$@" >>actual + EOF + + cat >expect <<-EOF && + hooks/update refs/tags/PRE 0000000000000000000000000000000000000000 63ac8e7bcdb882293465435909f54a96de17d4f7 + hooks/reference-transaction prepared + hooks/reference-transaction committed + hooks/update refs/tags/POST 0000000000000000000000000000000000000000 99d53161c3a0a903b6561b9f6c0c665b3a476401 + hooks/reference-transaction prepared + hooks/reference-transaction committed + EOF + + git push ./target-repo.git PRE POST && + test_cmp expect target-repo.git/actual +' + test_done -- cgit v1.3-5-g9baa From 09b2aa30c916bd6facb47a06711755f043b6f37e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 11 Aug 2020 02:53:47 -0400 Subject: t1416: avoid hard-coded sha1 ids The test added by e5256c82e5 (refs: fix interleaving hook calls with reference-transaction hook, 2020-08-07) uses hard-coded sha1 object ids in its expected output. This causes it to fail when run with GIT_TEST_DEFAULT_HASH=sha256. Let's make use of the oid variables we define earlier, as the rest of the nearby tests do. Signed-off-by: Jeff King Reviewed-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1416-ref-transaction-hooks.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index d4d19194bf..f6e741c6c0 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -7,6 +7,7 @@ test_description='reference transaction hooks' test_expect_success setup ' mkdir -p .git/hooks && test_commit PRE && + PRE_OID=$(git rev-parse PRE) && test_commit POST && POST_OID=$(git rev-parse POST) ' @@ -120,10 +121,10 @@ test_expect_success 'interleaving hook calls succeed' ' EOF cat >expect <<-EOF && - hooks/update refs/tags/PRE 0000000000000000000000000000000000000000 63ac8e7bcdb882293465435909f54a96de17d4f7 + hooks/update refs/tags/PRE $ZERO_OID $PRE_OID hooks/reference-transaction prepared hooks/reference-transaction committed - hooks/update refs/tags/POST 0000000000000000000000000000000000000000 99d53161c3a0a903b6561b9f6c0c665b3a476401 + hooks/update refs/tags/POST $ZERO_OID $POST_OID hooks/reference-transaction prepared hooks/reference-transaction committed EOF -- cgit v1.3-5-g9baa