[BUG] git branch --merged $unknown_checksum segfaults

Hi,

I hope this minor buglet is not known already..
I did not look further.

Seen with
$ dpkg-query -W git
git	1:1.7.2.5-3
and
git	1:1.7.9-1
cheers,

git init
git branch --merged 0000000000000000000000000000000000000000
Segmentation fault
touch f
git commit -m one f
[master (root-commit) f11ad60] one
 0 files changed, 0 insertions(+), 0 deletions(-)
  create mode 100644 f
git branch --merged $((echo "obase=16;ibase=16;$(git log -n1 --format='%H'| tr '[[:lower:]]' '[[:upper:]]')+1") | bc)
Segmentation fault

print_ref_list looks up the merge_filter_ref and assumes that a valid
pointer is returned. When the object doesn't exist, it tries to
dereference a NULL pointer. This can be the case when git branch
--merged is given an argument that isn't a valid commit name.

Check whether the lookup returns a NULL pointer and die with an error
if it does. Add a test, while we're at it.

Signed-off-by: Carlos Martín Nieto <redacted>
---

It certainly looks like --merged was only ever supposed to be used
with branch names, as it assumed that get_sha1() would catch the
errors.

I'm not sure if "bad object" or "invalid object" fits better. "bad
object" might have a stronger implication that it exists but is
corrupt.

 builtin/branch.c  |    3 +++
 t/t3200-branch.sh |    4 ++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index cb17bc3..b63d5fe 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -530,6 +530,9 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 	if (merge_filter != NO_FILTER) {
 		struct commit *filter;
 		filter = lookup_commit_reference_gently(merge_filter_ref, 0);
+		if (!filter)
+			die("bad object %s", sha1_to_hex(merge_filter_ref));
+
 		filter->object.flags |= UNINTERESTING;
 		add_pending_object(&ref_list.revs,
 				   (struct object *) filter, "");

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index dd1aceb..9fe1d8f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -653,4 +653,8 @@ test_expect_success 'refuse --edit-description on unborn branch for now' '
 	)
 '
 
+test_expect_success '--merged catches invalid object names' '
+	test_must_fail git branch --merged 0000000000000000000000000000000000000000
+'
+
 test_done

-- 
1.7.9.rc0

On Mon, Feb 27, 2012 at 04:11:53PM +0100, Carlos Martín Nieto wrote:

print_ref_list looks up the merge_filter_ref and assumes that a valid
pointer is returned. When the object doesn't exist, it tries to
dereference a NULL pointer. This can be the case when git branch
--merged is given an argument that isn't a valid commit name.

Check whether the lookup returns a NULL pointer and die with an error
if it does. Add a test, while we're at it.

Signed-off-by: Carlos Martín Nieto <redacted>
---

It certainly looks like --merged was only ever supposed to be used
with branch names, as it assumed that get_sha1() would catch the
errors.

I'm not sure if "bad object" or "invalid object" fits better. "bad
object" might have a stronger implication that it exists but is
corrupt.

You would also get NULL if the object exists but is not a commit. Maybe:

  die("object '%s' does not point to a commit", ...)

would be better? It covers the wrong-type case, and is still technically
true when the object does not exist.

-Peff