git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] send-pack: fix push nego. when remote has refs

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jun 23, 2021, 22:30 UTC
Message-ID
<175da5f02b319bb637700e4f6665ee346674e1b0.1624486920.git.jonathantanmy@google.com>
In-Reply-To
<cover.1624486920.git.jonathantanmy@google.com>

Commit 477673d6f3 ("send-pack: support push negotiation", 2021-05-05) did not test the case in which a remote advertises at least one ref. In such a case, "remote_refs" in get_commons_through_negotiation() in send-pack.c would also contain those refs with a zero ref->new_oid (in addition to the refs being pushed with a nonzero ref->new_oid). Passing them as negotiation tips to "git fetch" causes an error, so filter them out.

(The exact error that would happen in "git fetch" in this case is a segmentation fault, which is unwanted. This will be fixed in the subsequent commit.)

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 send-pack.c           | 6 ++++--
 t/t5516-fetch-push.sh | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 9cb9f71650..85945becf0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -425,8 +425,10 @@ static void get_commons_through_negotiation(const char *url,
 	child.no_stdin = 1;
 	child.out = -1;
 	strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
-	for (ref = remote_refs; ref; ref = ref->next)
-		strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (!is_null_oid(&ref->new_oid))
+			strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
+	}
 	strvec_push(&child.args, url);
 
 	if (start_command(&child))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 5ce32e531a..e383ba662f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -201,6 +201,7 @@ test_expect_success 'push with negotiation' '
 	# Without negotiation
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
 	echo now pushing without negotiation &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 push testrepo refs/heads/main:refs/remotes/origin/main &&
@@ -210,6 +211,7 @@ test_expect_success 'push with negotiation' '
 	rm event &&
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main &&
 	grep_wrote 2 event # 1 commit, 1 tree
@@ -219,6 +221,7 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f
 	rm event &&
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
 	GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
 		git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
@@ -1783,6 +1786,7 @@ test_expect_success 'http push with negotiation' '
 	# Without negotiation
 	test_create_repo "$SERVER" &&
 	test_config -C "$SERVER" http.receivepack true &&
+	test_commit -C "$SERVER" unrelated_commit &&
 	git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
 	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 \
@@ -1794,6 +1798,7 @@ test_expect_success 'http push with negotiation' '
 	rm -rf "$SERVER" &&
 	test_create_repo "$SERVER" &&
 	test_config -C "$SERVER" http.receivepack true &&
+	test_commit -C "$SERVER" unrelated_commit &&
 	git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
 	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 -c push.negotiate=1 \
-- 
2.32.0.288.g62a8d224e6-goog
Previous: Jonathan TanNext: Emily Shaffer
Message 2 of 24 in “Push negotiation fixes”
  1. 0/3 Push negotiation fixesJonathan Tan, Jun 23, 2021
  2. 2/3 send-pack: fix push nego. when remote has refsJonathan Tan, Jun 23, 2021
  3. Emily ShafferJul 13, 2021
  4. Jonathan TanJul 14, 2021
  5. 1/3 send-pack: fix push.negotiate with remote helperJonathan Tan, Jun 23, 2021
  6. Emily ShafferJul 13, 2021
  7. Jonathan TanJul 14, 2021
  8. Ævar Arnfjörð BjarmasonJul 13, 2021
  9. Jonathan TanJul 14, 2021
  10. Ævar Arnfjörð BjarmasonJul 14, 2021
  11. 3/3 fetch: die on invalid --negotiation-tip hashJonathan Tan, Jun 23, 2021
  12. Emily ShafferJul 13, 2021
  13. Ævar Arnfjörð BjarmasonJul 13, 2021
  14. Jonathan TanJul 14, 2021
  15. Ævar Arnfjörð BjarmasonJul 14, 2021
  16. 0/3 Push negotiation fixesJonathan Tan, Jul 15, 2021
  17. 1/3 send-pack: fix push.negotiate with remote helperJonathan Tan, Jul 15, 2021
  18. Ævar Arnfjörð BjarmasonJul 27, 2021
  19. 2/3 send-pack: fix push nego. when remote has refsJonathan Tan, Jul 15, 2021
  20. Ævar Arnfjörð BjarmasonJul 27, 2021
  21. Jeff KingJul 27, 2021
  22. Jonathan TanJul 27, 2021
  23. 3/3 fetch: die on invalid --negotiation-tip hashJonathan Tan, Jul 15, 2021
  24. Junio C HamanoJul 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.