git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Clemens Fruhwirth" <clemens@endorphin.org>,
	"Jan Pokorný" <poki@fnusa.cz>,
	"Corentin BOMPARD" <corentin.bompard@etu.univ-lyon1.fr>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] pull, fetch: fix segfault in --set-upstream option
Date: Mon, 19 Jul 2021 16:30:51 +0200	[thread overview]
Message-ID: <patch-1.1-2d8f3e59e1f-20210719T142808Z-avarab@gmail.com> (raw)
In-Reply-To: <8636b96be256b47d207e543995abbecde9ca5055.camel@fnusa.cz>

Fix a segfault in the --set-upstream option added in
24bc1a12926 (pull, fetch: add --set-upstream option, 2019-08-19) added
in v2.24.0.

The code added there did not do the same checking we do for "git
branch" itself since 8efb8899cfe (branch: segfault fixes and
validation, 2013-02-23), which in turn fixed the same sort of segfault
I'm fixing now in "git branch --set-upstream-to", see
6183d826ba6 (branch: introduce --set-upstream-to, 2012-08-20).

The error message I'm adding here is an amalgamation of the error
added for "git branch" in 8efb8899cfe, and the error output
install_branch_config() itself emits, i.e. it trims "refs/heads/" from
the name and says "branch X on remote", not "branch refs/heads/X on
remote".

Reported-by: Clemens Fruhwirth <clemens@endorphin.org>
Reported-by: Jan Pokorný <poki@fnusa.cz>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Mon, Jul 19 2021, Jan Pokorný wrote:

> Hello,
>
> Clemens Fruhwirth píše v Po 05. 07. 2021 v 17:46 +0200:
>> What did you do before the bug happened? (Steps to reproduce your
>> issue)
>> 
>> Run "git pull origin nixos-unstable --rebase --set-upstream"
>> on a repo that had no branch set, e.g. when running "git branch" gave
>> "* (no branch)"
>> 

Thanks for the report both & sorry that this fell through the cracks
for so long.

The bug itself seems rather obvious given the benefit of that
reproduction scenario & backtrace, as noted above we've been playing
whack-a-mole with a related segfault since 2012.

It would be nice if you could test this patch and confirm, but given
the trivality of the segfault it shouldn't be necessary, I'm confident
that this fixes the bug you two reported. Thanks both!

 builtin/fetch.c         | 11 +++++++++++
 t/t5553-set-upstream.sh | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9191620e50c..c2eac8f15e5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1623,6 +1623,17 @@ static int do_fetch(struct transport *transport,
 			}
 		}
 		if (source_ref) {
+			if (!branch) {
+				const char *shortname = NULL;
+				if (!skip_prefix(source_ref->name,
+						 "refs/heads/", &shortname))
+					shortname = source_ref->name;
+				    
+				die(_("could not set upstream of HEAD to '%s' from '%s' when "
+				      "it does not point to any branch."),
+				    shortname, transport->remote->name);
+			}
+
 			if (!strcmp(source_ref->name, "HEAD") ||
 			    starts_with(source_ref->name, "refs/heads/"))
 				install_branch_config(0,
diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
index b1d614ce18c..ae90554645e 100755
--- a/t/t5553-set-upstream.sh
+++ b/t/t5553-set-upstream.sh
@@ -91,6 +91,17 @@ test_expect_success 'fetch --set-upstream with valid URL sets upstream to URL' '
 	check_config_missing other2
 '
 
+test_expect_success 'fetch --set-upstream with a detached HEAD' '
+	git checkout HEAD^0 &&
+	test_when_finished "git checkout -" &&
+	cat >expect <<-\EOF &&
+	fatal: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+	EOF
+	test_must_fail git fetch --set-upstream upstream main 2>actual.raw &&
+	grep ^fatal: actual.raw >actual &&
+	test_cmp expect actual
+'
+
 # tests for pull --set-upstream
 
 test_expect_success 'setup bare parent pull' '
@@ -178,4 +189,15 @@ test_expect_success 'pull --set-upstream with valid URL and branch sets branch'
 	check_config_missing other2
 '
 
+test_expect_success 'pull --set-upstream with a detached HEAD' '
+	git checkout HEAD^0 &&
+	test_when_finished "git checkout -" &&
+	cat >expect <<-\EOF &&
+	fatal: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+	EOF
+	test_must_fail git pull --set-upstream upstream main 2>actual.raw &&
+	grep ^fatal: actual.raw >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.32.0.874.ge7a9d58bfcf


  reply	other threads:[~2021-07-19 14:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 15:46 git pull --set-upstream segfaults on branchless repo Clemens Fruhwirth
2021-07-19 10:04 ` Jan Pokorný
2021-07-19 14:30   ` Ævar Arnfjörð Bjarmason [this message]
2021-07-19 15:17     ` [PATCH] pull, fetch: fix segfault in --set-upstream option Junio C Hamano
2021-08-23 12:56     ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2021-08-24  7:30       ` Clemens Fruhwirth
2021-08-24  8:49         ` Ævar Arnfjörð Bjarmason
2021-08-30 14:41       ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2021-08-30 17:46         ` Junio C Hamano
2021-08-31 13:58         ` [PATCH v4] " Ævar Arnfjörð Bjarmason
2021-08-31 16:40           ` Junio C Hamano
2021-08-31 20:20             ` Ævar Arnfjörð Bjarmason
2021-09-01 17:44               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-1.1-2d8f3e59e1f-20210719T142808Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=clemens@endorphin.org \
    --cc=corentin.bompard@etu.univ-lyon1.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=poki@fnusa.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).