[PATCH] We do not like "HEAD" as a new branch name

[PATCH] We do not like "HEAD" as a new branch name

[Johannes Schindelin]

This makes git-check-ref-format fail for "HEAD". Since the check is only
executed when creating refs, the existing symbolic ref is safe.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	This patch was triggered by Alex' mail. Even if it should be a 
	rare case, it could catch some bad errors: Just try to run

		git checkout -b HEAD

	and imagine what a regular git user makes of the error 
	message...

 refs.c                 |    5 +++++
 t/t3300-funny-names.sh |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

04d60f0bf743f91d03e03acebf123db527cb7507
diff --git a/refs.c b/refs.c
index bda45f7..293bfe7 100644
--- a/refs.c
+++ b/refs.c
@@ -332,6 +332,11 @@ int check_ref_format(const char *ref)
 		if (ch == '.' || bad_ref_char(ch))
 			return -1;
 
+		/* do not allow "HEAD" as ref name */
+		if (ch == 'H' && (!strcmp(cp, "EAD") ||
+					!strncmp(cp, "EAD/", 4)))
+			return -1;
+
 		/* scan the rest of the path component */
 		while ((ch = *cp++) != 0) {
 			if (bad_ref_char(ch))
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 897c378..8ebd896 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -139,4 +139,6 @@ test_expect_success 'git-apply non-git d
 	 git-apply --stat | sed -e "s/|.*//" -e "s/ *\$//" >current &&
 	 diff -u expected current'
 
+test_expect_failure 'HEAD is special' 'git checkout -b HEAD'
+
 test_done
-- 
0.99.9.GIT

Re: [PATCH] We do not like "HEAD" as a new branch name

[H. Peter Anvin]

Johannes Schindelin wrote:
> This makes git-check-ref-format fail for "HEAD". Since the check is only
> executed when creating refs, the existing symbolic ref is safe.

> diff --git a/refs.c b/refs.c
> index bda45f7..293bfe7 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -332,6 +332,11 @@ int check_ref_format(const char *ref)
>  		if (ch == '.' || bad_ref_char(ch))
>  			return -1;
>  
> +		/* do not allow "HEAD" as ref name */
> +		if (ch == 'H' && (!strcmp(cp, "EAD") ||
> +					!strncmp(cp, "EAD/", 4)))
> +			return -1;
> +
>  		/* scan the rest of the path component */
>  		while ((ch = *cp++) != 0) {

If you're have to open-code it, you might want to just do it all the way:

if (ch == 'H' && cp[0] == 'E' && cp[1] == 'A' && cp[2] == 'D' &&
     (cp[3] == '\0' || cp[3] == '/'))

	-hpa

Re: [PATCH] We do not like "HEAD" as a new branch name

[Johannes Schindelin]

Hi,

On Thu, 15 Dec 2005, H. Peter Anvin wrote:

> Johannes Schindelin wrote:
> > This makes git-check-ref-format fail for "HEAD". Since the check is only
> > executed when creating refs, the existing symbolic ref is safe.
> 
> > diff --git a/refs.c b/refs.c
> > index bda45f7..293bfe7 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -332,6 +332,11 @@ int check_ref_format(const char *ref)
> >  		if (ch == '.' || bad_ref_char(ch))
> >  			return -1;
> >  +		/* do not allow "HEAD" as ref name */
> > +		if (ch == 'H' && (!strcmp(cp, "EAD") ||
> > +					!strncmp(cp, "EAD/", 4)))
> > +			return -1;
> > +
> >  		/* scan the rest of the path component */
> >  		while ((ch = *cp++) != 0) {
> 
> If you're have to open-code it, you might want to just do it all the way:
> 
> if (ch == 'H' && cp[0] == 'E' && cp[1] == 'A' && cp[2] == 'D' &&
>     (cp[3] == '\0' || cp[3] == '/'))

;-)

I don't really care. I could have used !strcmp(cp - 1, "HEAD"), just as 
well...

Ciao,
Dscho

Re: [PATCH] We do not like "HEAD" as a new branch name

[Alex Riesen]

On 12/15/05, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > If you're have to open-code it, you might want to just do it all the way:
> >
> I don't really care. I could have used !strcmp(cp - 1, "HEAD"), just as
> well...
>

...which would be more readable and greppable.

Re: [PATCH] We do not like "HEAD" as a new branch name

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +		/* do not allow "HEAD" as ref name */
> +		if (ch == 'H' && (!strcmp(cp, "EAD") ||
> +					!strncmp(cp, "EAD/", 4)))

Why forbid HEAD in the middle, like "refs/heads/HEAD/frotz"?
Confusion avoidance?

We might want to forbid anything that matches /^.*_?HEAD$/ to catch
ORIG_HEAD for example, though.

Re: [PATCH] We do not like "HEAD" as a new branch name

[Johannes Schindelin]

Hi,

On Thu, 15 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +		/* do not allow "HEAD" as ref name */
> > +		if (ch == 'H' && (!strcmp(cp, "EAD") ||
> > +					!strncmp(cp, "EAD/", 4)))
> 
> Why forbid HEAD in the middle, like "refs/heads/HEAD/frotz"?
> Confusion avoidance?

Exactly.

> We might want to forbid anything that matches /^.*_?HEAD$/ to catch
> ORIG_HEAD for example, though.

Okay, I think this is what *I* would want to be illegal:

HEAD,
ORIG_HEAD,
FETCH_HEAD
MERGE_HEAD

Others? Or should I really test for just *anything* ending in _HEAD 
besides HEAD itself?

Ciao,
Dscho

Re: [PATCH] We do not like "HEAD" as a new branch name

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, I think this is what *I* would want to be illegal:
>
> HEAD,
> ORIG_HEAD,
> FETCH_HEAD
> MERGE_HEAD
>
> Others? Or should I really test for just *anything* ending in _HEAD 
> besides HEAD itself?

Just to futureproof ourselves, how about this?

	/^(?:.*/)?(?:[A-Z0-9]+_)?HEAD$/

The last path component being "HEAD" or capital-or-digit
followed by underscore followed by "HEAD".

Re: [PATCH] We do not like "HEAD" as a new branch name

[Johannes Schindelin]

Hi,

On Thu, 15 Dec 2005, Junio C Hamano wrote:

> Just to futureproof ourselves, how about this?
> 
> 	/^(?:.*/)?(?:[A-Z0-9]+_)?HEAD$/
> 
> The last path component being "HEAD" or capital-or-digit
> followed by underscore followed by "HEAD".

Okay. Why not go the full nine yards, and not be difficult about it:

diff --git a/refs.c b/refs.c
--- a/refs.c
+++ b/refs.c
@@ -345,6 +345,11 @@ int check_ref_format(const char *ref)
 		if (!ch) {
 			if (level < 2)
 				return -1; /* at least of form "heads/blah" */
+
+			/* do not allow ref name to end in "HEAD" */
+			if (cp - ref > 4 && !strcmp(cp - 4, "HEAD"))
+				return -1;
+
 			return 0;
 		}
 	}

Ciao,
Dscho

Re: [PATCH] We do not like "HEAD" as a new branch name

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay. Why not go the full nine yards, and not be difficult about it:

That's fine.  There is an off-by-one in your code, though.
I've committed it with a tweak.

Re: [PATCH] We do not like "HEAD" as a new branch name

[Johannes Schindelin]

Hi,

On Thu, 15 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Okay. Why not go the full nine yards, and not be difficult about it:
> 
> That's fine.  There is an off-by-one in your code, though.
> I've committed it with a tweak.

Thanks. I did test it (quickly), and t3300 said it was fine. Strange.

Ciao,
Dscho