git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Davide Berardi <davide.berardi6@unibo.it>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: [PATCH v2] clone: Don't segfault on -b specifing a non-commit
Date: Sun, 3 Nov 2019 18:07:18 +0000	[thread overview]
Message-ID: <20191103180716.GA72007@carpenter.lan> (raw)
In-Reply-To: <20191101002432.GA49846@carpenter.lan>

The code in "git clone -b B" to decide what branch to check out
assumed that B points at a commit object without checking,
leading to dereferencing a NULL pointer and causing a segmentation
fault.

Just aborting the operation when it happens is not a very
attractive option because we would be left with a directory
without .git/HEAD that cannot be used as a valid repository the
user can attempt to recover from the situation by checking out
something.

Fall back to use the 'master' branch, which is what we use when
the command without the "-b" option cannot figure out what
branch the remote side points with its HEAD.

Signed-off-by: Davide Berardi <berardi.dav@gmail.com>
---
 builtin/clone.c         | 49 ++++++++++++++++++++++++++++++++++++-----
 commit.c                | 16 +++++++++++---
 commit.h                |  4 ++++
 t/t5609-clone-branch.sh | 16 +++++++++++++-
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f665b28ccc..f185b7f193 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -704,10 +704,46 @@ static void update_remote_refs(const struct ref *refs,
 	}
 }
 
-static void update_head(const struct ref *our, const struct ref *remote,
+static struct commit *lookup_commit_helper(const struct ref *our,
+					   const struct ref *remote,
+					   const char *msg, int *err)
+{
+	const struct object_id *tip = NULL;
+	struct commit *tip_commit = NULL;
+
+	if (our)
+		tip = &our->old_oid;
+	else if (remote)
+		tip = &remote->old_oid;
+
+	if (!tip)
+		return NULL;
+
+	tip_commit = lookup_commit_reference_gently_err(the_repository, tip, 1, err);
+	if (!tip_commit) {
+		/*
+		 * The given non-commit cannot be checked out,
+		 * so have a 'master' branch and leave it unborn.
+		 */
+		warning(_("non-commit cannot be checked out"));
+		create_symref("HEAD", "refs/heads/master", msg);
+		return NULL;
+	}
+
+	return tip_commit;
+}
+
+static int update_head(const struct ref *our, const struct ref *remote,
 			const char *msg)
 {
+	int err = 0;
 	const char *head;
+	struct commit *c = NULL;
+
+	c = lookup_commit_helper(our, remote, msg, &err);
+	if (err)
+		return -1;
+
 	if (our && skip_prefix(our->name, "refs/heads/", &head)) {
 		/* Local default branch link */
 		if (create_symref("HEAD", our->name, NULL) < 0)
@@ -718,8 +754,6 @@ static void update_head(const struct ref *our, const struct ref *remote,
 			install_branch_config(0, head, option_origin, our->name);
 		}
 	} else if (our) {
-		struct commit *c = lookup_commit_reference(the_repository,
-							   &our->old_oid);
 		/* --branch specifies a non-branch (i.e. tags), detach HEAD */
 		update_ref(msg, "HEAD", &c->object.oid, NULL, REF_NO_DEREF,
 			   UPDATE_REFS_DIE_ON_ERR);
@@ -732,6 +766,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
 		update_ref(msg, "HEAD", &remote->old_oid, NULL, REF_NO_DEREF,
 			   UPDATE_REFS_DIE_ON_ERR);
 	}
+	return err;
 }
 
 static int checkout(int submodule_progress)
@@ -1249,7 +1284,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			   branch_top.buf, reflog_msg.buf, transport,
 			   !is_local, filter_options.choice);
 
-	update_head(our_head_points_at, remote_head, reflog_msg.buf);
+	err = update_head(our_head_points_at, remote_head, reflog_msg.buf) < 0;
 
 	/*
 	 * We want to show progress for recursive submodule clones iff
@@ -1268,8 +1303,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 
 	junk_mode = JUNK_LEAVE_REPO;
-	fetch_if_missing = 1;
-	err = checkout(submodule_progress);
+	if (!err) {
+		fetch_if_missing = 1;
+		err = checkout(submodule_progress);
+	}
 
 	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_top);
diff --git a/commit.c b/commit.c
index a98de16e3d..ffb5230f0f 100644
--- a/commit.c
+++ b/commit.c
@@ -26,16 +26,26 @@ int save_commit_buffer = 1;
 
 const char *commit_type = "commit";
 
-struct commit *lookup_commit_reference_gently(struct repository *r,
-		const struct object_id *oid, int quiet)
+struct commit *lookup_commit_reference_gently_err(struct repository *r,
+		const struct object_id *oid, int quiet, int *err)
 {
+	struct commit *retval;
 	struct object *obj = deref_tag(r,
 				       parse_object(r, oid),
 				       NULL, 0);
 
 	if (!obj)
 		return NULL;
-	return object_as_type(r, obj, OBJ_COMMIT, quiet);
+	retval = object_as_type(r, obj, OBJ_COMMIT, quiet);
+	if (!retval && err)
+		*err = 1;
+	return retval;
+}
+
+struct commit *lookup_commit_reference_gently(struct repository *r,
+		const struct object_id *oid, int quiet)
+{
+	return lookup_commit_reference_gently_err(r, oid, quiet, NULL);
 }
 
 struct commit *lookup_commit_reference(struct repository *r, const struct object_id *oid)
diff --git a/commit.h b/commit.h
index f5295ca7f3..157c5dc526 100644
--- a/commit.h
+++ b/commit.h
@@ -70,6 +70,10 @@ struct commit *lookup_commit_reference(struct repository *r,
 struct commit *lookup_commit_reference_gently(struct repository *r,
 					      const struct object_id *oid,
 					      int quiet);
+struct commit *lookup_commit_reference_gently_err(struct repository *r,
+						  const struct object_id *oid,
+						  int quiet,
+						  int *err);
 struct commit *lookup_commit_reference_by_name(const char *name);
 
 /*
diff --git a/t/t5609-clone-branch.sh b/t/t5609-clone-branch.sh
index 6e7a7be052..d57f750eeb 100755
--- a/t/t5609-clone-branch.sh
+++ b/t/t5609-clone-branch.sh
@@ -20,7 +20,10 @@ test_expect_success 'setup' '
 	 echo one >file && git add file && git commit -m one &&
 	 git checkout -b two &&
 	 echo two >file && git add file && git commit -m two &&
-	 git checkout master) &&
+	 git checkout master &&
+	 blob=$(git rev-parse HEAD:file)  &&
+	 echo $blob > .git/refs/heads/broken-tag &&
+	 echo $blob > .git/refs/heads/broken-head) &&
 	mkdir empty &&
 	(cd empty && git init)
 '
@@ -67,4 +70,15 @@ test_expect_success 'clone -b not allowed with empty repos' '
 	test_must_fail git clone -b branch empty clone-branch-empty
 '
 
+test_expect_success 'clone -b with a non-commit tag must fallback' '
+	test_must_fail git clone -b broken-tag parent clone-broken-tag &&
+	(cd clone-broken-tag &&
+	 check_HEAD master)
+'
+test_expect_success 'clone -b with a non-commit head must fallback' '
+	test_must_fail git clone -b broken-head parent clone-broken-head &&
+	(cd clone-broken-head &&
+	 check_HEAD master)
+'
+
 test_done
-- 
2.22.0


  parent reply	other threads:[~2019-11-03 18:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01  0:24 [PATCH] Segmentation Fault on non-commit --branch clone Davide Berardi
2019-11-01 19:08 ` Johannes Schindelin
2019-11-01 19:35   ` Jeff King
2019-11-02 10:16     ` Junio C Hamano
2019-11-03 18:16       ` Davide Berardi
2019-11-04  3:55         ` Junio C Hamano
2019-11-02  9:18   ` Junio C Hamano
2019-11-01 19:43 ` Jeff King
2019-11-02 10:07 ` Junio C Hamano
2019-11-03 18:07 ` Davide Berardi [this message]
2019-11-05  4:37   ` [PATCH v2] clone: Don't segfault on -b specifing a non-commit Jeff King
2019-11-06  1:36     ` Junio C Hamano
2019-11-06  4:05       ` Jeff King
2019-11-06  9:53         ` 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=20191103180716.GA72007@carpenter.lan \
    --to=davide.berardi6@unibo.it \
    --cc=git@vger.kernel.org \
    --subject='Re: [PATCH v2] clone: Don'\''t segfault on -b specifing a non-commit' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox