Merge branch 'ab/fetch-negotiate-segv-fix'

gitster · gitster · commit b2fc8226294e · 2021-07-16T17:42:48.000-07:00
Code recently added to support common ancestry negotiation during
"git push" did not sanity check its arguments carefully enough.

* ab/fetch-negotiate-segv-fix:
  fetch: fix segfault in --negotiate-only without --negotiation-tip=*
  fetch: document the --negotiate-only option
  send-pack.c: move "no refs in common" abort earlier
diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
@@ -69,7 +69,8 @@ fetch.negotiationAlgorithm::
 	setting defaults to "skipping".
 	Unknown values will cause 'git fetch' to error out.
 +
-See also the `--negotiation-tip` option for linkgit:git-fetch[1].
+See also the `--negotiate-only` and `--negotiation-tip` options to
+linkgit:git-fetch[1].
 
 fetch.showForcedUpdates::
 	Set to false to enable `--no-show-forced-updates` in
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
@@ -62,8 +62,17 @@ The argument to this option may be a glob on ref names, a ref, or the (possibly
 abbreviated) SHA-1 of a commit. Specifying a glob is equivalent to specifying
 this option multiple times, one for each matching ref name.
 +
-See also the `fetch.negotiationAlgorithm` configuration variable
-documented in linkgit:git-config[1].
+See also the `fetch.negotiationAlgorithm` and `push.negotiate`
+configuration variables documented in linkgit:git-config[1], and the
+`--negotiate-only` option below.
+
+--negotiate-only::
+	Do not fetch anything from the server, and instead print the
+	ancestors of the provided `--negotiation-tip=*` arguments,
+	which we have in common with the server.
++
+Internally this is used to implement the `push.negotiate` option, see
+linkgit:git-config[1].
 
 --dry-run::
 	Show what would be done, without making any changes.
diff --git a/builtin/fetch.c b/builtin/fetch.c
@@ -1990,6 +1990,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		fetch_config_from_gitmodules(sfjc, rs);
 	}
 
+	if (negotiate_only && !negotiation_tip.nr)
+		die(_("--negotiate-only needs one or more --negotiate-tip=*"));
+
 	if (deepen_relative) {
 		if (deepen_relative < 0)
 			die(_("Negative depth in --deepen is not supported"));
diff --git a/send-pack.c b/send-pack.c
@@ -486,6 +486,12 @@ int send_pack(struct send_pack_args *args,
 	const char *push_cert_nonce = NULL;
 	struct packet_reader reader;
 
+	if (!remote_refs) {
+		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
+			"Perhaps you should specify a branch.\n");
+		return 0;
+	}
+
 	git_config_get_bool("push.negotiate", &push_negotiate);
 	if (push_negotiate)
 		get_commons_through_negotiation(args->url, remote_refs, &commons);
@@ -534,11 +540,6 @@ int send_pack(struct send_pack_args *args,
 		}
 	}
 
-	if (!remote_refs) {
-		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
-			"Perhaps you should specify a branch.\n");
-		return 0;
-	}
 	if (args->atomic && !atomic_supported)
 		die(_("the receiving end does not support --atomic push"));
 
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
@@ -599,6 +599,22 @@ setup_negotiate_only () {
 	test_commit -C client three
 }
 
+test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
+	SERVER="server" &&
+	URI="file://$(pwd)/server" &&
+
+	setup_negotiate_only "$SERVER" "$URI" &&
+
+	cat >err.expect <<-\EOF &&
+	fatal: --negotiate-only needs one or more --negotiate-tip=*
+	EOF
+
+	test_must_fail git -c protocol.version=2 -C client fetch \
+		--negotiate-only \
+		origin 2>err.actual &&
+	test_cmp err.expect err.actual
+'
+
 test_expect_success 'file:// --negotiate-only' '
 	SERVER="server" &&
 	URI="file://$(pwd)/server" &&

Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,8 @@ fetch.negotiationAlgorithm::`
`69`	`69`	`setting defaults to "skipping".`
`70`	`70`	`Unknown values will cause 'git fetch' to error out.`
`71`	`71`	`+`
`72`		-See also the `--negotiation-tip` option for linkgit:git-fetch[1].
	`72`	+See also the `--negotiate-only` and `--negotiation-tip` options to
	`73`	`+linkgit:git-fetch[1].`
`73`	`74`
`74`	`75`	`fetch.showForcedUpdates::`
`75`	`76`	Set to false to enable `--no-show-forced-updates` in
Original file line number	Diff line number	Diff line change
`@@ -1990,6 +1990,9 @@ int cmd_fetch(int argc, const char *argv, const char prefix)`
`1990`	`1990`	`fetch_config_from_gitmodules(sfjc, rs);`
`1991`	`1991`	`}`
`1992`	`1992`
	`1993`	`+ if (negotiate_only && !negotiation_tip.nr)`
	`1994`	`+ die(_("--negotiate-only needs one or more --negotiate-tip=*"));`
	`1995`	`+`
`1993`	`1996`	`if (deepen_relative) {`
`1994`	`1997`	`if (deepen_relative < 0)`
`1995`	`1998`	`die(_("Negative depth in --deepen is not supported"));`