Re: [PATCH 0/3] object-name: don't allow @ as a branch name

[shejialuo <shejialuo@gmail.com>]

On Tue, Oct 08, 2024 at 04:19:10PM +0200, Kristoffer Haugsbakk wrote:
> On Tue, Oct 8, 2024, at 15:19, shejialuo wrote:
> > On Mon, Oct 07, 2024 at 10:15:16PM +0200, Kristoffer Haugsbakk wrote:
> >
> > [snip]
> >
> >>   §2 Disallow `HEAD` as a branch name
> >> 
> >> This was done later in 2017:
> >> 
> >> https://lore.kernel.org/git/20171114114259.8937-1-kaartic.sivaraam@gmail.com/
> >> 
> >>   §2 `refs/heads/@` is apparently disallowed by git-refs(1)
> >> 
> >> See `t/t1508-at-combinations.sh`:
> >> 
> >> ```
> >> error: refs/heads/@: badRefName: invalid refname format
> >> ```
> >> 
> >
> > It's true that using "git refs verify" will report "refs/heads/@" is a
> > bad refname.
> >
> > From the man page of the "git-check-ref-format(1)", it is clear that
> >
> >     9. They cannot be the single character @.
> >
> > Because I am interesting in this patch which is highly relevant with my
> > recent work, so I try somethings here and find some interesting results
> > as below shows.
> >
> >     $ git check-ref-format refs/heads/@
> >     $ echo $? # will be 0
> >     # git check-ref-format --allow-onelevel @
> >     # echo $? # will be 1
> >
> > The reason why "git refs verify" will report this error is that in the
> > code implementation, I have to iterate every file in the filesystem. So
> > it's convenient for me to do the following:
> >
> >     if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
> >         ret = fsck_report(...);
> >     }
> >
> > Because I specify "REFNAME_ALLOW_ONELEVEL" here, so it will follow the
> > "git check-ref-format --allow-onelevel" command thus reporting an error
> > to the user.
> >
> > I am curious why "git check-ref-format refs/heads/@" will succeed, so I
> > try to use "git symbolic-ref" and "git update-ref" to verify to test the
> > behavior.
> >
> >     $ git symbolic-ref refs/heads/@ refs/heads/master
> >     error: cannot lock ref 'refs/heads/@': unable to resolve reference 
> > 'refs/heads/@': reference broken
> >     $ git update-ref refs/heads/@ refs/heads/master
> >     fatal: update_ref failed for ref 'refs/heads/@': cannot lock ref 
> > 'refs/heads/@': unable to resolve reference 'refs/heads/@': reference 
> > broken
> >
> > So, we are not consistent here. I guess the reason why "git
> > check-ref-format refs/heads/@" will succeed is that we allow user create
> > this kind of branch.
> >
> > If we decide to not allow user to create such refs. We should also
> > change the behavior of the "check_refname_format" function. (I am not
> > familiar with the internal implementation, this is my guess)
> >
> > Thanks,
> > Jialuo
> 
> Thanks for the careful analysis.

Please ignore the above analysis which is not true. (Today I am writing
code for my work). Currently, we truly allow "refs/heads/@" as the refname.
And also for "git check-ref-format", "git update-ref" and "git symbolic-ref"

When I did the experiments above, I forgot to clear the state which
makes the "git update-ref" and "git symbolic-ref" fail. So, there are
some faults in "git refs verify". I will fix in my current work.

So, if we decide to not allow "refs/heads/@", we should also update "git
check-ref-format", "git update-ref" and "git symbolic-ref" to align with
this behavior.

Thanks,
Jialuo