RFC patch, 6 partsGit 3.0: restrict hex object IDs to lowercase only

41 messages between Jul 29, 2026 and Sep 7, 2026, from brian m. carlson, Junio C Hamano, Jeff King, Michael Montalbo, Phillip Wood, Elijah Newren.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

brian m. carlsonJul 29, 2026, 23:32 UTC on lore

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(-)

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to 7 files +9 −9

hex-ll.h, hex.c, mailinfo.c, ref-filter.c, strbuf.c, url.c, urlmatch.c

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;

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to 6 files +8 −6

diagnose.c, hex-ll.h, hex.c, http-push.c, notes.c, object-file.c

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

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to 4 files +51 −10

color.c, hex-ll.c, hex-ll.h, pkt-line.c

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)

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to 7 files +12 −10

builtin/index-pack.c, diagnose.c, hex-ll.c, hex-ll.h, http-push.c, notes.c, object-file.c

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

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to object-name.c +3 −10

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;

brian m. carlsonJul 29, 2026, 23:32 UTC in reply to brian m. carlson on lore

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

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(-)

Show changes to 4 files +16 −2

Documentation/BreakingChanges.adoc, hex-ll.h, t/t1503-rev-parse-verify.sh, t/t5324-split-commit-graph.sh

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

Junio C HamanoJul 30, 2026, 08:21 UTC in reply to brian m. carlson on lore

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

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

Show 18 quoted lines

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

brian m. carlsonJul 30, 2026, 21:18 UTC in reply to Junio C Hamano on lore

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

On 2026-07-30 at 08:21:46, Junio C Hamano wrote:

Show 26 quoted lines

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

Junio C HamanoJul 31, 2026, 03:24 UTC in reply to brian m. carlson on lore

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

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

Show 14 quoted lines

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

Show 13 quoted lines

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

Junio C HamanoJul 31, 2026, 07:38 UTC in reply to brian m. carlson on lore

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

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

Show 12 quoted lines

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

Show 69 quoted lines

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

Show 32 quoted lines

>  
>  /*
> @@ -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)

Junio C HamanoJul 31, 2026, 07:38 UTC in reply to brian m. carlson on lore

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

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

Show 9 quoted lines

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

Junio C HamanoJul 31, 2026, 07:48 UTC in reply to brian m. carlson on lore

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

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

Junio C HamanoJul 31, 2026, 12:33 UTC in reply to Junio C Hamano on lore

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

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

Show 8 quoted lines

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

Jeff KingAug 1, 2026, 14:35 UTC in reply to Junio C Hamano on lore

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

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

Show 31 quoted lines

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

Jeff KingAug 1, 2026, 14:45 UTC in reply to brian m. carlson on lore

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

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

Show 6 quoted lines

> 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

Junio C HamanoAug 1, 2026, 18:22 UTC in reply to Jeff King on lore

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

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.

Show 6 quoted lines

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

brian m. carlsonAug 2, 2026, 21:55 UTC in reply to Jeff King on lore

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

On 2026-08-01 at 14:45:27, Jeff King wrote:

Show 24 quoted lines

> 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

brian m. carlsonAug 2, 2026, 22:09 UTC in reply to Junio C Hamano on lore

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

On 2026-07-31 at 07:48:14, Junio C Hamano wrote:

Show 8 quoted lines

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

Show 7 quoted lines

> 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

Junio C HamanoAug 4, 2026, 19:32 UTC in reply to brian m. carlson on lore

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

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

brian m. carlsonAug 4, 2026, 21:46 UTC in reply to Junio C Hamano on lore

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

On 2026-08-04 at 19:32:18, Junio C Hamano wrote:

Show 5 quoted lines

> 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

Michael MontalboAug 5, 2026, 03:09 UTC in reply to brian m. carlson on lore

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

On Sun, Aug 2, 2026 at 3:10 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 5 quoted lines

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

Phillip WoodAug 25, 2026, 09:04 UTC in reply to brian m. carlson on lore

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

Hi brian

On 30/07/2026 00:32, brian m. carlson wrote:

Show 18 quoted lines

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

Can you say a bit more about the security problems please - I'm trying to understand why ABCDEF is a security risk when abcdef^0 isn't.

Thanks

Phillip

Show 86 quoted lines

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

Junio C HamanoAug 25, 2026, 15:39 UTC in reply to brian m. carlson on lore

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

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

Show 17 quoted lines

> 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(-)

Now this is an embarrassingly late review. I hope this is not a sign that nobody is paying attention on the list these days X-<.

Show 15 quoted lines

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

That's 68-6f if I am not mistaken ;-).

Junio C HamanoAug 25, 2026, 16:11 UTC in reply to brian m. carlson on lore

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

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

Show 14 quoted lines

> 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(-)

This is a hard-to-review patch in the sense that what we see in the patch may be perfectly good, but we cannot see what is left out, either by mistake or by misdesign. So I checked out the state with this patch (and no later ones) applied, and eyeballed the output of

    $ git grep -n -e HEX_KIND_MIXED

At this step, a few explicit uses of HEX_KIND_MIXED remain that I think should have been converted to HEX_KIND_OID.

 * builtin/index-pack.c:repack_local_links() spawns a pack-objects
   process and reads its output.  As we are reading from a known
   version of Git (i.e., pack-objects that came with the index-pack
   that runs this code), we do not need to be lenient and can use
   HEX_KIND_OID here.

 * notes.c:load_subtree() has two calls to hex_to_bytes() to read
   paths in a notes tree, and this patch updates only one to use
   HEX_KIND_OID, leaving the other one HEX_KIND_MIXED, which we
   probably should change at the same time (if there is a valid
   reason, it deserves an in-code comment to explain it).

The remaining uses of HEX_KIND_MIXED look mostly OK.

 - color.c uses MIXED to decode things like #AAFF00, which will be
   correct forever.

 - mailinfo.c uses MIXED to decode Q encoding, and we have no power
   or business to forbid uppercase hex there.

 - pkt-line.c:packet_length() uses MIXED to decode the packet length
   expressed in the four hex digits at the beginning.  We could
   forbid uppercase hex there (our length bytes have always been
   lowercase) if we wanted to, but HEX_KIND_OID is not the enum to
   use to do so.

 - ref-filter.c:append_literal() is similar to the next one.

 - strbuf.c:strbuf_expand_literal() uses MIXED to decode %0A into line
   feed, etc.  We could forbid uppercase hex there if we wanted to,
   but HEX_KIND_OID is not the enum to use to do so.

 - url.c:url_decode_internal() uses MIXED to decode %2F into '/',
   etc., and we have no power or business to forbid uppercase hex
   there.

 - urlmatch.c:append_normalized_escapes() uses MIXED to decode %2F
   into '/' before escaping it back with %02X.

Junio C HamanoAug 25, 2026, 16:19 UTC in reply to brian m. carlson on lore

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

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

Show 37 quoted lines

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

When hex_out[] is given by the caller, they used to get a downcased version of object name. After your planned transition to forbid uppercase hex, they will get an error, which is exactly as you intend.

However, during transition, they will *not* get an error (as KIND_OID is still KIND_MIXED before the transition), and they will see the hex_out[] filled with object names in the original case, without canonicalization that the original code gave them.

While seemingly harmless, because repo_for_each_abbrev() doesn't seem to malfunction on uppercase string metadata for disambiguation), we may want to mention that this changes API contract (until we forbid uppercase input altogether).

Junio C HamanoAug 25, 2026, 16:36 UTC in reply to brian m. carlson on lore

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

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

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

This was a bit hard to read and puzzled me, as you have two "A" and change only one of them to "b".

Is the idea that we wanted to make sure we use lowercase letters, because we do not want to see the tested "verify" command fail for now-forbidden uppercase hex but we want the command to read the data as valid hex and fail because it notices the corruption? So the first hunk is a no-op change (i.e., the first hash identifier on the first line is corrupt with the 30-th char in the file replaced with either 'a' or 'A'), while the second hunk is not (i.e., the second hash identifier on the second line in the file is corrupt with the 70-th char in the file replaced with 'A' but it is OK with 'a' because in the SHA-256 mode, the correct character for the place happens to be 'a')? It is puzzling if that is the case, because what this series wanted to tighten was that we used to treat hex chars case insensitively. So, if 'a' happened to be the right uncorrupted value for position 70, how did the original that replaced it to 'A' tested a "corrupted" state?

Show 22 quoted lines

> 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

Elijah NewrenAug 25, 2026, 19:44 UTC in reply to brian m. carlson on lore

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

On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 7 quoted lines

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

As Junio noted, you may want to call out that your replacement drops the case-normalization that the former parse_oid_prefix() provided.

[...]

> -               unsigned char val;

[...]

> +               int val = hexval(c, HEX_KIND_OID);
> +
> +               if (val < 0)
>                         return -1;

[...]

>                if (oid_out) {
>                        if (!(i & 1))
>                                val <<= 4;
>                        oid_out->hash[i >> 1] |= val;

hexval returns unsigned int. Is there a risk that someone "tries to fix" that discrepancy by changing val to unsigned int here, inadvertently causing the `if` immediately below to become dead code?

In patch 1, in hex2chr, you used a (val & ~0xf) check together with an unsigned int val; would that make sense here, or is that overkill?

Elijah NewrenAug 25, 2026, 19:44 UTC in reply to brian m. carlson on lore

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

On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 7 quoted lines

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

[...snip...]

> -               corrupt_file "$graphdir/commit-graph-chain" 30 "A" &&
> +               corrupt_file "$graphdir/commit-graph-chain" 30 "a" &&

[...]

> -               corrupt_file "$graphdir/commit-graph-chain" 70 "A" &&
> +               corrupt_file "$graphdir/commit-graph-chain" 70 "b" &&

which "A" is the commit message referring to?

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

You've indented with tabs here while the surrounding paragraphs use spaces; is that going to mess up rendering?

brian m. carlsonAug 25, 2026, 21:36 UTC in reply to Phillip Wood on lore

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

On 2026-08-25 at 09:04:36, Phillip Wood wrote:

Show 24 quoted lines

> Hi brian
> 
> On 30/07/2026 00:32, brian m. carlson wrote:
> > 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.
> 
> Can you say a bit more about the security problems please - I'm trying to
> understand why ABCDEF is a security risk when abcdef^0 isn't.

There's two cases I've seen. The first is that people assume an object ID is unique in hex form. So if we have some policy to enforce, say, that we can't allow certain objects, people will check against the lowercase version when they may get the uppercase version somewhere (say, user input or a specially crafted protocol message), which bypasses the check.

The other case is where we try to distinguish between an object ID and a ref, branch, or tag. If our regexp has `[0-9a-f]{40}` or `[0-9a-f]{64}` and we assume that if it matches it's an object ID and if it's not it's a ref, that's not correct here. We'd need to match the uppercase version as well, but experience shows that people overwhelmingly do not do that.

-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

brian m. carlsonAug 25, 2026, 21:41 UTC in reply to Elijah Newren on lore

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

On 2026-08-25 at 19:44:34, Elijah Newren wrote:

Show 12 quoted lines

> On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > 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.
> 
> As Junio noted, you may want to call out that your replacement drops
> the case-normalization that the former parse_oid_prefix() provided.

Will fix in v2.

Show 19 quoted lines

> [...]
> > -               unsigned char val;
> [...]
> > +               int val = hexval(c, HEX_KIND_OID);
> > +
> > +               if (val < 0)
> >                         return -1;
> [...]
> >                if (oid_out) {
> >                        if (!(i & 1))
> >                                val <<= 4;
> >                        oid_out->hash[i >> 1] |= val;
> 
> hexval returns unsigned int.  Is there a risk that someone "tries to
> fix" that discrepancy by changing val to unsigned int here,
> inadvertently causing the `if` immediately below to become dead code?
> 
> In patch 1, in hex2chr, you used a (val & ~0xf) check together with an
> unsigned int val; would that make sense here, or is that overkill?

I can re-roll with an appropriate change, sure. I think that we'd need to have a slightly different check, but I'll tidy it up accordingly.

-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

brian m. carlsonAug 25, 2026, 21:44 UTC in reply to Junio C Hamano on lore

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

On 2026-08-25 at 15:39:43, Junio C Hamano wrote:

Show 41 quoted lines

> "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>
> > ---
> >  color.c    |  2 +-
> >  hex-ll.c   | 37 ++++++++++++++++++++++++++++++++++++-
> >  hex-ll.h   | 14 ++++++++++----
> >  pkt-line.c |  8 ++++----
> >  4 files changed, 51 insertions(+), 10 deletions(-)
> 
> 
> Now this is an embarrassingly late review.  I hope this is not a
> sign that nobody is paying attention on the list these days X-<.
> 
> > +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 */
> 
> That's 68-6f if I am not mistaken ;-).

So it is.  Will fix in v2.

I think I accidentally included the uppercase but not lowercase variants when creating the original array and then copied and pasted the line, but messed up the comment.

-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

Phillip WoodSep 7, 2026, 13:37 UTC in reply to brian m. carlson on lore

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

Hi brian

Thanks for the examples, sorry it has taken so long for me to respond

On 25/08/2026 22:36, brian m. carlson wrote:

Show 32 quoted lines

> On 2026-08-25 at 09:04:36, Phillip Wood wrote:
>> Hi brian
>>
>> On 30/07/2026 00:32, brian m. carlson wrote:
>>> 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.
>>
>> Can you say a bit more about the security problems please - I'm trying to
>> understand why ABCDEF is a security risk when abcdef^0 isn't.
> 
> There's two cases I've seen.  The first is that people assume an object
> ID is unique in hex form.  So if we have some policy to enforce, say,
> that we can't allow certain objects, people will check against the
> lowercase version when they may get the uppercase version somewhere
> (say, user input or a specially crafted protocol message), which
> bypasses the check.

I'm a bit unclear how upper case hex can defeat that policy but ref names dont. If the input is not being checked to ensure it is a hex object id wont a ref pointing to a commit we're trying to restrict access to also defeat the check?

Show 6 quoted lines

> The other case is where we try to distinguish between an object ID and a
> ref, branch, or tag.  If our regexp has `[0-9a-f]{40}` or `[0-9a-f]{64}`
> and we assume that if it matches it's an object ID and if it's not it's
> a ref, that's not correct here.  We'd need to match the uppercase
> version as well, but experience shows that people overwhelmingly do not
> do that.

That makes more sense to me. It also makes me wonder if we should forbid refnames where the last component looks like an object id.

Thanks

Phillip

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 0/7] Git 3.0: restrict hex object IDs to lowercase only

This series removes the ability to use uppercase hex in object IDs in Git 3.0.

Changes from v1:

* Fix incorrect hex range in comment.
* Update commit messages.
* Move t5324 fixes to a separate commit and expand commit message.
* Restore lowercasing in parse_oid_prefix.
* Possibly other miscellaneous changes which I have forgotten.

brian m. carlson (7):
  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
  t5324: adjust tests for corrupt commit-graph
  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                      | 15 +++---------
 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, 91 insertions(+), 39 deletions(-)

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 4/7] hex: label usages of hex parsing for object IDs

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(-)

Show changes to 6 files +8 −6

diagnose.c, hex-ll.h, hex.c, http-push.c, notes.c, object-file.c

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 b5c5bad3db..43b4b61c70 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1031,13 +1031,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 892be4bbb2..c4bb3263cc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1079,7 +1079,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);

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 2/7] hex: allow specifying hex type with hex2chr

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(-)

Show changes to 7 files +9 −9

hex-ll.h, hex.c, mailinfo.c, ref-filter.c, strbuf.c, url.c, urlmatch.c

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 bdf54f6f59..cd02677e42 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3636,7 +3636,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;

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 3/7] hex: make hex_to_bytes accept kind of hex to use

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(-)

Show changes to 7 files +12 −10

builtin/index-pack.c, diagnose.c, hex-ll.c, hex-ll.h, http-push.c, notes.c, object-file.c

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 6b2a87e2d3..44ac72ef1e 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 8f5a4e4644..3b7d22a824 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 b8f3faaed9..b5c5bad3db 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1031,12 +1031,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 a4cbf8b081..892be4bbb2 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1079,7 +1079,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);

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 1/7] hex: add functionality for lowercase-only hex

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(-)

Show changes to 4 files +51 −10

color.c, hex-ll.c, hex-ll.h, pkt-line.c

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..8f5a4e4644 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-6f */
+	 -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)

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 5/7] object-name: use hexval

Because hexval returns an unsigned int, check to see if the value is invalid by looking for any bits beyond a single unsigned character. In addition, be sure to continue to force the hexadecimal value to lowercase.

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

Show changes to object-name.c +4 −11

diff --git a/object-name.c b/object-name.c
index 4eda8c8eac..8f2da51547 100644
--- a/object-name.c
+++ b/object-name.c
@@ -236,20 +236,13 @@ 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 & ~0xff)
 			return -1;
-		}
 
 		if (hex_out)
-			hex_out[i] = c;
+			hex_out[i] = tolower(c);
 		if (oid_out) {
 			if (!(i & 1))
 				val <<= 4;

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 6/7] t5324: adjust tests for corrupt commit-graph

In a future commit, we'll no longer allow uppercase object IDs. When that happens, t5324 will fail because the error message from corrupting commit-graph files will be different. The current test looks for an error message that occurs when the commit-graph file has a name that is parseable as a valid object ID but is not a valid commit-graph file (which, on case-sensitive systems, is of course checked case sensitively).

However, in the future, we need the object ID to continue to be valid, and that means that the hex character we use must be lowercase. In SHA-256, though, one of the characters we want to corrupt is legitimately already an "a", so switch to using the character "b" in both of these cases, which is the correct character for neither SHA-1 nor SHA-256. If we adopt another hash algorithm in the future, we may need to adjust these values again.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t5324-split-commit-graph.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to t/t5324-split-commit-graph.sh +2 −2

diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index bf7ba0e558..53cddfe437 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 "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
@@ -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

brian m. carlsonSep 7, 2026, 19:59 UTC in reply to brian m. carlson on lore

[PATCH v2 7/7] hex: allow only lowercase object IDs in breaking changes mode

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 had only one test in our codebase that failed when we reject uppercase object IDs.

In preparation for Git 3.0, only allow lowercase hex object IDs in breaking changes mode and document this as well. Add a new test to verify we reject new uppercase object IDs.

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

Show changes to 3 files +14 −0

Documentation/BreakingChanges.adoc, hex-ll.h, t/t1503-rev-parse-verify.sh

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