[PATCH] update-ref: fail create operation over stdin if ref already exists

From: Aman Gupta <redacted>

Signed-off-by: Aman Gupta <redacted>
Signed-off-by: Michael Haggerty <redacted>
---
My colleague Aman ran across this bug and wrote the fix.  I didn't
notice this bug, but I just verified that it is also fixed by my
mh/ref-transaction patch series (albeit without a test case).

Because the bug could cause somebody to overwrite a reference
unintentionally, I propose that we apply this unintrusive fix to
maint.  When mh/ref-transaction makes it to a release, the bug will
continue to be fixed, but in a different way.

 builtin/update-ref.c  |  1 +
 t/t1400-update-ref.sh | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 1292cfe..5c208bb 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c

@@ -147,6 +147,7 @@ static void parse_cmd_create(const char *next)
 	struct ref_update *update;
 
 	update = update_alloc();
+	update->have_old = 1;
 
 	if ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])
 		update_store_ref_name(update, ref.buf);

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 6ffd82f..e130c52 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh

@@ -820,7 +820,18 @@ test_expect_success 'stdin -z update ref fails with bad old value' '
 	test_must_fail git rev-parse --verify -q $c
 '
 
+test_expect_success 'stdin -z create ref fails when ref exists' '
+	git update-ref $c $m &&
+	git rev-parse "$c" >expect &&
+	printf $F "create $c" "$m~1" >stdin &&
+	test_must_fail git update-ref -z --stdin <stdin 2>err &&
+	grep "fatal: Cannot lock the ref '"'"'$c'"'"'" err &&
+	git rev-parse "$c" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'stdin -z create ref fails with bad new value' '
+	git update-ref -d "$c" &&
 	printf $F "create $c" "does-not-exist" >stdin &&
 	test_must_fail git update-ref -z --stdin <stdin 2>err &&
 	grep "fatal: invalid new value for ref $c: does-not-exist" err &&

-- 
1.9.0

On 04/02/2014 04:09 AM, Michael Haggerty wrote:

From: Aman Gupta <redacted>

[snip]

Looks good.

+test_expect_success 'stdin -z create ref fails when ref exists' '

Strictly speaking we should have a non-z mode test too.

Thanks,
-Brad

On 04/02/2014 02:57 PM, Brad King wrote:

On 04/02/2014 04:09 AM, Michael Haggerty wrote:

quoted

From: Aman Gupta <redacted>

[snip]

quoted

@@ -147,6 +147,7 @@ static void parse_cmd_create(const char *next)
 	struct ref_update *update;
 
 	update = update_alloc();
+	update->have_old = 1;

Looks good.

quoted

+test_expect_success 'stdin -z create ref fails when ref exists' '

Strictly speaking we should have a non-z mode test too.

This code path is used regardless of whether -z is used, so I don't
think it is necessary to duplicate the test.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/