From: Nicolas Pitre <nico@fluxnic.net>
Date: Tue, 27 Aug 2013 21:43:01 GMT
Subject: Re: [PATCH 09/23] pack v4: commit object encoding
Message-ID: <alpine.LFD.2.03.1308271740110.14472@syhkavp.arg>
In-Reply-To: <xmqqob8ic2kv.fsf@gitster.dls.corp.google.com>

On Tue, 27 Aug 2013, Junio C Hamano wrote:

> Nicolas Pitre <nico@fluxnic.net> writes:
> 
> > On Tue, 27 Aug 2013, Junio C Hamano wrote:
> >
> >> Nicolas Pitre <nico@fluxnic.net> writes:
> >> 
> >> > +	/* parse the "tree" line */
> >> > +	if (in + 46 >= tail || memcmp(in, "tree ", 5) || in[45] != '\n')
> >> > +		goto bad_data;
> >> > +	if (get_sha1_hex(in + 5, sha1) < 0)
> >> > +		goto bad_data;
> >> 
> >> Is this strict enough to guarantee roundtrip hash identity?  Because
> >> get_sha1_hex() accepts hexadecimal represented with uppercase A-F,
> >> you need to reject such a "broken" commit object, no?
> >
> > BTW, is there any such objects in existence where sha1 ascii strings are 
> > represented using uppercase letters?
> 
> Any commit or tag object that refers to another object with SHA-1
> using uppercase letters is broken and invalid.  get_sha1_hex() is
> not limited to reading these (i.e. it also is used to read object
> name given on the command line) so it is lenient, but the above
> codepath should care so that the result of hashing will stay the
> same.

Indeed, hence my concern about encoding the original case.

> > Because there would be a simple 
> > way to encode that fact in the pack v4 sha1 reference... but that change 
> > has to happen now.
> 
> Hence, I do not think we care.
> 
> > I'm already claiming we won't support mixed case though.
> 
> Yeah, I am already claiming we won't support any uppercase ;-).

Perfect!

I've added a get_sha1_lowhex() to my tree.


Nicolas