Re: Buggy handling of non-canonical ref names

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Aug 25, 2011, 07:54 UTC
Message-ID: <4E55FFAA.9030904@alum.mit.edu>
In-Reply-To: <7vaaayo369.fsf@alter.siamese.dyndns.org>

On 08/25/2011 12:27 AM, Junio C Hamano wrote:

Show 22 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>> 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 can live with either way, but I should point out that such an extra slash can be problematic when used naively in conjunction with Python's standard glue-together-pathname function, os.path.join() [1]:

    $ python
    >>> import os
    >>> os.path.join('.git', '/foo/bar')
    '/foo/bar'
    >>>

Maybe there are other examples of libraries with these semantics.

Show 9 quoted lines

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

Indeed, regardless of the policy about leading slashes, this is a good plan. I will try to find time to work on it.

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

Michael

[1] http://docs.python.org/library/os.path.html#os.path.join

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/