[PATCH v1] hash: handle invalid algorithms when formatting object IDs

[PATCH v1] hash: handle invalid algorithms when formatting object IDs

[shrimech]

ObjectID::algo is publicly mutable, so an ObjectID may contain an algorithm
identifier that is not recognized by HashAlgorithm. Display previously
unwrapped as_slice(), which returns an error for such identifiers, causing
formatting to panic.

Format the full hash storage for invalid identifiers and append an
invalid-hash-algo marker containing the unsupported identifier. Add a
regression test covering this case while preserving the existing output for
valid algorithms.

Signed-off-by: shrimech <eddinen77@gmail.com>
---

Notes:
    Base Ref: v2.56.0
    Web-Diff: https://github.com/larsxschneider/git/commit/c95530cf38
    Checkout: git fetch https://github.com/larsxschneider/git fix/objectid-display-invalid-algo-v1 && git checkout c95530cf38

 src/hash.rs | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/hash.rs b/src/hash.rs
index e1f2d31fc3..87242d58b1 100644
--- a/src/hash.rs
+++ b/src/hash.rs
@@ -78,13 +78,19 @@ impl ObjectID {
 }
 
 impl Display for ObjectID {
-    /// Format this object ID as a hex object ID.
+    /// Format this object ID as a hex object ID, or mark it if its algorithm is invalid.
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let hash = self.as_slice().unwrap();
+        let hash = match self.as_slice() {
+            Ok(hash) => hash,
+            Err(_) => &self.hash,
+        };
         for x in hash {
             write!(f, "{:02x}", x)?;
         }
-        Ok(())
+        match self.algo() {
+            Ok(_) => Ok(()),
+            Err(e) => write!(f, ":invalid-hash-algo-{}", e.0),
+        }
     }
 }
 
@@ -366,7 +372,7 @@ pub mod c {
 
 #[cfg(test)]
 mod tests {
-    use super::{CryptoDigest, HashAlgorithm, ObjectID};
+    use super::{CryptoDigest, HashAlgorithm, ObjectID, GIT_MAX_RAWSZ};
     use std::io::Write;
 
     fn all_algos() -> &'static [HashAlgorithm] {
@@ -439,6 +445,19 @@ mod tests {
         }
     }
 
+    #[test]
+    fn invalid_object_ids_format_without_panicking() {
+        let oid = ObjectID {
+            hash: [0xab; GIT_MAX_RAWSZ],
+            algo: 0,
+        };
+
+        assert_eq!(
+            format!("{}", oid),
+            format!("{}:invalid-hash-algo-0", "ab".repeat(GIT_MAX_RAWSZ))
+        );
+    }
+
     #[test]
     fn hasher_works_correctly() {
         for algo in all_algos() {

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
-- 
2.47.3

[PATCH v1] hash: handle invalid algorithms when formatting object IDs

[shrimech]

ObjectID::algo is publicly mutable, so an ObjectID may contain an algorithm
identifier that is not recognized by HashAlgorithm. Display previously
unwrapped as_slice(), which returns an error for such identifiers, causing
formatting to panic.

Format the full hash storage for invalid identifiers and append an
invalid-hash-algo marker containing the unsupported identifier. Add a
regression test covering this case while preserving the existing output for
valid algorithms.

Signed-off-by: shrimech <eddinen77@gmail.com>
---

Notes:
    Base Ref: v2.56.0
    Web-Diff: https://github.com/shrimech/git/commit/c95530cf38
    Checkout: git fetch https://github.com/shrimech/git fix/objectid-display-invalid-algo-v1 && git checkout c95530cf38

 src/hash.rs | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/hash.rs b/src/hash.rs
index e1f2d31fc3..87242d58b1 100644
--- a/src/hash.rs
+++ b/src/hash.rs
@@ -78,13 +78,19 @@ impl ObjectID {
 }
 
 impl Display for ObjectID {
-    /// Format this object ID as a hex object ID.
+    /// Format this object ID as a hex object ID, or mark it if its algorithm is invalid.
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let hash = self.as_slice().unwrap();
+        let hash = match self.as_slice() {
+            Ok(hash) => hash,
+            Err(_) => &self.hash,
+        };
         for x in hash {
             write!(f, "{:02x}", x)?;
         }
-        Ok(())
+        match self.algo() {
+            Ok(_) => Ok(()),
+            Err(e) => write!(f, ":invalid-hash-algo-{}", e.0),
+        }
     }
 }
 
@@ -366,7 +372,7 @@ pub mod c {
 
 #[cfg(test)]
 mod tests {
-    use super::{CryptoDigest, HashAlgorithm, ObjectID};
+    use super::{CryptoDigest, HashAlgorithm, ObjectID, GIT_MAX_RAWSZ};
     use std::io::Write;
 
     fn all_algos() -> &'static [HashAlgorithm] {
@@ -439,6 +445,19 @@ mod tests {
         }
     }
 
+    #[test]
+    fn invalid_object_ids_format_without_panicking() {
+        let oid = ObjectID {
+            hash: [0xab; GIT_MAX_RAWSZ],
+            algo: 0,
+        };
+
+        assert_eq!(
+            format!("{}", oid),
+            format!("{}:invalid-hash-algo-0", "ab".repeat(GIT_MAX_RAWSZ))
+        );
+    }
+
     #[test]
     fn hasher_works_correctly() {
         for algo in all_algos() {

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
-- 
2.47.3

Re: [PATCH v1] hash: handle invalid algorithms when formatting object IDs

[Tanga Joris]

Hello shrimech,

Thank you for the patch.

I tried it on top of master (0ffa3e35c7). "cargo test" passes, and
the new test fails without your change to fmt(), so it does catch
the problem.

On Tue, 3 Nov 2026 at 07:42, shrimech <eddinen77@gmail.com> wrote:
> +            Err(e) => write!(f, ":invalid-hash-algo-{}", e.0),

I have one question. In the C code, algo 0 is not treated as an
error when printing: hash_to_hex_algop_r() in hex.c falls back to
the default hash ("Our struct object_id has been memset to 0, so
default to printing using the default hash"). Your test uses algo 0
as the invalid case, so for algo 0 the Rust output would be
different from C. Is that intended?

brian m. carlson wrote most of src/hash.rs, so I have added him on
Cc, I hope that is okay:)

Also the first patch versions can be sent as [PATCH] instead of
[PATCH v1]. Either way works. The v1 format is less common though.

Thanks,
Tanga

Re: [PATCH v1] hash: handle invalid algorithms when formatting object IDs

[brian m. carlson]

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

On 2026-11-05 at 14:47:00, Tanga Joris wrote:
> Hello shrimech,
> 
> Thank you for the patch.
> 
> I tried it on top of master (0ffa3e35c7). "cargo test" passes, and
> the new test fails without your change to fmt(), so it does catch
> the problem.
> 
> On Tue, 3 Nov 2026 at 07:42, shrimech <eddinen77@gmail.com> wrote:
> > +            Err(e) => write!(f, ":invalid-hash-algo-{}", e.0),
> 
> I have one question. In the C code, algo 0 is not treated as an
> error when printing: hash_to_hex_algop_r() in hex.c falls back to
> the default hash ("Our struct object_id has been memset to 0, so
> default to printing using the default hash"). Your test uses algo 0
> as the invalid case, so for algo 0 the Rust output would be
> different from C. Is that intended?

This is not a valid configuration.  I have a patch on my sha256-interop
branch, 5b208e4a3d34ffba0fbeafe991643d6d47a5caa0, that should make the
members not public.  It depends partially on things other in that
series, but it should be fine to drop those pieces if you want to pick
that up and send it in.

If you pass the structure from C, it's intended that it panics if algo
is zero.  That's because we don't want to rely on the behaviour of
depending on a mutable global in Rust, since that's unsafe and leads to
action at a distance.  We really should just drop that fallback and fix
all of the relevant callers at some point.

> brian m. carlson wrote most of src/hash.rs, so I have added him on
> Cc, I hope that is okay:)

Minor nit: I use they/them pronouns.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA
Speaking for myself only

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