branch: correctly reject refs/heads/{-dash,HEAD}

strbuf_check_branch_ref() is the central place where many codepaths
see if a proposed name is suitable for the name of a branch.  It was
designed to allow us to get stricter than the check_refname_format()
check used for refnames in general, and we already use it to reject
a branch whose name begins with a '-'.  The function gets a strbuf
and a string "name", and returns non-zero if the name is not
appropriate as the name for a branch.  When the name is good, it
places the full refname for the branch with the proposed name in the
strbuf before it returns.

However, it turns out that one caller looks at what is in the strbuf
even when the function returns an error.  Make the function populate
the strbuf even when it returns an error.  That way, when "-dash" is
given as name, "refs/heads/-dash" is placed in the strbuf when
returning an error to copy_or_rename_branch(), which notices that
the user is trying to recover with "git branch -m -- -dash dash" to
rename "-dash" to "dash".

While at it, use the same mechanism to also reject "HEAD" as a
branch name.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index c7c5ab376ccb56b3a4f4533f61293bd543ea8097..67961d6e47b67eac4372cbe5ffb37fa3d97c5861 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1332,9 +1332,19 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
        strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
-       if (name[0] == '-')
-               return -1;
+
+       /*
+        * This splice must be done even if we end up rejecting the
+        * name; builtin/branch.c::copy_or_rename_branch() still wants
+        * to see what the name expanded to so that "branch -m" can be
+        * used as a tool to correct earlier mistakes.
+        */
        strbuf_splice(sb, 0, 0, "refs/heads/", 11);
+
+       if (*name == '-' ||
+           !strcmp(sb->buf, "refs/heads/HEAD"))
+               return -1;
+
        return check_refname_format(sb->buf, 0);
 }
 

diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index e88349c8a0483b97ea38013de61d799003fd85b5..c7878a60edfdcf24f3c45367f58e62b0ce6dab71 100755 (executable)
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -331,4 +331,47 @@ test_expect_success 'update-ref --stdin -z fails delete with bad ref name' '
        grep "fatal: invalid ref format: ~a" err
 '
 
+test_expect_success 'branch rejects HEAD as a branch name' '
+       test_must_fail git branch HEAD HEAD^ &&
+       test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'checkout -b rejects HEAD as a branch name' '
+       test_must_fail git checkout -B HEAD HEAD^ &&
+       test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'update-ref can operate on refs/heads/HEAD' '
+       git update-ref refs/heads/HEAD HEAD^ &&
+       git show-ref refs/heads/HEAD &&
+       git update-ref -d refs/heads/HEAD &&
+       test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'branch -d can remove refs/heads/HEAD' '
+       git update-ref refs/heads/HEAD HEAD^ &&
+       git branch -d HEAD &&
+       test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'branch -m can rename refs/heads/HEAD' '
+       git update-ref refs/heads/HEAD HEAD^ &&
+       git branch -m HEAD tail &&
+       test_must_fail git show-ref refs/heads/HEAD &&
+       git show-ref refs/heads/tail
+'
+
+test_expect_success 'branch -d can remove refs/heads/-dash' '
+       git update-ref refs/heads/-dash HEAD^ &&
+       git branch -d -- -dash &&
+       test_must_fail git show-ref refs/heads/-dash
+'
+
+test_expect_success 'branch -m can rename refs/heads/-dash' '
+       git update-ref refs/heads/-dash HEAD^ &&
+       git branch -m -- -dash dash &&
+       test_must_fail git show-ref refs/heads/-dash &&
+       git show-ref refs/heads/dash
+'
+
 test_done