Re: [PATCH 09/23] pack v4: commit object encoding
On Tue, 27 Aug 2013, Junio C Hamano wrote:
Show 25 quoted lines
> 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.
Show 9 quoted lines
> > 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