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

[Jeff King <peff@peff.net>]

On Wed, Apr 27, 2016 at 04:37:28PM -0400, Jeff King wrote:

> > > 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?

BTW, if there isn't a superset relationship here, then I suspect I may
have introduced some loosening or inconsistencies in 03afcbe
(read_packed_refs: avoid double-checking sane refs, 2015-04-16). So any
tightening now may simply be fixing that (which doesn't change anything
with respect to correctness, but may give people more confidence that
the tightening is not breaking something people have been depending on).

-Peff

PS Reading over that commit message, I think it mis-states
   "refname_is_safe() can only be true if check_refname_format() also
   failed". It should be "!refname_is_safe() ...". I.e., the condition can
   only trigger if...