Re: [PATCH 09/23] pack v4: commit object encoding

Nicolas Pitre [off-list ref] writes:

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

quoted

Nicolas Pitre [off-list ref] writes:

quoted

+	/* 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.

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

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

Nicolas Pitre [off-list ref] writes:

quoted

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

quoted

Nicolas Pitre [off-list ref] writes:

quoted

+	/* 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.

quoted

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.

quoted

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