Re: [PATCH 3/3] branch: forbid refs/heads/HEAD

From: Jeff King <peff@peff.net>
Date: Oct 13, 2017, 13:15 UTC
Message-ID: <20171013131501.3qarwhanktfvgjqd@sigill.intra.peff.net>
In-Reply-To: <20171013051132.3973-4-gitster@pobox.com>

On Fri, Oct 13, 2017 at 02:11:32PM +0900, Junio C Hamano wrote:

Show 7 quoted lines

> 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 '-'.
> 
> Use it to also reject "HEAD" as a branch name.

Heh, I just pointed somebody to this a day or two ago as #leftoverbit. I guess it's taken now. :)

Related to this: should we do a better job of confirming that the refname is available for use?

If you do:

  git branch foo
  git checkout -b foo/bar

then "foo/bar" is not available. And for "checkout -b", we'd notice when we tried to create the ref. But for:

  git checkout --orphan foo/bar

we'd update HEAD with a non-viable name, and only find out later during "git commit". That's not the end of the world, but it might be nice to complain when writing the symlink.

Largely orthogonal to the problem you're solving here, but I suspect it may touch the same code, so it might be worth thinking about while we're here.

Show 15 quoted lines

> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index e88349c8a0..3ecb2eab0c 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -331,4 +331,12 @@ 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_expect_success 'checkout -b rejects HEAD as a branch name' '
> +	test_must_fail git checkout -B HEAD HEAD^
> +'

Should we test that:

  git update-ref refs/heads/HEAD HEAD^

continues to work?

-Peff