Subject: Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

From: David Turner <[email-masked]>
Date: Thu, 28 Apr 2016 13:44:09 -0400

On Wed, 2016-04-27 at 16:37 -0400, Jeff King wrote:
> On Wed, Apr 27, 2016 at 04:34:53PM -0400, David Turner wrote:
> 
> > > I thought the point is that one is a lesser check than the other,
> > > and
> > > we
> > > need different rules for different situations. So we might allow
> > > deletion on a refname that does not pass check_refname_format(),
> > > but
> > > we
> > > must make sure it is not going to cause any mischief (e.g.,
> > > escaping
> > > ".git" and deleting random files).
> > > 
> > > But anything writing a _new_ refname (whether the actual ref, or
> > > referencing it via a symref) should be using
> > > check_refname_format()
> > > before writing.
> > 
> > Unfortunately, neither check is lesser -- refname_is_safe allows
> > refs/heads//foo but not a/b while check_refname_format allows a/b
> > but
> > not refs/heads//foo.  So sometimes we need both, while other times
> > we
> > just need one :(
> 
> IMHO, that sounds like a bug. check_refname_format() should
> conceptually[1] be a superset of refname_is_safe(). Is there a case
> where we would want to _allow_ a refname that is not safe to look at
> on
> disk?

The only such case I can think of is the case where there is a symref
to such a bad refname, and we want to delete said symref.