[RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[brian m. carlson]

As far as I can tell, Git has always emitted hex object IDs in
lowercase, but our object ID parser accepts both uppercase and
lowercase.  This leads to much software relying on hex object IDs being
broken because it doesn't handle uppercase object IDs and this can even
lead to security problems when people assume that an object ID has a
unique hex form.

This series proposes to remove the ability to use uppercase hex in
object IDs in Git 3.0.  It is RFC simply because it's not clear if
there's the desire to do this, although the series should be fully
functional.

As further evidence of why we should do this, I'll note that there is
exactly one testcase in our testsuite that fails due to this change
(fixed in the last patch) and it's not clear that it fails
intentionally.  If we decide not to adopt this series, it would probably
be prudent to add some additional tests for the uppercase variant of hex
object IDs.

brian m. carlson (6):
  hex: add functionality for lowercase-only hex
  hex: allow specifying hex type with hex2chr
  hex: make hex_to_bytes accept kind of hex to use
  hex: label usages of hex parsing for object IDs
  object-name: use hexval
  hex: allow only lowercase object IDs in breaking changes mode

 Documentation/BreakingChanges.adoc |  5 ++++
 builtin/index-pack.c               |  2 +-
 color.c                            |  2 +-
 diagnose.c                         |  2 +-
 hex-ll.c                           | 39 ++++++++++++++++++++++++++++--
 hex-ll.h                           | 24 +++++++++++++-----
 hex.c                              |  2 +-
 http-push.c                        |  5 ++--
 mailinfo.c                         |  2 +-
 notes.c                            |  5 ++--
 object-file.c                      |  2 +-
 object-name.c                      | 13 +++-------
 pkt-line.c                         |  8 +++---
 ref-filter.c                       |  2 +-
 strbuf.c                           |  2 +-
 t/t1503-rev-parse-verify.sh        |  5 ++++
 t/t5324-split-commit-graph.sh      |  4 +--
 url.c                              |  2 +-
 urlmatch.c                         |  2 +-
 19 files changed, 90 insertions(+), 38 deletions(-)

[RFC PATCH 1/6] hex: add functionality for lowercase-only hex

[brian m. carlson]

We currently allow both upper and lower case for all hex values in Git.
However, in a future commit, we'll want to change that to allow only
lowercase values in some cases.  To prepare for that case, provide a
table to convert hex values using lowercase only and an enum to let us
choose which we want, wiring it up to the hexval function.

For now, keep things completely the same by specifying only the
variant that accepts both lowercase and uppercase to avoid changing
behavior.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 color.c    |  2 +-
 hex-ll.c   | 37 ++++++++++++++++++++++++++++++++++++-
 hex-ll.h   | 14 ++++++++++----
 pkt-line.c |  8 ++++----
 4 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/color.c b/color.c
index 00b53f97ac..9015d0faf1 100644
--- a/color.c
+++ b/color.c
@@ -72,7 +72,7 @@ static int get_hex_color(const char **inp, int width, unsigned char *out)
 	unsigned int val;
 
 	assert(width == 1 || width == 2);
-	val = (hexval(in[0]) << 4) | hexval(in[width - 1]);
+	val = (hexval(in[0], HEX_KIND_MIXED) << 4) | hexval(in[width - 1], HEX_KIND_MIXED);
 	if (val & ~0xff)
 		return -1;
 	*inp += width;
diff --git a/hex-ll.c b/hex-ll.c
index 4d7ece1de5..fa85e91827 100644
--- a/hex-ll.c
+++ b/hex-ll.c
@@ -36,10 +36,45 @@ const signed char hexval_table[256] = {
 	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f8-ff */
 };
 
+const signed char hexval_lc_table[256] = {
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 00-07 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 08-0f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 10-17 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 18-1f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 20-27 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 28-2f */
+	  0,  1,  2,  3,  4,  5,  6,  7,		/* 30-37 */
+	  8,  9, -1, -1, -1, -1, -1, -1,		/* 38-3f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 40-47 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 48-4f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 50-57 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 58-5f */
+	 -1, 10, 11, 12, 13, 14, 15, -1,		/* 60-67 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 68-67 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 70-77 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 78-7f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 80-87 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 88-8f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 90-97 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 98-9f */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* a0-a7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* a8-af */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* b0-b7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* b8-bf */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* c0-c7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* c8-cf */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* d0-d7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* d8-df */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* e0-e7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* e8-ef */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f0-f7 */
+	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f8-ff */
+};
+
 int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
 {
 	for (; len; len--, hex += 2) {
-		unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
+		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
 
 		if (val & ~0xff)
 			return -1;
diff --git a/hex-ll.h b/hex-ll.h
index a381fa8556..da1b5239b2 100644
--- a/hex-ll.h
+++ b/hex-ll.h
@@ -1,10 +1,16 @@
 #ifndef HEX_LL_H
 #define HEX_LL_H
 
+enum hexkind {
+	HEX_KIND_MIXED = 0,
+	HEX_KIND_LOWER = 1,
+};
+
 extern const signed char hexval_table[256];
-static inline unsigned int hexval(unsigned char c)
+extern const signed char hexval_lc_table[256];
+static inline unsigned int hexval(unsigned char c, enum hexkind kind)
 {
-	return hexval_table[c];
+	return kind == HEX_KIND_MIXED ? hexval_table[c] : hexval_lc_table[c];
 }
 
 /*
@@ -13,8 +19,8 @@ static inline unsigned int hexval(unsigned char c)
  */
 static inline int hex2chr(const char *s)
 {
-	unsigned int val = hexval(s[0]);
-	return (val & ~0xf) ? val : (val << 4) | hexval(s[1]);
+	unsigned int val = hexval(s[0], HEX_KIND_MIXED);
+	return (val & ~0xf) ? val : (val << 4) | hexval(s[1], HEX_KIND_MIXED);
 }
 
 /*
diff --git a/pkt-line.c b/pkt-line.c
index 3fc3e9ea70..338075558c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -378,10 +378,10 @@ int packet_length(const char lenbuf_hex[4], size_t size)
 {
 	if (size < 4)
 		BUG("buffer too small");
-	return	hexval(lenbuf_hex[0]) << 12 |
-		hexval(lenbuf_hex[1]) <<  8 |
-		hexval(lenbuf_hex[2]) <<  4 |
-		hexval(lenbuf_hex[3]);
+	return	hexval(lenbuf_hex[0], HEX_KIND_MIXED) << 12 |
+		hexval(lenbuf_hex[1], HEX_KIND_MIXED) <<  8 |
+		hexval(lenbuf_hex[2], HEX_KIND_MIXED) <<  4 |
+		hexval(lenbuf_hex[3], HEX_KIND_MIXED);
 }
 
 static const char *find_packfile_uri_path(const char *buffer)

[RFC PATCH 2/6] hex: allow specifying hex type with hex2chr

[brian m. carlson]

We have several places where we use hex2chr.  One of those is parsing
object IDs, but others decode quoted-printable or percent encoding.  All
of them accept both uppercase and lowercase hex.

In a future commit, we'll change some of these cases, so make hex2chr
accept the kind of encoding to use: lowercase only hex or any kind of
hex.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 hex-ll.h     | 6 +++---
 hex.c        | 2 +-
 mailinfo.c   | 2 +-
 ref-filter.c | 2 +-
 strbuf.c     | 2 +-
 url.c        | 2 +-
 urlmatch.c   | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hex-ll.h b/hex-ll.h
index da1b5239b2..26847c7b2f 100644
--- a/hex-ll.h
+++ b/hex-ll.h
@@ -17,10 +17,10 @@ static inline unsigned int hexval(unsigned char c, enum hexkind kind)
  * Convert two consecutive hexadecimal digits into a char.  Return a
  * negative value on error.  Don't run over the end of short strings.
  */
-static inline int hex2chr(const char *s)
+static inline int hex2chr(const char *s, enum hexkind kind)
 {
-	unsigned int val = hexval(s[0], HEX_KIND_MIXED);
-	return (val & ~0xf) ? val : (val << 4) | hexval(s[1], HEX_KIND_MIXED);
+	unsigned int val = hexval(s[0], kind);
+	return (val & ~0xf) ? val : (val << 4) | hexval(s[1], kind);
 }
 
 /*
diff --git a/hex.c b/hex.c
index f02832140d..6150bdcbf8 100644
--- a/hex.c
+++ b/hex.c
@@ -9,7 +9,7 @@ static int get_hash_hex_algop(const char *hex, unsigned char *hash,
 			      const struct git_hash_algo *algop)
 {
 	for (size_t i = 0; i < algop->rawsz; i++) {
-		int val = hex2chr(hex);
+		int val = hex2chr(hex, HEX_KIND_MIXED);
 		if (val < 0)
 			return -1;
 		*hash++ = val;
diff --git a/mailinfo.c b/mailinfo.c
index 13949ff31e..85c3119048 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -396,7 +396,7 @@ static int decode_q_segment(struct strbuf *out, const struct strbuf *q_seg,
 			int ch, d = *in;
 			if (d == '\n' || !d)
 				break; /* drop trailing newline */
-			ch = hex2chr(in);
+			ch = hex2chr(in, HEX_KIND_MIXED);
 			if (ch >= 0) {
 				strbuf_addch(out, ch);
 				in += 2;
diff --git a/ref-filter.c b/ref-filter.c
index 29aca08ce7..884bcd8fc5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3567,7 +3567,7 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting
 			if (cp[1] == '%')
 				cp++;
 			else {
-				int ch = hex2chr(cp + 1);
+				int ch = hex2chr(cp + 1, HEX_KIND_MIXED);
 				if (0 <= ch) {
 					strbuf_addch(s, ch);
 					cp += 3;
diff --git a/strbuf.c b/strbuf.c
index 44955669e8..88d23f8ac5 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -457,7 +457,7 @@ size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder)
 		return 1;
 	case 'x':
 		/* %x00 == NUL, %x0a == LF, etc. */
-		ch = hex2chr(placeholder + 1);
+		ch = hex2chr(placeholder + 1, HEX_KIND_MIXED);
 		if (ch < 0)
 			return 0;
 		strbuf_addch(sb, ch);
diff --git a/url.c b/url.c
index a59818278f..b4d72f784a 100644
--- a/url.c
+++ b/url.c
@@ -62,7 +62,7 @@ static char *url_decode_internal(const char **query, int len,
 		}
 
 		if (c == '%' && (len < 0 || len >= 3)) {
-			int val = hex2chr(q + 1);
+			int val = hex2chr(q + 1, HEX_KIND_MIXED);
 			if (0 < val) {
 				strbuf_addch(out, val);
 				q += 3;
diff --git a/urlmatch.c b/urlmatch.c
index 20bc2d009c..989f1d794b 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -50,7 +50,7 @@ static int append_normalized_escapes(struct strbuf *buf,
 		if (ch == '%') {
 			if (from_len < 2)
 				return 0;
-			ch = hex2chr(from);
+			ch = hex2chr(from, HEX_KIND_MIXED);
 			if (ch < 0)
 				return 0;
 			from += 2;

[RFC PATCH 3/6] hex: make hex_to_bytes accept kind of hex to use

[brian m. carlson]

Similarly to the previous commit, introduce an option for hex_to_bytes
to allow us to specify the kind of hex to use: lowercase only or not.
For now, everything remains the same as before, but we will change
things in a future commit.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/index-pack.c | 2 +-
 diagnose.c           | 2 +-
 hex-ll.c             | 4 ++--
 hex-ll.h             | 2 +-
 http-push.c          | 5 +++--
 notes.c              | 5 +++--
 object-file.c        | 2 +-
 7 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index bc86925ad0..9660e5967b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1866,7 +1866,7 @@ static void repack_local_links(void)
 	while (strbuf_getline_lf(&line, out) != EOF) {
 		unsigned char binary[GIT_MAX_RAWSZ];
 		if (line.len != the_hash_algo->hexsz ||
-		    !hex_to_bytes(binary, line.buf, line.len))
+		    !hex_to_bytes(binary, line.buf, line.len, HEX_KIND_MIXED))
 			die(_("index-pack: Expecting full hex object ID lines only from pack-objects."));
 
 		/*
diff --git a/diagnose.c b/diagnose.c
index 5092bf80d3..fc11cea229 100644
--- a/diagnose.c
+++ b/diagnose.c
@@ -112,7 +112,7 @@ static void loose_objs_stats(struct strbuf *buf, const char *path)
 	while ((e = readdir_skip_dot_and_dotdot(dir)) != NULL)
 		if (get_dtype(e, &count_path, 0) == DT_DIR &&
 		    strlen(e->d_name) == 2 &&
-		    !hex_to_bytes(&c, e->d_name, 1)) {
+		    !hex_to_bytes(&c, e->d_name, 1, HEX_KIND_MIXED)) {
 			strbuf_setlen(&count_path, base_path_len);
 			strbuf_addf(&count_path, "%s/", e->d_name);
 			total += (count = count_files(&count_path));
diff --git a/hex-ll.c b/hex-ll.c
index fa85e91827..b2e9684693 100644
--- a/hex-ll.c
+++ b/hex-ll.c
@@ -71,10 +71,10 @@ const signed char hexval_lc_table[256] = {
 	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f8-ff */
 };
 
-int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
+int hex_to_bytes(unsigned char *binary, const char *hex, size_t len, enum hexkind kind)
 {
 	for (; len; len--, hex += 2) {
-		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
+		unsigned int val = (hexval(hex[0], kind) << 4) | hexval(hex[1], kind);
 
 		if (val & ~0xff)
 			return -1;
diff --git a/hex-ll.h b/hex-ll.h
index 26847c7b2f..fe698f0c76 100644
--- a/hex-ll.h
+++ b/hex-ll.h
@@ -28,6 +28,6 @@ static inline int hex2chr(const char *s, enum hexkind kind)
  * values to `binary` as `len` bytes. Return 0 on success, or -1 if
  * the input does not consist of hex digits).
  */
-int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);
+int hex_to_bytes(unsigned char *binary, const char *hex, size_t len, enum hexkind kind);
 
 #endif
diff --git a/http-push.c b/http-push.c
index 94a1fac9ab..0cc990d395 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1030,12 +1030,13 @@ static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)
 	if (strlen(path) != the_hash_algo->hexsz + 1)
 		return -1;
 
-	if (hex_to_bytes(oid->hash, path, 1))
+	if (hex_to_bytes(oid->hash, path, 1, HEX_KIND_MIXED))
 		return -1;
 	path += 2;
 	path++; /* skip '/' */
 
-	return hex_to_bytes(oid->hash + 1, path, the_hash_algo->rawsz - 1);
+	return hex_to_bytes(oid->hash + 1, path, the_hash_algo->rawsz - 1,
+			    HEX_KIND_MIXED);
 }
 
 static void process_ls_object(struct remote_ls_ctx *ls)
diff --git a/notes.c b/notes.c
index ec9c2cb150..99b8b15d81 100644
--- a/notes.c
+++ b/notes.c
@@ -428,7 +428,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 				goto handle_non_note;
 
 			if (hex_to_bytes(object_oid.hash + prefix_len, entry.path,
-					 hashsz - prefix_len))
+					 hashsz - prefix_len, HEX_KIND_MIXED))
 				goto handle_non_note; /* entry.path is not a SHA1 */
 
 			memset(object_oid.hash + hashsz, 0, GIT_MAX_RAWSZ - hashsz);
@@ -442,7 +442,8 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 				/* internal nodes must be trees */
 				goto handle_non_note;
 
-			if (hex_to_bytes(object_oid.hash + len++, entry.path, 1))
+			if (hex_to_bytes(object_oid.hash + len++, entry.path, 1,
+					 HEX_KIND_MIXED))
 				goto handle_non_note; /* entry.path is not a SHA1 */
 
 			/*
diff --git a/object-file.c b/object-file.c
index 7ff2b730ac..8427b2802a 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1473,7 +1473,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 		strbuf_add(path, de->d_name, namelen);
 		if (namelen == algop->hexsz - 2 &&
 		    !hex_to_bytes(oid.hash + 1, de->d_name,
-				  algop->rawsz - 1)) {
+				  algop->rawsz - 1, HEX_KIND_MIXED)) {
 			oid_set_algo(&oid, algop);
 			memset(oid.hash + algop->rawsz, 0,
 			       GIT_MAX_RAWSZ - algop->rawsz);

[RFC PATCH 4/6] hex: label usages of hex parsing for object IDs

[brian m. carlson]

In preparation for a future change, label the hex parsing we're doing
for object IDs by defining a constant called HEX_KIND_OID.  This is
currently the same as HEX_KIND_MIXED, so there is no functional change
here.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 diagnose.c    | 2 +-
 hex-ll.h      | 2 ++
 hex.c         | 2 +-
 http-push.c   | 4 ++--
 notes.c       | 2 +-
 object-file.c | 2 +-
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/diagnose.c b/diagnose.c
index fc11cea229..9c652d36a6 100644
--- a/diagnose.c
+++ b/diagnose.c
@@ -112,7 +112,7 @@ static void loose_objs_stats(struct strbuf *buf, const char *path)
 	while ((e = readdir_skip_dot_and_dotdot(dir)) != NULL)
 		if (get_dtype(e, &count_path, 0) == DT_DIR &&
 		    strlen(e->d_name) == 2 &&
-		    !hex_to_bytes(&c, e->d_name, 1, HEX_KIND_MIXED)) {
+		    !hex_to_bytes(&c, e->d_name, 1, HEX_KIND_OID)) {
 			strbuf_setlen(&count_path, base_path_len);
 			strbuf_addf(&count_path, "%s/", e->d_name);
 			total += (count = count_files(&count_path));
diff --git a/hex-ll.h b/hex-ll.h
index fe698f0c76..9da76f17e8 100644
--- a/hex-ll.h
+++ b/hex-ll.h
@@ -6,6 +6,8 @@ enum hexkind {
 	HEX_KIND_LOWER = 1,
 };
 
+#define HEX_KIND_OID HEX_KIND_MIXED
+
 extern const signed char hexval_table[256];
 extern const signed char hexval_lc_table[256];
 static inline unsigned int hexval(unsigned char c, enum hexkind kind)
diff --git a/hex.c b/hex.c
index 6150bdcbf8..4e1e81af3f 100644
--- a/hex.c
+++ b/hex.c
@@ -9,7 +9,7 @@ static int get_hash_hex_algop(const char *hex, unsigned char *hash,
 			      const struct git_hash_algo *algop)
 {
 	for (size_t i = 0; i < algop->rawsz; i++) {
-		int val = hex2chr(hex, HEX_KIND_MIXED);
+		int val = hex2chr(hex, HEX_KIND_OID);
 		if (val < 0)
 			return -1;
 		*hash++ = val;
diff --git a/http-push.c b/http-push.c
index 0cc990d395..132d26d6a1 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1030,13 +1030,13 @@ static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)
 	if (strlen(path) != the_hash_algo->hexsz + 1)
 		return -1;
 
-	if (hex_to_bytes(oid->hash, path, 1, HEX_KIND_MIXED))
+	if (hex_to_bytes(oid->hash, path, 1, HEX_KIND_OID))
 		return -1;
 	path += 2;
 	path++; /* skip '/' */
 
 	return hex_to_bytes(oid->hash + 1, path, the_hash_algo->rawsz - 1,
-			    HEX_KIND_MIXED);
+			    HEX_KIND_OID);
 }
 
 static void process_ls_object(struct remote_ls_ctx *ls)
diff --git a/notes.c b/notes.c
index 99b8b15d81..7e9e3eb2d2 100644
--- a/notes.c
+++ b/notes.c
@@ -443,7 +443,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 				goto handle_non_note;
 
 			if (hex_to_bytes(object_oid.hash + len++, entry.path, 1,
-					 HEX_KIND_MIXED))
+					 HEX_KIND_OID))
 				goto handle_non_note; /* entry.path is not a SHA1 */
 
 			/*
diff --git a/object-file.c b/object-file.c
index 8427b2802a..4bcff66442 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1473,7 +1473,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 		strbuf_add(path, de->d_name, namelen);
 		if (namelen == algop->hexsz - 2 &&
 		    !hex_to_bytes(oid.hash + 1, de->d_name,
-				  algop->rawsz - 1, HEX_KIND_MIXED)) {
+				  algop->rawsz - 1, HEX_KIND_OID)) {
 			oid_set_algo(&oid, algop);
 			memset(oid.hash + algop->rawsz, 0,
 			       GIT_MAX_RAWSZ - algop->rawsz);

[RFC PATCH 5/6] object-name: use hexval

[brian m. carlson]

We've open-coded a different implementation of parsing hex values here
when we already have a perfectly good one in hexval.  This
implementation will almost certainly be slower because it isn't
table-driven, unlike the other one, and since it's not constant time it
has no other advantages either.  To tidy things up and prepare for
future work, switch to hexval in this case.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 object-name.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/object-name.c b/object-name.c
index 83efba0ba6..d2d81b3511 100644
--- a/object-name.c
+++ b/object-name.c
@@ -236,17 +236,10 @@ static int parse_oid_prefix(const char *name, int len,
 {
 	for (int i = 0; i < len; i++) {
 		unsigned char c = name[i];
-		unsigned char val;
-		if (c >= '0' && c <= '9') {
-			val = c - '0';
-		} else if (c >= 'a' && c <= 'f') {
-			val = c - 'a' + 10;
-		} else if (c >= 'A' && c <='F') {
-			val = c - 'A' + 10;
-			c -= 'A' - 'a';
-		} else {
+		int val = hexval(c, HEX_KIND_OID);
+
+		if (val < 0)
 			return -1;
-		}
 
 		if (hex_out)
 			hex_out[i] = c;

[RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[brian m. carlson]

Git has historically allowed either lowercase or uppercase hex for
object IDs, but it has always emitted only lowercase.  This has caused
people to expect only lowercase and not handle uppercase.

As an example, Git's own example hooks look for "[0-9a-f]" in several
places, but there are many other Git-adjacent pieces of software,
including Gitolite, which make the assumption that object IDs are always
lowercase.  This is not to criticize the authors of these projects, but
rather to point out how common this assumption is.  In fact, it's so
common that we have only one test in our codebase that fails when we
reject uppercase object IDs.

More critically, it leads people to make security-based assumptions that
an object ID either does not contain uppercase characters or that an
object ID can be expressed uniquely in hex form, neither of which are
currently true.  Git itself normally uses binary object IDs, which
avoids many of these problems, but most other projects deal primarily in
hex object IDs, so they are more affected.

In preparation for Git 3.0, only allow lowercase hex object IDs in
breaking changes mode and document this as well.  Update the single
failing test and add a new one to verify we reject new uppercase object
IDs.  Note that in t5324, we change the hex character from "A" to "b"
because in SHA-256 mode, "a" is the correct value, so our test_must_fail
assertion will unexpectedly succeed in that case.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/BreakingChanges.adoc | 5 +++++
 hex-ll.h                           | 4 ++++
 t/t1503-rev-parse-verify.sh        | 5 +++++
 t/t5324-split-commit-graph.sh      | 4 ++--
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
index 73bb939359..dbc46d14e3 100644
--- a/Documentation/BreakingChanges.adoc
+++ b/Documentation/BreakingChanges.adoc
@@ -171,6 +171,11 @@ JGit, libgit2 and Gitoxide need to support it.
   matches the default branch name used in new repositories by many of the
   big Git forges.
 
+* Git will accept hex object IDs only in lowercase. The fact that Git has
+	historically allowed uppercase characters in hex object IDs has been the
+	source of a variety of bugs and security problems in software using Git. We
+	don't expect most users to notice any change.
+
 * Git will require Rust as a mandatory part of the build process. While Git
   already started to adopt Rust in Git 2.49, all parts written in Rust are
   optional for the time being. This includes:
diff --git a/hex-ll.h b/hex-ll.h
index 9da76f17e8..2f9c8d7c25 100644
--- a/hex-ll.h
+++ b/hex-ll.h
@@ -6,7 +6,11 @@ enum hexkind {
 	HEX_KIND_LOWER = 1,
 };
 
+#ifdef WITH_BREAKING_CHANGES
+#define HEX_KIND_OID HEX_KIND_LOWER
+#else
 #define HEX_KIND_OID HEX_KIND_MIXED
+#endif
 
 extern const signed char hexval_table[256];
 extern const signed char hexval_lc_table[256];
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index 87638a4a2c..f07b45de5a 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -60,6 +60,11 @@ test_expect_success 'works with one good rev' '
 	test "$rev_head" = "$HASH4"
 '
 
+test_expect_success WITH_BREAKING_CHANGES 'rejects uppercase revs' '
+	UC_HASH=$(echo "$HASH1" | tr a-f A-F) &&
+	test_must_fail git rev-parse --verify "$UC_HASH"
+'
+
 test_expect_success 'fails with any bad rev or many good revs' '
 	test_must_fail git rev-parse --verify 2>error &&
 	test_grep "single revision" error &&
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index bf7ba0e558..29db815c77 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -349,7 +349,7 @@ test_expect_success 'verify after commit-graph-chain corruption (base)' '
 		test_must_fail git commit-graph verify 2>test_err &&
 		grep -v "^+" test_err >err &&
 		test_grep "invalid commit-graph chain" err &&
-		corrupt_file "$graphdir/commit-graph-chain" 30 "A" &&
+		corrupt_file "$graphdir/commit-graph-chain" 30 "a" &&
 		test_must_fail git commit-graph verify 2>test_err &&
 		grep -v "^+" test_err >err &&
 		test_grep "unable to find all commit-graph files" err
@@ -364,7 +364,7 @@ test_expect_success 'verify after commit-graph-chain corruption (tip)' '
 		test_must_fail git commit-graph verify 2>test_err &&
 		grep -v "^+" test_err >err &&
 		test_grep "invalid commit-graph chain" err &&
-		corrupt_file "$graphdir/commit-graph-chain" 70 "A" &&
+		corrupt_file "$graphdir/commit-graph-chain" 70 "b" &&
 		test_must_fail git commit-graph verify 2>test_err &&
 		grep -v "^+" test_err >err &&
 		test_grep "unable to find all commit-graph files" err

Re: [RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> As far as I can tell, Git has always emitted hex object IDs in
> lowercase, but our object ID parser accepts both uppercase and
> lowercase.  This leads to much software relying on hex object IDs being
> broken because it doesn't handle uppercase object IDs and this can even
> lead to security problems when people assume that an object ID has a
> unique hex form.
>
> This series proposes to remove the ability to use uppercase hex in
> object IDs in Git 3.0.  It is RFC simply because it's not clear if
> there's the desire to do this, although the series should be fully
> functional.
>
> As further evidence of why we should do this, I'll note that there is
> exactly one testcase in our testsuite that fails due to this change
> (fixed in the last patch) and it's not clear that it fails
> intentionally.  If we decide not to adopt this series, it would probably
> be prudent to add some additional tests for the uppercase variant of hex
> object IDs.

Before going there, we should hear a solid argument why doing this
might be beneficial longer term.  "Just because we might be able to
without harming too many users" is probably not good enough, when it
is not accompanied by "... the (low) risk may be worth taking because
we will gain such and such benefit".

Re: [RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

On 2026-07-30 at 08:21:46, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > As far as I can tell, Git has always emitted hex object IDs in
> > lowercase, but our object ID parser accepts both uppercase and
> > lowercase.  This leads to much software relying on hex object IDs being
> > broken because it doesn't handle uppercase object IDs and this can even
> > lead to security problems when people assume that an object ID has a
> > unique hex form.
> >
> > This series proposes to remove the ability to use uppercase hex in
> > object IDs in Git 3.0.  It is RFC simply because it's not clear if
> > there's the desire to do this, although the series should be fully
> > functional.
> >
> > As further evidence of why we should do this, I'll note that there is
> > exactly one testcase in our testsuite that fails due to this change
> > (fixed in the last patch) and it's not clear that it fails
> > intentionally.  If we decide not to adopt this series, it would probably
> > be prudent to add some additional tests for the uppercase variant of hex
> > object IDs.
> 
> Before going there, we should hear a solid argument why doing this
> might be beneficial longer term.  "Just because we might be able to
> without harming too many users" is probably not good enough, when it
> is not accompanied by "... the (low) risk may be worth taking because
> we will gain such and such benefit".

While I can't speak about the details, I've actually seen multiple
security vulnerabilities show up because people didn't realize that
uppercase hex was a thing in Git object IDs and so filtering or other
sanitizing was ineffective.  That's the real motivation behind this
change.

Also, as patch 6 says, a large amount of Git-adjacent software,
including common implementations such as Gitolite, don't accept them or
don't handle them correctly.  A quick code search for `[0-9a-f]{40}` on
GitHub shows a lot of these tools.  Our own hook examples even use a
similar pattern, and although in that case they are accepting only Git's
output, users see those as examples of how to parse object IDs.

The situation is presently that Git will accept them and this leads to
surprising behaviour, but almost all adjacent software rejects or
mishandles them.  I'm arguing that we should stop accepting hex object
ID formats that cannot be effectively used in the Git ecosystem but
whose presence is effectively only ever the source of misbehaviour and
security vulnerabilities.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [RFC PATCH 4/6] hex: label usages of hex parsing for object IDs

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> In preparation for a future change, label the hex parsing we're doing
> for object IDs by defining a constant called HEX_KIND_OID.  This is
> currently the same as HEX_KIND_MIXED, so there is no functional change
> here.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  diagnose.c    | 2 +-
>  hex-ll.h      | 2 ++
>  hex.c         | 2 +-
>  http-push.c   | 4 ++--
>  notes.c       | 2 +-
>  object-file.c | 2 +-
>  6 files changed, 8 insertions(+), 6 deletions(-)

OK.  It makes sense to say "we are reading object names", than "we
are reading hex spelled in both cases".  Are we throwing the "not
object names but derived from the same hash function" things like
packname and rerere database key into the same category?

> diff --git a/diagnose.c b/diagnose.c
> index fc11cea229..9c652d36a6 100644
> --- a/diagnose.c
> +++ b/diagnose.c
> @@ -112,7 +112,7 @@ static void loose_objs_stats(struct strbuf *buf, const char *path)
>  	while ((e = readdir_skip_dot_and_dotdot(dir)) != NULL)
>  		if (get_dtype(e, &count_path, 0) == DT_DIR &&
>  		    strlen(e->d_name) == 2 &&
> -		    !hex_to_bytes(&c, e->d_name, 1, HEX_KIND_MIXED)) {
> +		    !hex_to_bytes(&c, e->d_name, 1, HEX_KIND_OID)) {
>  			strbuf_setlen(&count_path, base_path_len);
>  			strbuf_addf(&count_path, "%s/", e->d_name);
>  			total += (count = count_files(&count_path));

Re: [RFC PATCH 1/6] hex: add functionality for lowercase-only hex

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> We currently allow both upper and lower case for all hex values in Git.
> However, in a future commit, we'll want to change that to allow only
> lowercase values in some cases.  To prepare for that case, provide a
> table to convert hex values using lowercase only and an enum to let us
> choose which we want, wiring it up to the hexval function.
>
> For now, keep things completely the same by specifying only the
> variant that accepts both lowercase and uppercase to avoid changing
> behavior.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---

In "some" cases?  I wonder what other cases there are that we MUST
accept uppercase variants.  Obviously the network protocol where we
are willing to talk to reimplementation of Git by others is one.  I
do not think we historically produced anything in uppercase.

> diff --git a/color.c b/color.c
> index 00b53f97ac..9015d0faf1 100644
> --- a/color.c
> +++ b/color.c
> @@ -72,7 +72,7 @@ static int get_hex_color(const char **inp, int width, unsigned char *out)
>  	unsigned int val;
>  
>  	assert(width == 1 || width == 2);
> -	val = (hexval(in[0]) << 4) | hexval(in[width - 1]);
> +	val = (hexval(in[0], HEX_KIND_MIXED) << 4) | hexval(in[width - 1], HEX_KIND_MIXED);
>  	if (val & ~0xff)
>  		return -1;
>  	*inp += width;
> diff --git a/hex-ll.c b/hex-ll.c
> index 4d7ece1de5..fa85e91827 100644
> --- a/hex-ll.c
> +++ b/hex-ll.c
> @@ -36,10 +36,45 @@ const signed char hexval_table[256] = {
>  	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f8-ff */
>  };
>  
> +const signed char hexval_lc_table[256] = {
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 00-07 */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 08-0f */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 10-17 */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 18-1f */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 20-27 */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 28-2f */
> +	  0,  1,  2,  3,  4,  5,  6,  7,		/* 30-37 */
> +	  8,  9, -1, -1, -1, -1, -1, -1,		/* 38-3f */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 40-47 */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 48-4f */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 50-57 */
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 58-5f */
> +	 -1, 10, 11, 12, 13, 14, 15, -1,		/* 60-67 */
> ...
> +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* f8-ff */
> +};
> +
>  int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
>  {
>  	for (; len; len--, hex += 2) {
> -		unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
> +		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
>  
>  		if (val & ~0xff)
>  			return -1;
>
> diff --git a/hex-ll.h b/hex-ll.h
> index a381fa8556..da1b5239b2 100644
> --- a/hex-ll.h
> +++ b/hex-ll.h
> @@ -1,10 +1,16 @@
>  #ifndef HEX_LL_H
>  #define HEX_LL_H
>  
> +enum hexkind {
> +	HEX_KIND_MIXED = 0,
> +	HEX_KIND_LOWER = 1,
> +};
> +
>  extern const signed char hexval_table[256];
> -static inline unsigned int hexval(unsigned char c)
> +extern const signed char hexval_lc_table[256];
> +static inline unsigned int hexval(unsigned char c, enum hexkind kind)
>  {
> -	return hexval_table[c];
> +	return kind == HEX_KIND_MIXED ? hexval_table[c] : hexval_lc_table[c];
>  }

It is very welcome to make sure we are conservative in what we
produce, but be liberal in what we accept.  In that sense, use of
HEX_KIND_LOWER goes directly against the Robustness Principle.

>  
>  /*
> @@ -13,8 +19,8 @@ static inline unsigned int hexval(unsigned char c)
>   */
>  static inline int hex2chr(const char *s)
>  {
> -	unsigned int val = hexval(s[0]);
> -	return (val & ~0xf) ? val : (val << 4) | hexval(s[1]);
> +	unsigned int val = hexval(s[0], HEX_KIND_MIXED);
> +	return (val & ~0xf) ? val : (val << 4) | hexval(s[1], HEX_KIND_MIXED);
>  }
>  
>  /*
> diff --git a/pkt-line.c b/pkt-line.c
> index 3fc3e9ea70..338075558c 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -378,10 +378,10 @@ int packet_length(const char lenbuf_hex[4], size_t size)
>  {
>  	if (size < 4)
>  		BUG("buffer too small");
> -	return	hexval(lenbuf_hex[0]) << 12 |
> -		hexval(lenbuf_hex[1]) <<  8 |
> -		hexval(lenbuf_hex[2]) <<  4 |
> -		hexval(lenbuf_hex[3]);
> +	return	hexval(lenbuf_hex[0], HEX_KIND_MIXED) << 12 |
> +		hexval(lenbuf_hex[1], HEX_KIND_MIXED) <<  8 |
> +		hexval(lenbuf_hex[2], HEX_KIND_MIXED) <<  4 |
> +		hexval(lenbuf_hex[3], HEX_KIND_MIXED);
>  }
>  
>  static const char *find_packfile_uri_path(const char *buffer)

Re: [RFC PATCH 3/6] hex: make hex_to_bytes accept kind of hex to use

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> -int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
> +int hex_to_bytes(unsigned char *binary, const char *hex, size_t len, enum hexkind kind)
>  {
>  	for (; len; len--, hex += 2) {
> -		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
> +		unsigned int val = (hexval(hex[0], kind) << 4) | hexval(hex[1], kind);
>  
>  		if (val & ~0xff)
>  			return -1;

It depends on how big 'len' would be to matter, but if we are
looping for a long stretch, choosing which one of the two hexval
tables to use outside the loop and using that inside may of course
be more performant.

I wondered how ugly such a restructure of the API would look like,
and it does not look _too_ bad.

	void *hextable = hex_table(HEX_KIND_MIXED);

	for (; len; len--, hex += 2) {
		unsigned int val =
			(hexval(hex[0], hextable) << 4) | hexval(hex[1], hextable);
		...
	}

The true type of hextable would be "signed char [256]", but the
callers of the hexval() function do not need to know it, hence I
chose "void *" here.

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Git has historically allowed either lowercase or uppercase hex for
> object IDs, but it has always emitted only lowercase.  This has caused
> people to expect only lowercase and not handle uppercase.

It is violation of Postel's Law by other people.  We do not
necessarily have to follow suit.

Even though I said throwing object names in a single category makes
sense, it may make sense to treat the object names that we locally
use to access our own object database and those that we use when
talking with _other_ people on the net separately for the Robustness
principle, we keep being strict in what we produce and stick to
lowercase, while accepting uppercase produced by those third-party
reimplementations of Git.

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[Junio C Hamano]

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

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> Git has historically allowed either lowercase or uppercase hex for
>> object IDs, but it has always emitted only lowercase.  This has caused
>> people to expect only lowercase and not handle uppercase.
>
> It is violation of Postel's Law by other people.  We do not
> necessarily have to follow suit.

Imagine we somehow misbehave badly when we have two loose object
files storing the same object's contents.  Let us further imagine
that we can download these individual loose object files from
others, perhaps via the dumb HTTP transport.

If we tried to be robust, we would be liberal in what we accept,
even though we try to be strict in what we produce.  In this
hypothetical scenario, if we talk to someone else over the dumb
HTTP transport and find that they have an objects/AB/ directory, we
may try to be liberal and say, "Ah, that is a fan-out directory
housing all their loose objects whose names begin with 'ab'."

This is the right thing to do for those who liberally accept
others' data.

But we might further say, "Let us enumerate and download what we do
not have locally.  They have a file 012345...EF (38 hex characters)
in that directory, which stores the object AB012345...EF (40 hex
characters) in loose object form," and then conclude, "and we do not
have it," even when we actually have the file ab/012345...ef in
all-lowercase form locally!

However, liberally accepting AB/012345...EF and storing it verbatim
in our own store will break things because, in this hypothetical
scenario, we will misbehave when we have both ab/012345...ef (which
we had from the start) and AB/012345...EF (which we just downloaded)
at the same time.

The approach taken by this RFC series is to stop recognizing their
objects/AB/ as a valid fan-out directory and their AB/012345...EF as
a valid loose object file.  While I agree that this is certainly
one way to avoid entering such a state and triggering bad behavior,
I think the real solution that honors the robustness principle is to
still recognize objects/AB/012345...EF as valid, recognize it as a
loose object file for ab012345...ef, and notice that it represents
the same object ab012345...ef we already have.  Then we can avoid
misbehaving without being less liberal than we used to be.

If the system had been case-sensitive from day one, and ignoring
uppercase hex had been the norm from the beginning, I would not have
found it so disturbing that we reject case-insensitive object names
and being stricter than folks with those other systems may feel is
necessary.

Tightening the rule after twenty years is the part I am most
hesitant to accept.  So, I dunno.

Re: [RFC PATCH 3/6] hex: make hex_to_bytes accept kind of hex to use

[Jeff King]

On Fri, Jul 31, 2026 at 12:38:17AM -0700, Junio C Hamano wrote:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > -int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
> > +int hex_to_bytes(unsigned char *binary, const char *hex, size_t len, enum hexkind kind)
> >  {
> >  	for (; len; len--, hex += 2) {
> > -		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
> > +		unsigned int val = (hexval(hex[0], kind) << 4) | hexval(hex[1], kind);
> >  
> >  		if (val & ~0xff)
> >  			return -1;
> 
> It depends on how big 'len' would be to matter, but if we are
> looping for a long stretch, choosing which one of the two hexval
> tables to use outside the loop and using that inside may of course
> be more performant.
> 
> I wondered how ugly such a restructure of the API would look like,
> and it does not look _too_ bad.
> 
> 	void *hextable = hex_table(HEX_KIND_MIXED);
> 
> 	for (; len; len--, hex += 2) {
> 		unsigned int val =
> 			(hexval(hex[0], hextable) << 4) | hexval(hex[1], hextable);
> 		...
> 	}
> 
> The true type of hextable would be "signed char [256]", but the
> callers of the hexval() function do not need to know it, hence I
> chose "void *" here.

I had the same thought when reading this, but I wondered if the compiler
might be able to hoist the comparison out of the loop itself (because
hexval() it inlined anyway). It doesn't seem to do so, though (at least
with gcc-15). It loads both table addresses into registers, but there's
still a branch in the loop to decide which table to use.

So in theory this kind of manual hoisting could help.  Might not be that
big a deal with branch prediction, though.

-Peff

Re: [RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[Jeff King]

On Thu, Jul 30, 2026 at 09:18:40PM +0000, brian m. carlson wrote:

> The situation is presently that Git will accept them and this leads to
> surprising behaviour, but almost all adjacent software rejects or
> mishandles them.  I'm arguing that we should stop accepting hex object
> ID formats that cannot be effectively used in the Git ecosystem but
> whose presence is effectively only ever the source of misbehaviour and
> security vulnerabilities.

Another interesting case is upper-case hex within objects:

  $ git rev-parse HEAD
  b85b9595a8136c79551340c3d73443a62eddd893

  $ git cat-file commit HEAD |
    perl -lpe '
        if (/^parent (.*)/) {
		$_ = "parent " . uc($1);
	}
    ' |
    git hash-object -w -t commit --stdin
  5a08c6b3f06d91c4a09c8d7ea6e9c8ce200b7698

Now there's a parallel history of otherwise identical commits. I think
this is mostly "if it hurts don't do it", but we generally try to avoid
multiple representations of the same data within the object model.

I think only commits and tags are subject to this (because the tree
hashes are binary). I don't know if you'd be able to stumble into this
accidentally with most Git commands. We don't intentionally normalize
case anywhere, but I think most code will round-trip through a binary
hash at some point (so "git commit-tree 1234ABCD" would incidentally
normalize the case).

-Peff

Re: [RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Now there's a parallel history of otherwise identical commits. I think
> this is mostly "if it hurts don't do it", but we generally try to avoid
> multiple representations of the same data within the object model.

True.  Already almost an empty rebase that delays the clock by 1
seconds is a perfectly normal thing, and the way we treat such a
parallel history with the original would be the same as such an
uppercase parallel history, so I do not think it is a huge issue.

A tree object hierarchy consists fully of binary links, so we are
OK.  Only the top-level object name within a commit/tag may have
multiple representation of the same tree objects.

'git diff COMMIT-A COMMIT-B' would notice that they record the same
tree without opening two tree objects, even if these two commits
record a normal tree and uppercase equivalent tree, as diff_tree()
layer will be called with the binary representation of the tree
object names.

As Brian alluded to in his discussion starter message, we normalize
the case by going binary in many places (we cannot unfortunately say
"in strategic places"), and these are such cases.

> I think only commits and tags are subject to this (because the tree
> hashes are binary). I don't know if you'd be able to stumble into this
> accidentally with most Git commands. We don't intentionally normalize
> case anywhere, but I think most code will round-trip through a binary
> hash at some point (so "git commit-tree 1234ABCD" would incidentally
> normalize the case).

Re: [RFC PATCH 0/6] Git 3.0: restrict hex object IDs to lowercase only

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

On 2026-08-01 at 14:45:27, Jeff King wrote:
> Another interesting case is upper-case hex within objects:
> 
>   $ git rev-parse HEAD
>   b85b9595a8136c79551340c3d73443a62eddd893
> 
>   $ git cat-file commit HEAD |
>     perl -lpe '
>         if (/^parent (.*)/) {
> 		$_ = "parent " . uc($1);
> 	}
>     ' |
>     git hash-object -w -t commit --stdin
>   5a08c6b3f06d91c4a09c8d7ea6e9c8ce200b7698
> 
> Now there's a parallel history of otherwise identical commits. I think
> this is mostly "if it hurts don't do it", but we generally try to avoid
> multiple representations of the same data within the object model.
> 
> I think only commits and tags are subject to this (because the tree
> hashes are binary). I don't know if you'd be able to stumble into this
> accidentally with most Git commands. We don't intentionally normalize
> case anywhere, but I think most code will round-trip through a binary
> hash at some point (so "git commit-tree 1234ABCD" would incidentally
> normalize the case).

Yes, this is true.  I agree that multiple representations is a problem,
and although that can be an issue with signatures, we shouldn't make it
worse.

In addition, those objects cannot be round-tripped through the
interoperability code (which only writes lowercase object IDs), so
they're effectively locked to SHA-1 only.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

On 2026-07-31 at 07:48:14, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > Git has historically allowed either lowercase or uppercase hex for
> > object IDs, but it has always emitted only lowercase.  This has caused
> > people to expect only lowercase and not handle uppercase.
> 
> It is violation of Postel's Law by other people.  We do not
> necessarily have to follow suit.

Postel's Law was a great idea on the early Internet, but it is
unfortunately no longer a good idea.  The problem is that being liberal
in what you accept these days usually has security implications.

TLS cannot be liberal in what it accepts because that means potentially
allowing attacker-controlled data.  Even HTTP cannot do that because
we've seen where refusing to reject requests with both Content-Length
and Transfer-Encoding: chunked means that two parts of a backend can
disagree on the content, allowing request smuggling.

We've seen these problems in our code where not caring about CR comes
back to bite us on Windows in a security-sensitive way.

Modern development effectively requires being clear and definitive about
what data is accepted and what is not, as well as what meaning is given
to the data that is accepted.

> Even though I said throwing object names in a single category makes
> sense, it may make sense to treat the object names that we locally
> use to access our own object database and those that we use when
> talking with _other_ people on the net separately for the Robustness
> principle, we keep being strict in what we produce and stick to
> lowercase, while accepting uppercase produced by those third-party
> reimplementations of Git.

Unfortunately, that also doesn't fix most of the security problems I've
seen, which involve object IDs that get passed on the command line when
tools invoke Git.  It does fix the problem with round-tripping objects
between hash algorithms, though, but I don't really want to audit every
use of oid_to_hex in our codebase to half-fix this situation.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Postel's Law was a great idea on the early Internet, but it is
> unfortunately no longer a good idea.  The problem is that being liberal
> in what you accept these days usually has security implications.

I am afraid that is debatable, though.

I would grant you that you can increase the attack surface by being
carelessly liberal.  Recall my example of allowing mixed-case names
for loose object files and storing them verbatim on a case-sensitive
filesystem without normalizing the names; that is an example of
being carelessly liberal.

But is it a good excuse to give up being careful, declare it is
impossible to be careful enough, and punt?

Will queue, but I invite others to chime in.  My practical side says
we should just take the series as it is much less work for us to
declare that any incompatibility fallout is the problem of other
people who have reimplementations of Git, but my more principled
side feels dirty, just for saying this ;-).

Thanks.

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On 2026-08-04 at 19:32:18, Junio C Hamano wrote:
> Will queue, but I invite others to chime in.  My practical side says
> we should just take the series as it is much less work for us to
> declare that any incompatibility fallout is the problem of other
> people who have reimplementations of Git, but my more principled
> side feels dirty, just for saying this ;-).

I appreciate that, and I do welcome other viewpoints here.  While I
think this series a good idea (or I wouldn't have sent it, obviously),
it's really up to the project what the right thing is and if the
consensus is that this should be dropped, then we can do that.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode

[Michael Montalbo]

On Sun, Aug 2, 2026 at 3:10 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> Modern development effectively requires being clear and definitive about
> what data is accepted and what is not, as well as what meaning is given
> to the data that is accepted.
>

I agree with this idea, and the topic inspired me to explore how mixing
upper and lowercase hex oids might be "abused" today. Interestingly, I
found out it is possible to mix upper and lowercase formats within one
oid. Depending on output path, Git either normalizes the casing or
preserves the raw form stored. I didn't come up with a specific way to
take advantage of this behavior yet, but one could imagine a scenario
where downstream consumers of Git output each have their own way
of parsing such an ambiguously formatted oid and behave in different
ways that at best may cause confusion and at worst could be used
maliciously.

To reproduce a mixed case oid scenario:

    #!/bin/sh
    set -eu

    ( repo=$(mktemp -d); cd "$repo"
      export GIT_PAGER=cat
      git init -q
      git config user.name Tester && git config user.email tester@example.com

      echo one >f && git add f && git commit -qm base
      echo two >f && git commit -qam child

      # Re-spell the child's parent OID with a mixed-case tail, re-store the
      # object.  Nothing else about the commit changes.
      parent=$(git rev-parse HEAD^)
      upper=$(printf %s "$parent" | tr '[:lower:]' '[:upper:]')
      mixed=$(printf %s "$parent" | cut -c1-10)$(printf %s "$parent" |
cut -c11- | tr '[:lower:]' '[:upper:]')
      git cat-file commit HEAD | sed "s/^parent .*/parent $mixed/" >crafted-obj
      crafted=$(git hash-object -w -t commit --stdin <crafted-obj)
      git update-ref refs/heads/mixed "$crafted"

      echo "== the two commit objects differ by one field, case only =="
      git cat-file commit HEAD >canon-obj
      diff canon-obj crafted-obj || true

      echo
      echo "== they are distinct commits =="
      printf 'canonical: %s\nmixed    : %s\n' "$(git rev-parse HEAD)" "$crafted"

      echo
      echo "== both parent spellings resolve to the same object =="
      git rev-parse "$parent" "$upper"

      echo
      echo "== the one parent is spelled two ways, by output path =="
      printf 'raw (as stored) : '; git log -1 --pretty=raw mixed | sed
-n 's/^parent //p'
      printf '%%P  (normalized): '; git log -1 --format='%P' mixed
    )

produces:

    == the two commit objects differ by one field, case only ==
    2c2
    < parent f3b4ff525d82ddcec0cc8597842c73b72e4c5aba
    ---
    > parent f3b4ff525d82DDCEC0CC8597842C73B72E4C5ABA

    == they are distinct commits ==
    canonical: 52d41f6a261b49a18d38933b59a65f2dc919f9ac
    mixed    : ed20e85251a37eb01009faaaa8fbc23baf8bdc72

    == both parent spellings resolve to the same object ==
    f3b4ff525d82ddcec0cc8597842c73b72e4c5aba
    f3b4ff525d82ddcec0cc8597842c73b72e4c5aba

    == the one parent is spelled two ways, by output path ==
    raw (as stored) : f3b4ff525d82DDCEC0CC8597842C73B72E4C5ABA
    %P  (normalized): f3b4ff525d82ddcec0cc8597842c73b72e4c5aba