diff --git a/src/lib.rs b/src/lib.rs index 628fc94daa..17212133cf 100644 --- a/src/lib.rs +++ b/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 { - 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); diff --git a/src/refspec.rs b/src/refspec.rs index 7717ab6787..afbbdfc2a9 100644 --- a/src/refspec.rs +++ b/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 { - 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 { - 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, + ); + } +} diff --git a/src/transaction.rs b/src/transaction.rs index d4116b050f..ffdb9e9ed2 100644 --- a/src/transaction.rs +++ b/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, + ); + } } diff --git a/src/tree.rs b/src/tree.rs index 2fde618c83..4f50bf5358 100644 --- a/src/tree.rs +++ b/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> { - 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()); + } }