git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brad King <brad.king@kitware.com>,
	Stefan Beller <sbeller@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Ronnie Sahlberg <ronniesahlberg@gmail.com>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 2/2] update-ref: fix "verify" command with missing <oldvalue>
Date: Thu, 11 Dec 2014 00:47:52 +0100	[thread overview]
Message-ID: <1418255272-5875-3-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1418255272-5875-1-git-send-email-mhagger@alum.mit.edu>

If "git update-ref --stdin" was given a "verify" command with no
"<newvalue>" at all (not even zeros), the code was mistakenly setting
have_old=0 (and leaving old_sha1 uninitialized). But this is
incorrect: this command is supposed to verify that the reference
doesn't exist. So in this case we really need old_sha1 to be set to
null_sha1 and have_old to be set to 1.

Moreover, since have_old was being set to zero, *no* check of the old
value was being done, so the new value of the reference was being set
unconditionally to the value in new_sha1. new_sha1, in turn, was set
to null_sha1 in the expectation that that was the old value and it
shouldn't be changed. But because the precondition was not being
checked, the result was that the reference was being deleted
unconditionally.

So, if <oldvalue> is missing, set have_old unconditionally and set
old_sha1 to null_sha1.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/update-ref.c  | 14 +++++---------
 t/t1400-update-ref.sh |  4 ++--
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6c9be05..1993529 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -282,26 +282,22 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
 	char *refname;
 	unsigned char new_sha1[20];
 	unsigned char old_sha1[20];
-	int have_old;
 
 	refname = parse_refname(input, &next);
 	if (!refname)
 		die("verify: missing <ref>");
 
 	if (parse_next_sha1(input, &next, old_sha1, "verify", refname,
-			    PARSE_SHA1_OLD)) {
-		hashclr(new_sha1);
-		have_old = 0;
-	} else {
-		hashcpy(new_sha1, old_sha1);
-		have_old = 1;
-	}
+			    PARSE_SHA1_OLD))
+		hashclr(old_sha1);
+
+	hashcpy(new_sha1, old_sha1);
 
 	if (*next != line_termination)
 		die("verify %s: extra input: %s", refname, next);
 
 	if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
-				   update_flags, have_old, msg, &err))
+				   update_flags, 1, msg, &err))
 		die("%s", err.buf);
 
 	update_flags = 0;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 6a3cdd1..6805b9e 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -655,7 +655,7 @@ test_expect_success 'stdin verify fails for mistaken null value' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'stdin verify fails for mistaken empty value' '
+test_expect_success 'stdin verify fails for mistaken empty value' '
 	M=$(git rev-parse $m) &&
 	test_when_finished "git update-ref $m $M" &&
 	git rev-parse $m >expect &&
@@ -1020,7 +1020,7 @@ test_expect_success 'stdin -z verify fails for mistaken null value' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'stdin -z verify fails for mistaken empty value' '
+test_expect_success 'stdin -z verify fails for mistaken empty value' '
 	M=$(git rev-parse $m) &&
 	test_when_finished "git update-ref $m $M" &&
 	git rev-parse $m >expect &&
-- 
2.1.3

  parent reply	other threads:[~2014-12-10 23:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10 23:47 [PATCH 0/2] Fix a bug with update-ref "verify" and no oldvalue Michael Haggerty
2014-12-10 23:47 ` [PATCH 1/2] t1400: add some more tests of "update-ref --stdin"'s verify command Michael Haggerty
2014-12-11  0:10   ` Stefan Beller
2014-12-11 16:19   ` Brad King
2014-12-10 23:47 ` Michael Haggerty [this message]
2014-12-11  0:21   ` [PATCH 2/2] update-ref: fix "verify" command with missing <oldvalue> Stefan Beller
2014-12-11 16:19   ` Brad King

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=1418255272-5875-3-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=brad.king@kitware.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.com \
    /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).