threads / patch / 2852

patchWe do not like "HEAD" as a new branch name

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

## tl;dr

10 messages between Dec 15, 2005 and Dec 16, 2005. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Johannes Schindelin· Dec 15, 2005, 11:46 UTC · lore

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
Show changes to 2 files +7 −0

refs.c, t/t3300-funny-names.sh

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
H. Peter Anvin· Dec 15, 2005, 16:27 UTC · re: Johannes Schindelin · lore

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

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.
Show 15 quoted lines
> 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
Johannes Schindelin· Dec 15, 2005, 16:40 UTC · re: H. Peter Anvin · lore

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

Hi,
On Thu, 15 Dec 2005, H. Peter Anvin wrote:
Show 23 quoted lines
> 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

Alex Riesen· Dec 15, 2005, 17:00 UTC · re: Johannes Schindelin · lore

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

On 12/15/05, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 6 quoted lines
> >
> > 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.
Junio C Hamano· Dec 15, 2005, 19:34 UTC · re: Johannes Schindelin · lore

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

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.

Johannes Schindelin· Dec 15, 2005, 22:41 UTC · re: Junio C Hamano · lore

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

Hi,
On Thu, 15 Dec 2005, Junio C Hamano wrote:
Show 8 quoted lines
> 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

Junio C Hamano· Dec 15, 2005, 23:38 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 9 quoted lines
> 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".

Johannes Schindelin· Dec 16, 2005, 01:40 UTC · re: Junio C Hamano · lore

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

Hi,
On Thu, 15 Dec 2005, Junio C Hamano wrote:
Show 6 quoted lines
> 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:
Show changes to refs.c +5 −0
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
Junio C Hamano· Dec 16, 2005, 06:34 UTC · re: Johannes Schindelin · lore

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

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.

Johannes Schindelin· Dec 16, 2005, 11:32 UTC · re: Junio C Hamano · lore

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

Hi,
On Thu, 15 Dec 2005, Junio C Hamano wrote:
Show 6 quoted lines
> 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

← back to recent threads