Re: [PATCH 0/3] object-name: don't allow @ as a branch name
- From
shejialuo <shejialuo@gmail.com>
- Date
- Oct 18, 2024, 14:21 UTC
- Message-ID
- <ZxJu13wpSiJ_kDdB@ArchLinux>
- In-Reply-To
- <3af78a3c-afb9-4ce7-aea0-a5bbddd4f34a@app.fastmail.com>
On Tue, Oct 08, 2024 at 04:19:10PM +0200, Kristoffer Haugsbakk wrote:
Show 72 quoted lines
> 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