diff --git a/gitoxide-core/src/repository/credential.rs b/gitoxide-core/src/repository/credential.rs index c901f06f2ba..9684250bd4d 100644 --- a/gitoxide-core/src/repository/credential.rs +++ b/gitoxide-core/src/repository/credential.rs @@ -15,9 +15,14 @@ pub fn function(repo: gix::Repository, action: gix::credentials::program::main:: std::io::stdin(), std::io::stdout(), |action, context| -> Result<_, Error> { - let (mut cascade, _action, prompt_options) = repo.config_snapshot().credential_helpers(gix::url::parse( - context.url.as_ref().expect("framework assures URL is present").as_ref(), - )?)?; + let url = context + .url + .clone() + .or_else(|| context.to_url()) + .ok_or(Error::Protocol(gix::credentials::protocol::Error::UrlMissing))?; + let (mut cascade, _action, prompt_options) = repo + .config_snapshot() + .credential_helpers(gix::url::parse(url.as_ref())?)?; cascade .invoke( match action { diff --git a/gix-credentials/src/program/main.rs b/gix-credentials/src/program/main.rs index 34f4163a01e..ef7556c5977 100644 --- a/gix-credentials/src/program/main.rs +++ b/gix-credentials/src/program/main.rs @@ -55,7 +55,7 @@ pub enum Error { Context(#[from] crate::protocol::context::decode::Error), #[error("Credentials for {url:?} could not be obtained")] CredentialsMissing { url: BString }, - #[error("The url wasn't provided in input - the git credentials protocol mandates this")] + #[error("Either 'url' field or both 'protocol' and 'host' fields must be provided")] UrlMissing, } @@ -89,15 +89,19 @@ pub(crate) mod function { let mut buf = Vec::::with_capacity(512); stdin.read_to_end(&mut buf)?; let ctx = Context::from_bytes(&buf)?; - if ctx.url.is_none() { + if ctx.url.is_none() && (ctx.protocol.is_none() || ctx.host.is_none()) { return Err(Error::UrlMissing); } - let res = credentials(action, ctx).map_err(|err| Error::Helper { source: Box::new(err) })?; + let res = credentials(action, ctx.clone()).map_err(|err| Error::Helper { source: Box::new(err) })?; match (action, res) { (Action::Get, None) => { - return Err(Error::CredentialsMissing { - url: Context::from_bytes(&buf)?.url.expect("present and checked above"), - }) + let ctx_for_error = ctx; + let url = ctx_for_error + .url + .clone() + .or_else(|| ctx_for_error.to_url()) + .expect("URL is available either directly or via protocol+host which we checked for"); + return Err(Error::CredentialsMissing { url }); } (Action::Get, Some(ctx)) => ctx.write_to(stdout)?, (Action::Erase | Action::Store, None) => {} diff --git a/gix-credentials/src/protocol/context/mod.rs b/gix-credentials/src/protocol/context/mod.rs index 01ee151f306..ccbe9e0954a 100644 --- a/gix-credentials/src/protocol/context/mod.rs +++ b/gix-credentials/src/protocol/context/mod.rs @@ -92,7 +92,11 @@ mod mutate { /// normally this isn't the case. #[allow(clippy::result_large_err)] pub fn destructure_url_in_place(&mut self, use_http_path: bool) -> Result<&mut Self, protocol::Error> { - let url = gix_url::parse(self.url.as_ref().ok_or(protocol::Error::UrlMissing)?.as_ref())?; + if self.url.is_none() { + self.url = Some(self.to_url().ok_or(protocol::Error::UrlMissing)?); + } + + let url = gix_url::parse(self.url.as_ref().expect("URL is present after check above").as_ref())?; self.protocol = Some(url.scheme.as_str().into()); self.username = url.user().map(ToOwned::to_owned); self.password = url.password().map(ToOwned::to_owned); diff --git a/gix-credentials/src/protocol/mod.rs b/gix-credentials/src/protocol/mod.rs index 2920780bf95..7a71c9c4b3d 100644 --- a/gix-credentials/src/protocol/mod.rs +++ b/gix-credentials/src/protocol/mod.rs @@ -20,7 +20,7 @@ pub type Result = std::result::Result, Error>; pub enum Error { #[error(transparent)] UrlParse(#[from] gix_url::parse::Error), - #[error("The 'url' field must be set when performing a 'get/fill' action")] + #[error("Either 'url' field or both 'protocol' and 'host' fields must be provided")] UrlMissing, #[error(transparent)] ContextDecode(#[from] context::decode::Error), diff --git a/gix-credentials/tests/program/main.rs b/gix-credentials/tests/program/main.rs new file mode 100644 index 00000000000..0a6c985e267 --- /dev/null +++ b/gix-credentials/tests/program/main.rs @@ -0,0 +1,94 @@ +use gix_credentials::program::main; +use std::io::Cursor; + +#[derive(Debug, thiserror::Error)] +#[error("Test error")] +struct TestError; + +#[test] +fn protocol_and_host_without_url_is_valid() { + let input = b"protocol=https\nhost=github.com\n"; + let mut output = Vec::new(); + + let mut called = false; + let result = main( + ["get".into()], + Cursor::new(input), + &mut output, + |_action, context| -> Result, TestError> { + assert_eq!(context.protocol.as_deref(), Some("https")); + assert_eq!(context.host.as_deref(), Some("github.com")); + assert_eq!(context.url, None, "the URL isn't automatically populated"); + called = true; + + Ok(None) + }, + ); + + // This should fail because our mock helper returned None (no credentials found) + // but it should NOT fail because of missing URL + match result { + Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => { + assert!( + called, + "The helper gets called, but as nothing is provided in the function it ulimately fails" + ); + } + other => panic!("Expected CredentialsMissing error, got: {other:?}"), + } +} + +#[test] +fn missing_protocol_with_only_host_or_protocol_fails() { + for input in ["host=github.com\n", "protocol=https\n"] { + let mut output = Vec::new(); + + let mut called = false; + let result = main( + ["get".into()], + Cursor::new(input), + &mut output, + |_action, _context| -> Result, TestError> { + called = true; + Ok(None) + }, + ); + + match result { + Err(gix_credentials::program::main::Error::UrlMissing) => { + assert!(!called, "the context is lacking, hence nothing gets called"); + } + other => panic!("Expected UrlMissing error, got: {other:?}"), + } + } +} + +#[test] +fn url_alone_is_valid() { + let input = b"url=https://github.com\n"; + let mut output = Vec::new(); + + let mut called = false; + let result = main( + ["get".into()], + Cursor::new(input), + &mut output, + |_action, context| -> Result, TestError> { + called = true; + assert_eq!(context.url.unwrap(), "https://github.com"); + assert_eq!(context.host, None, "not auto-populated"); + assert_eq!(context.protocol, None, "not auto-populated"); + + Ok(None) + }, + ); + + // This should fail because our mock helper returned None (no credentials found) + // but it should NOT fail because of missing URL + match result { + Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => { + assert!(called); + } + other => panic!("Expected CredentialsMissing error, got: {other:?}"), + } +} diff --git a/gix-credentials/tests/program/mod.rs b/gix-credentials/tests/program/mod.rs index 3672dd18e38..12a307d0979 100644 --- a/gix-credentials/tests/program/mod.rs +++ b/gix-credentials/tests/program/mod.rs @@ -1 +1,2 @@ mod from_custom_definition; +mod main; diff --git a/gix-credentials/tests/protocol/context.rs b/gix-credentials/tests/protocol/context.rs index e60f3ebf850..e12c4549523 100644 --- a/gix-credentials/tests/protocol/context.rs +++ b/gix-credentials/tests/protocol/context.rs @@ -72,6 +72,48 @@ mod destructure_url_in_place { true, ); } + + #[test] + fn protocol_and_host_with_path_without_url_constructs_full_url() { + let mut ctx = Context { + protocol: Some("https".into()), + host: Some("github.com".into()), + path: Some("org/repo".into()), + username: Some("user".into()), + password: Some("pass-to-be-ignored".into()), + ..Default::default() + }; + ctx.destructure_url_in_place(false) + .expect("should work with protocol, host and path"); + + assert_eq!( + ctx.url.unwrap(), + "https://user@github.com/org/repo", + "URL should be constructed from all provided fields, except password" + ); + // Original fields should be preserved + assert_eq!(ctx.protocol.as_deref(), Some("https")); + assert_eq!(ctx.host.as_deref(), Some("github.com")); + assert_eq!(ctx.path.unwrap(), "org/repo"); + } + + #[test] + fn missing_protocol_or_host_without_url_fails() { + let mut ctx_no_protocol = Context { + host: Some("github.com".into()), + ..Default::default() + }; + assert_eq!( + ctx_no_protocol.destructure_url_in_place(false).unwrap_err().to_string(), + "Either 'url' field or both 'protocol' and 'host' fields must be provided" + ); + + let mut ctx_no_host = Context { + protocol: Some("https".into()), + ..Default::default() + }; + assert!(ctx_no_host.destructure_url_in_place(false).is_err()); + } } mod to_prompt {