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

From: Junio C Hamano <[email-masked]>
Date: Wed, 27 Apr 2016 10:59:53 -0700

Michael Haggerty <[email-masked]> writes:

> Does anybody have a use case for allowing un-normalized reference
> names like "refs/foo/../bar///baz"? I'm pretty certain they would not
> be handled correctly anyway, especially if they are not stored as
> loose references.

I wondered what codepath this follows:

    $ git rev-parse mhagger/wip//split-under-lock

get_sha1_basic() calls to dwim_ref() to learn real_ref and its
value.

dwim_ref() repeatedly gives the string to a known dwim rule, and the
"refs/remotes/%.*s" rule is expected to find that the user meant to
name "refs/remotes/mhagger/wip//split-under-lock".

This, still with doubled slash, is passed to resolve_ref_unsafe(),
which has a call to !refname_is_safe(refname) to reject the request.

So I think this will move the rejection early in the codepath than
how we reject the ref with doubled slash in the current code, but
the end result would be the same for this example.  The same is true
for heads//master or refs/heads//master.

There is another call to refname_is_safe() in resolve_ref_unsafe(),
which applies the sanity check to the string from a symref.  We seem
to allow

    $ git symbolic-ref refs/heads/SSS refs/heads//master

and we end up storing "ref: refs/heads//master" (or make a symbolic
link with doubled slashes), but the current code considers the
resulting symbolic link as "dangling".  Again, this change moves the
rejection a bit earlier in the codepath, without changing the end
result, I'd think.