From e611682893856cec41d14036dd3bd975b3745e70 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 15 Jul 2026 18:06:56 -0700 Subject: [PATCH 1/2] `RepoBuilder::branch()`: add regression tests for invalid branch names The test is currently marked as `#[should_panic]` because the implementation panics when the branch given is invalid, a follow-up commit will switch to returning a `Result`. --- src/build.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/build.rs b/src/build.rs index c1ef2567a4..ae1221dbb3 100644 --- a/src/build.rs +++ b/src/build.rs @@ -869,4 +869,11 @@ mod tests { repo.checkout_index(Some(&mut index), Some(&mut checkout_opts)) .unwrap(); } + + #[test] + #[should_panic] + fn invalid_repo_builder_branch() { + let mut builder = RepoBuilder::new(); + builder.branch("abc\x00xyz"); + } } From c31fcafb732d5b1dcd8efdbbec1261d09d0b2cd4 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 15 Jul 2026 18:11:44 -0700 Subject: [PATCH 2/2] `RepoBuilder::branch()`: return a `Result`, handle invalid values When the provided branch name cannot be converted to a `CString`, return an `Err` variant rather than panicking. --- src/build.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/build.rs b/src/build.rs index ae1221dbb3..726ed7c585 100644 --- a/src/build.rs +++ b/src/build.rs @@ -172,9 +172,9 @@ impl<'cb> RepoBuilder<'cb> { /// Specify the name of the branch to check out after the clone. /// /// If not specified, the remote's default branch will be used. - pub fn branch(&mut self, branch: &str) -> &mut RepoBuilder<'cb> { - self.branch = Some(CString::new(branch).unwrap()); - self + pub fn branch(&mut self, branch: &str) -> Result<&mut RepoBuilder<'cb>, Error> { + self.branch = Some(CString::new(branch)?); + Ok(self) } /// Configures options for bypassing the git-aware transport on clone. @@ -800,7 +800,9 @@ mod tests { let dst = td.path().join("foo"); RepoBuilder::new().clone(&url, &dst).unwrap(); fs::remove_dir_all(&dst).unwrap(); - assert!(RepoBuilder::new().branch("foo").clone(&url, &dst).is_err()); + let mut builder = RepoBuilder::new(); + builder.branch("foo").expect("Foo is a valid branch name"); + assert!(builder.clone(&url, &dst).is_err()); } #[test] @@ -871,9 +873,20 @@ mod tests { } #[test] - #[should_panic] fn invalid_repo_builder_branch() { let mut builder = RepoBuilder::new(); - builder.branch("abc\x00xyz"); + // Cannot use unwrap_err() because StashSaveOptions does not implement Debug + let result = match builder.branch("abc\x00xyz") { + Ok(_) => panic!("Expected an error"), + Err(e) => e, + }; + assert_eq!( + crate::Error::new( + crate::ErrorCode::GenericError, + crate::ErrorClass::None, + "data contained a nul byte that could not be represented as a string" + ), + result + ); } }