[CString panic reduction] Reduce panics without changing return types

src/lib.rs

@@ -955,7 +955,8 @@ impl ObjectType {
     /// Convert a string object type representation to its object type.
     #[expect(clippy::should_implement_trait)]
     pub fn from_str(s: &str) -> Option<ObjectType> {
-        let raw = unsafe { call!(raw::git_object_string2type(CString::new(s).unwrap())) };
+        let cstr = CString::new(s).ok()?;
+        let raw = unsafe { call!(raw::git_object_string2type(cstr)) };
         ObjectType::from_raw(raw)
     }
 }
@@ -1635,6 +1636,11 @@ mod tests {
         assert!(ObjectType::Blob.is_loose());
     }
 
+    #[test]
+    fn object_type_invalid() {
+        assert_eq!(None, ObjectType::from_str("ab\x0012"));
+    }
+
     #[test]
     fn convert_filemode() {
         assert_eq!(i32::from(FileMode::Blob), 0o100644);

src/refspec.rs

@@ -37,7 +37,9 @@ impl<'remote> Refspec<'remote> {
 
     /// Check if a refspec's destination descriptor matches a reference
     pub fn dst_matches(&self, refname: &str) -> bool {
-        let refname = CString::new(refname).unwrap();
+        let Ok(refname) = CString::new(refname) else {
+            return false;
+        };
         unsafe { raw::git_refspec_dst_matches(self.raw, refname.as_ptr()) == 1 }
     }
 
@@ -53,7 +55,9 @@ impl<'remote> Refspec<'remote> {
 
     /// Check if a refspec's source descriptor matches a reference
     pub fn src_matches(&self, refname: &str) -> bool {
-        let refname = CString::new(refname).unwrap();
+        let Ok(refname) = CString::new(refname) else {
+            return false;
+        };
         unsafe { raw::git_refspec_src_matches(self.raw, refname.as_ptr()) == 1 }
     }
 
@@ -74,7 +78,7 @@ impl<'remote> Refspec<'remote> {
 
     /// Transform a reference to its target following the refspec's rules
     pub fn transform(&self, name: &str) -> Result<Buf, Error> {
-        let name = CString::new(name).unwrap();
+        let name = CString::new(name)?;
         let buf = Buf::new();
         unsafe {
             try_call!(raw::git_refspec_transform(
@@ -88,7 +92,7 @@ impl<'remote> Refspec<'remote> {
 
     /// Transform a target reference to its source reference following the refspec's rules
     pub fn rtransform(&self, name: &str) -> Result<Buf, Error> {
-        let name = CString::new(name).unwrap();
+        let name = CString::new(name)?;
         let buf = Buf::new();
         unsafe {
             try_call!(raw::git_refspec_rtransform(
@@ -114,3 +118,90 @@ impl<'remote> Binding for Refspec<'remote> {
         self.raw
     }
 }
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn dst_matches_invalid() {
+        let (_td, repo) = crate::test::repo_init();
+        repo.remote("origin", "https://github.com/rust-lang/git2-rs")
+            .expect("Remote added");
+        let remote = repo.find_remote("origin").expect("Remote exists");
+        let specs: Vec<_> = remote.refspecs().collect();
+        assert_eq!(1, specs.len());
+        assert_eq!(
+            "+refs/heads/*:refs/remotes/origin/*",
+            specs[0].str().expect("Valid string")
+        );
+
+        assert!(!specs[0].dst_matches("ab\x0012"));
+    }
+
+    #[test]
+    fn src_matches_invalid() {
+        let (_td, repo) = crate::test::repo_init();
+        repo.remote("origin", "https://github.com/rust-lang/git2-rs")
+            .expect("Remote added");
+        let remote = repo.find_remote("origin").expect("Remote exists");
+        let specs: Vec<_> = remote.refspecs().collect();
+        assert_eq!(1, specs.len());
+        assert_eq!(
+            "+refs/heads/*:refs/remotes/origin/*",
+            specs[0].str().expect("Valid string")
+        );
+
+        assert!(!specs[0].src_matches("ab\x0012"));
+    }
+
+    #[test]
+    fn transform_invalid() {
+        let (_td, repo) = crate::test::repo_init();
+        repo.remote("origin", "https://github.com/rust-lang/git2-rs")
+            .expect("Remote added");
+        let remote = repo.find_remote("origin").expect("Remote exists");
+        let specs: Vec<_> = remote.refspecs().collect();
+        assert_eq!(1, specs.len());
+        assert_eq!(
+            "+refs/heads/*:refs/remotes/origin/*",
+            specs[0].str().expect("Valid string")
+        );
+
+        // Cannot use unwrap_err() because Buf does not implement Debug
+        let result = match specs[0].transform("ab\x0012") {
+            Ok(_) => panic!("Expected an err"),
+            Err(e) => e,
+        };
+        assert_eq!(
+            crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            ),
+            result,
+        );
+    }
+
+    #[test]
+    fn rtransform_invalid() {
+        let (_td, repo) = crate::test::repo_init();
+        repo.remote("origin", "https://github.com/rust-lang/git2-rs")
+            .expect("Remote added");
+        let remote = repo.find_remote("origin").expect("Remote exists");
+        let specs: Vec<_> = remote.refspecs().collect();
+        assert_eq!(1, specs.len());
+        assert_eq!(
+            "+refs/heads/*:refs/remotes/origin/*",
+            specs[0].str().expect("Valid string")
+        );
+
+        // Cannot use unwrap_err() because Buf does not implement Debug
+        let result = match specs[0].rtransform("ab\x0012") {
+            Ok(_) => panic!("Expected an err"),
+            Err(e) => e,
+        };
+        assert_eq!(
+            crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            ),
+            result,
+        );
+    }
+}

src/transaction.rs

@@ -38,7 +38,7 @@ impl<'repo> Binding for Transaction<'repo> {
 impl<'repo> Transaction<'repo> {
     /// Lock the specified reference by name.
     pub fn lock_ref(&mut self, refname: &str) -> Result<(), Error> {
-        let refname = CString::new(refname).unwrap();
+        let refname = CString::new(refname)?;
         unsafe {
             try_call!(raw::git_transaction_lock_ref(self.raw, refname));
         }
@@ -59,8 +59,8 @@ impl<'repo> Transaction<'repo> {
         reflog_signature: Option<&Signature<'_>>,
         reflog_message: &str,
     ) -> Result<(), Error> {
-        let refname = CString::new(refname).unwrap();
-        let reflog_message = CString::new(reflog_message).unwrap();
+        let refname = CString::new(refname)?;
+        let reflog_message = CString::new(reflog_message)?;
         unsafe {
             try_call!(raw::git_transaction_set_target(
                 self.raw,
@@ -87,9 +87,9 @@ impl<'repo> Transaction<'repo> {
         reflog_signature: Option<&Signature<'_>>,
         reflog_message: &str,
     ) -> Result<(), Error> {
-        let refname = CString::new(refname).unwrap();
-        let target = CString::new(target).unwrap();
-        let reflog_message = CString::new(reflog_message).unwrap();
+        let refname = CString::new(refname)?;
+        let target = CString::new(target)?;
+        let reflog_message = CString::new(reflog_message)?;
         unsafe {
             try_call!(raw::git_transaction_set_symbolic_target(
                 self.raw,
@@ -113,7 +113,7 @@ impl<'repo> Transaction<'repo> {
     /// written to the log (i.e. the `reflog_signature` and `reflog_message`
     /// parameters will be ignored).
     pub fn set_reflog(&mut self, refname: &str, reflog: Reflog) -> Result<(), Error> {
-        let refname = CString::new(refname).unwrap();
+        let refname = CString::new(refname)?;
         unsafe {
             try_call!(raw::git_transaction_set_reflog(
                 self.raw,
@@ -129,7 +129,7 @@ impl<'repo> Transaction<'repo> {
     ///
     /// The reference must have been locked via `lock_ref`.
     pub fn remove(&mut self, refname: &str) -> Result<(), Error> {
-        let refname = CString::new(refname).unwrap();
+        let refname = CString::new(refname)?;
         unsafe {
             try_call!(raw::git_transaction_remove(self.raw, refname));
         }
@@ -314,4 +314,120 @@ mod tests {
             Err(e) if is_not_locked_err(&e)
         ))
     }
+
+    #[test]
+    fn invalid_lock_ref() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.lock_ref("ab\x0012");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_target_refname() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let oid = Oid::from_bytes(&[1u8; 20]).unwrap();
+        let result = tx.set_target("ab\x0012", oid, None, "valid message");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_target_message() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let oid = Oid::from_bytes(&[1u8; 20]).unwrap();
+        let result = tx.set_target("refs/heads/main", oid, None, "ab\x0012");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_symbolic_target_refname() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.set_symbolic_target("ab\x0012", "refs/heads/main", None, "valid message");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_symbolic_target_target() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.set_symbolic_target("refs/heads/next", "ab\x0012", None, "valid message");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_symbolic_target_message() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.set_symbolic_target("refs/heads/next", "refs/heads/main", None, "ab\x0012");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_set_reflog() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let reflog = repo.reflog("dummy").expect("Valid name");
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.set_reflog("ab\x0012", reflog);
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
+
+    #[test]
+    fn invalid_remove() {
+        let (_td, repo) = crate::test::repo_init();
+
+        let mut tx = t!(repo.transaction());
+        let result = tx.remove("ab\x0012");
+        assert_eq!(
+            Err(crate::Error::from_str(
+                "data contained a nul byte that could not be represented as a string"
+            )),
+            result,
+        );
+    }
 }

src/tree.rs

@@ -164,7 +164,7 @@ impl<'repo> Tree<'repo> {
     ///
     /// This allows for non-UTF-8 filenames.
     pub fn get_name_bytes(&self, filename: &[u8]) -> Option<TreeEntry<'_>> {
-        let filename = CString::new(filename).unwrap();
+        let filename = CString::new(filename).ok()?;
         let ptr = unsafe { call!(raw::git_tree_entry_byname(&*self.raw(), filename)) };
         if ptr.is_null() {
             None
@@ -597,4 +597,22 @@ mod tests {
         let e = tree.walk(TreeWalkMode::PreOrder, |_, _| -1).unwrap_err();
         assert_eq!(e.class(), crate::ErrorClass::Callback);
     }
+
+    #[test]
+    fn invalid_name_bytes() {
+        let (td, repo) = crate::test::repo_init();
+
+        setup_repo(&td, &repo);
+
+        let head = repo.head().unwrap();
+        let target = head.target().unwrap();
+        let commit = repo.find_commit(target).unwrap();
+
+        let tree = repo.find_tree(commit.tree_id()).unwrap();
+        assert_eq!(tree.id(), commit.tree_id());
+        assert_eq!(tree.len(), 8);
+
+        let result = tree.get_name_bytes(b"ab\x0012");
+        assert!(result.is_none());
+    }
 }