From bc35a5080d8a5395b39eb4dff9bf4adcd253ea2a Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 13 May 2026 14:50:59 -0700 Subject: [PATCH 1/2] Add regression tests for `BlameHunk` methods with missing signatures When a `BlameHunk` is created from an arbitrary buffer via `Blame::blame_buffer()`, the original and final committer and author can be missing. This leads to creating `Signature` objects holding null pointers; subsequent attempts to access the name or email of the signature leads to null pointer dereferences. The new tests are currently failing and exists to show the error - they could not be split up and marked with `#[should_panic]`, which does not appear to work here. This may be because the error message is that "thread caused non-unwinding panic. aborting." after the null pointer is dereferenced, and the attribute only works with unwinding panics, I'm not sure. The test also confirms that the non-signature methods still work with arbitrary hunks. --- src/blame.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/blame.rs b/src/blame.rs index 13f3294191..64e5a59e27 100644 --- a/src/blame.rs +++ b/src/blame.rs @@ -432,4 +432,93 @@ mod tests { assert_eq!(blame_buffer.iter().count(), 2); assert!(line.final_commit_id().is_zero()); } + + #[test] + fn buffer_signatures() { + // Regression tests for #1253 + let td = tempfile::TempDir::new().unwrap(); + let path = td.path(); + + let repo = crate::Repository::init(path).unwrap(); + + { + let mut config = repo.config().unwrap(); + config.set_str("user.name", "name").unwrap(); + config.set_str("user.email", "email").unwrap(); + + fs::write(&path.join("README.md"), "Testing").unwrap(); + + let mut index = repo.index().unwrap(); + index.add_path(&Path::new("README.md")).unwrap(); + index.write().unwrap(); + + let id = index.write_tree().unwrap(); + let tree = repo.find_tree(id).unwrap(); + let sig = repo.signature().unwrap(); + repo.commit(Some("HEAD"), &sig, &sig, "Add README.md", &tree, &[]) + .unwrap(); + } + + let blame = repo.blame_file(&Path::new("README.md"), None).unwrap(); + // This hunk is safe to use + let hunk = blame.get_index(0).unwrap(); + + { + let final_author = hunk.final_signature(); + assert_eq!(Some("name"), final_author.name()); + assert_eq!(Some("email"), final_author.email()); + + let final_committer = hunk.final_committer(); + assert_eq!(Some("name"), final_committer.name()); + assert_eq!(Some("email"), final_committer.email()); + + let original_author = hunk.orig_signature(); + assert_eq!(Some("name"), original_author.name()); + assert_eq!(Some("email"), original_author.email()); + + let original_committer = hunk.orig_committer(); + assert_eq!(Some("name"), original_committer.name()); + assert_eq!(Some("email"), original_committer.email()); + } + + let arbitrary = blame.blame_buffer(b"abc123").unwrap(); + let hunk = arbitrary.get_index(0).unwrap(); + // This hunk is NOT safe to use + // the final_signature, final_committer, orig_signature, and + // orig_committer pointers are all NULL + // But the other methods still work + { + let final_commit_id = hunk.final_commit_id(); + assert!(final_commit_id.is_zero()); + + let original_commit_id = hunk.orig_commit_id(); + assert!(original_commit_id.is_zero()); + + assert_eq!(1, hunk.final_start_line()); + assert_eq!(0, hunk.orig_start_line()); + assert_eq!(Some(Path::new("README.md")), hunk.path()); + assert_eq!(false, hunk.is_boundary()); + assert_eq!(1, hunk.lines_in_hunk()); + assert_eq!(None, hunk.summary()); + assert_eq!(None, hunk.summary_bytes()); + } + + { + let final_author = hunk.final_signature(); + assert_eq!(Some("name"), final_author.name()); + assert_eq!(Some("email"), final_author.email()); + + let final_committer = hunk.final_committer(); + assert_eq!(Some("name"), final_committer.name()); + assert_eq!(Some("email"), final_committer.email()); + + let original_author = hunk.orig_signature(); + assert_eq!(Some("name"), original_author.name()); + assert_eq!(Some("email"), original_author.email()); + + let original_committer = hunk.orig_committer(); + assert_eq!(Some("name"), original_committer.name()); + assert_eq!(Some("email"), original_committer.email()); + } + } } From 2eafc457a4c59a7eae03846fede70b44a2042635 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 13 May 2026 15:01:06 -0700 Subject: [PATCH 2/2] Make `BlameHunk` methods for `Signature`s return `Option`s The `BlameHunk` might have null pointers for the original and final committer and author. The following methods now return an `Option>` rather than a `Signature<'_>` * `BlameHunk::final_signature()` * `BlameHunk::final_committer()` * `BlameHunk::orig_signature()` * `BlameHunk::orig_committer()` Fixes #1253 --- examples/blame.rs | 2 +- src/blame.rs | 86 +++++++++++++++++++++++++++++------------------ 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/examples/blame.rs b/examples/blame.rs index 202989b3f0..5aedfc660b 100644 --- a/examples/blame.rs +++ b/examples/blame.rs @@ -81,7 +81,7 @@ fn run(args: &Args) -> Result<(), git2::Error> { for (i, line) in reader.lines().enumerate() { if let (Ok(line), Some(hunk)) = (line, blame.get_line(i + 1)) { - let sig = hunk.final_signature(); + let sig = hunk.final_signature().expect("Should have a signature"); println!( "{} {} <{}> {}", hunk.final_commit_id(), diff --git a/src/blame.rs b/src/blame.rs index 64e5a59e27..e0f1089a46 100644 --- a/src/blame.rs +++ b/src/blame.rs @@ -106,18 +106,28 @@ impl<'blame> BlameHunk<'blame> { unsafe { Oid::from_raw(&(*self.raw).final_commit_id) } } - /// Returns signature for the author of the final commit. + /// Returns signature for the author of the final commit, if present. /// /// The final commit is the one identified by [Self::final_commit_id()]. - pub fn final_signature(&self) -> Signature<'_> { - unsafe { signature::from_raw_const(self, (*self.raw).final_signature) } + pub fn final_signature(&self) -> Option> { + let ptr = unsafe { (*self.raw).final_signature }; + if ptr.is_null() { + None + } else { + Some(unsafe { signature::from_raw_const(self, ptr) }) + } } - /// Returns signature for the committer of the final commit. + /// Returns signature for the committer of the final commit, if present. /// /// The final commit is the one identified by [Self::final_commit_id()]. - pub fn final_committer(&self) -> Signature<'_> { - unsafe { signature::from_raw_const(self, (*self.raw).final_committer) } + pub fn final_committer(&self) -> Option> { + let ptr = unsafe { (*self.raw).final_committer }; + if ptr.is_null() { + None + } else { + Some(unsafe { signature::from_raw_const(self, ptr) }) + } } /// Returns line number where this hunk begins. @@ -136,18 +146,28 @@ impl<'blame> BlameHunk<'blame> { unsafe { Oid::from_raw(&(*self.raw).orig_commit_id) } } - /// Returns signature of the author of the original commit. + /// Returns signature of the author of the original commit, if present. /// /// The original commit is the one identified by [Self::orig_commit_id()]. - pub fn orig_signature(&self) -> Signature<'_> { - unsafe { signature::from_raw_const(self, (*self.raw).orig_signature) } + pub fn orig_signature(&self) -> Option> { + let ptr = unsafe { (*self.raw).orig_signature }; + if ptr.is_null() { + None + } else { + Some(unsafe { signature::from_raw_const(self, ptr) }) + } } - /// Returns signature of the committer of the original commit. + /// Returns signature of the committer of the original commit, if present. /// /// The original commit is the one identified by [Self::orig_commit_id()]. - pub fn orig_committer(&self) -> Signature<'_> { - unsafe { signature::from_raw_const(self, (*self.raw).orig_committer) } + pub fn orig_committer(&self) -> Option> { + let ptr = unsafe { (*self.raw).orig_committer }; + if ptr.is_null() { + None + } else { + Some(unsafe { signature::from_raw_const(self, ptr) }) + } } /// Returns line number where this hunk begins. @@ -411,14 +431,20 @@ mod tests { let hunk = blame.get_index(0).unwrap(); assert_eq!(hunk.final_commit_id(), commit); - assert_eq!(hunk.final_signature().name(), sig.name()); - assert_eq!(hunk.final_signature().email(), sig.email()); - assert_eq!(hunk.orig_signature().name(), sig.name()); - assert_eq!(hunk.orig_signature().email(), sig.email()); - assert_eq!(hunk.final_committer().name(), committer_sig.name()); - assert_eq!(hunk.final_committer().email(), committer_sig.email()); - assert_eq!(hunk.orig_committer().name(), committer_sig.name()); - assert_eq!(hunk.orig_committer().email(), committer_sig.email()); + assert_eq!(hunk.final_signature().unwrap().name(), sig.name()); + assert_eq!(hunk.final_signature().unwrap().email(), sig.email()); + assert_eq!(hunk.orig_signature().unwrap().name(), sig.name()); + assert_eq!(hunk.orig_signature().unwrap().email(), sig.email()); + assert_eq!(hunk.final_committer().unwrap().name(), committer_sig.name()); + assert_eq!( + hunk.final_committer().unwrap().email(), + committer_sig.email() + ); + assert_eq!(hunk.orig_committer().unwrap().name(), committer_sig.name()); + assert_eq!( + hunk.orig_committer().unwrap().email(), + committer_sig.email() + ); assert_eq!(hunk.final_start_line(), 1); assert_eq!(hunk.path(), Some(Path::new("foo/bar"))); assert_eq!(hunk.lines_in_hunk(), 0); @@ -464,19 +490,19 @@ mod tests { let hunk = blame.get_index(0).unwrap(); { - let final_author = hunk.final_signature(); + let final_author = hunk.final_signature().unwrap(); assert_eq!(Some("name"), final_author.name()); assert_eq!(Some("email"), final_author.email()); - let final_committer = hunk.final_committer(); + let final_committer = hunk.final_committer().unwrap(); assert_eq!(Some("name"), final_committer.name()); assert_eq!(Some("email"), final_committer.email()); - let original_author = hunk.orig_signature(); + let original_author = hunk.orig_signature().unwrap(); assert_eq!(Some("name"), original_author.name()); assert_eq!(Some("email"), original_author.email()); - let original_committer = hunk.orig_committer(); + let original_committer = hunk.orig_committer().unwrap(); assert_eq!(Some("name"), original_committer.name()); assert_eq!(Some("email"), original_committer.email()); } @@ -505,20 +531,16 @@ mod tests { { let final_author = hunk.final_signature(); - assert_eq!(Some("name"), final_author.name()); - assert_eq!(Some("email"), final_author.email()); + assert!(final_author.is_none()); let final_committer = hunk.final_committer(); - assert_eq!(Some("name"), final_committer.name()); - assert_eq!(Some("email"), final_committer.email()); + assert!(final_committer.is_none()); let original_author = hunk.orig_signature(); - assert_eq!(Some("name"), original_author.name()); - assert_eq!(Some("email"), original_author.email()); + assert!(original_author.is_none()); let original_committer = hunk.orig_committer(); - assert_eq!(Some("name"), original_committer.name()); - assert_eq!(Some("email"), original_committer.email()); + assert!(original_committer.is_none()); } } }