Re: Buggy handling of non-canonical ref names

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 24, 2011, 22:27 UTC
Message-ID: <7vaaayo369.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vaaayps9z.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 15 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> What is the policy about reference names and their canonicalization?
>
> The overall policy has been that we care about well-formed input, and
> everything else is "undefined", even though as you found out some of them
> try to work sensibly.
>
>>     $ git check-ref-format /foo/bar ; echo $?
>>     0
>>
>>     $ git check-ref-format --print /foo/bar
>>     /foo/bar
>
> I think these are bogus. Patches welcome.

I actually think the former is correct and the latter should strip the leading slash. Essentially what "check-ref-format" (and the underlying check_ref_format() function) validates is if the given user string can be used under $GIT_DIR/refs/ to name a ref, and $GIT_DIR/refs//foo/bar is (because we "tolerate duplicated slashes" cf. comment in the function) the same as $GIT_DIR/refs/foo/bar and is allowed.

I do not know what the original motivation behind --print was, but it seems to want to show it canonicalized, so it should strip the leading slash as well.

I think what is missing is a unified way to canonicalize the refnames (which led to the inconsistencies you observed), and I strongly suspect that check_ref_format() should learn to return the canonicalized format (if asked by the caller) and the caller should use the canonicalized version after feeding end-user input to it.

Then the plumbing "check-ref-format --print" can use it just like any other caller that should be (or already are) using check_ref_format() to validate the end-user input.

Yes, such a change will update the overall policy I stated earlier and narrow the scope of "undefined" down a bit, by uniformly codifying that leading and duplicate slashes are removed to be nice to the user.