Skip to content

Commit 030e040

Browse files
CopilotByron
Copilot
and
Byron
committed
fix: credential fill to allow protocol+host without URL
Co-authored-by: Byron <[email protected]>
1 parent a80a150 commit 030e040

File tree

7 files changed

+173
-8
lines changed

7 files changed

+173
-8
lines changed

‎gitoxide-core/src/repository/credential.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@ pub fn function(repo: gix::Repository, action: gix::credentials::program::main::
1515
std::io::stdin(),
1616
std::io::stdout(),
1717
|action, context| -> Result<_, Error> {
18+
let url = context.url.clone().or_else(|| context.to_url()).ok_or_else(|| {
19+
Error::Protocol(gix::credentials::protocol::Error::UrlMissing)
20+
})?;
1821
let (mut cascade, _action, prompt_options) = repo.config_snapshot().credential_helpers(gix::url::parse(
19-
context.url.as_ref().expect("framework assures URL is present").as_ref(),
22+
url.as_ref(),
2023
)?)?;
2124
cascade
2225
.invoke(

‎gix-credentials/src/program/main.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub enum Error {
5555
Context(#[from] crate::protocol::context::decode::Error),
5656
#[error("Credentials for {url:?} could not be obtained")]
5757
CredentialsMissing { url: BString },
58-
#[error("The url wasn't provided in input - the git credentials protocol mandates this")]
58+
#[error("Either 'url' field or both 'protocol' and 'host' fields must be provided")]
5959
UrlMissing,
6060
}
6161

@@ -89,15 +89,17 @@ pub(crate) mod function {
8989
let mut buf = Vec::<u8>::with_capacity(512);
9090
stdin.read_to_end(&mut buf)?;
9191
let ctx = Context::from_bytes(&buf)?;
92-
if ctx.url.is_none() {
92+
if ctx.url.is_none() && (ctx.protocol.is_none() || ctx.host.is_none()) {
9393
return Err(Error::UrlMissing);
9494
}
9595
let res = credentials(action, ctx).map_err(|err| Error::Helper { source: Box::new(err) })?;
9696
match (action, res) {
9797
(Action::Get, None) => {
98-
return Err(Error::CredentialsMissing {
99-
url: Context::from_bytes(&buf)?.url.expect("present and checked above"),
100-
})
98+
let ctx_for_error = Context::from_bytes(&buf)?;
99+
let url = ctx_for_error.url.clone()
100+
.or_else(|| ctx_for_error.to_url())
101+
.expect("URL is available either directly or via protocol+host");
102+
return Err(Error::CredentialsMissing { url })
101103
}
102104
(Action::Get, Some(ctx)) => ctx.write_to(stdout)?,
103105
(Action::Erase | Action::Store, None) => {}

‎gix-credentials/src/protocol/context/mod.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,16 @@ mod mutate {
9292
/// normally this isn't the case.
9393
#[allow(clippy::result_large_err)]
9494
pub fn destructure_url_in_place(&mut self, use_http_path: bool) -> Result<&mut Self, protocol::Error> {
95-
let url = gix_url::parse(self.url.as_ref().ok_or(protocol::Error::UrlMissing)?.as_ref())?;
95+
if self.url.is_none() {
96+
// If URL is not present but protocol and host are, construct the URL
97+
if let Some(constructed_url) = self.to_url() {
98+
self.url = Some(constructed_url);
99+
} else {
100+
return Err(protocol::Error::UrlMissing);
101+
}
102+
}
103+
104+
let url = gix_url::parse(self.url.as_ref().expect("URL is present after check above").as_ref())?;
96105
self.protocol = Some(url.scheme.as_str().into());
97106
self.username = url.user().map(ToOwned::to_owned);
98107
self.password = url.password().map(ToOwned::to_owned);

‎gix-credentials/src/protocol/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub type Result = std::result::Result<Option<Outcome>, Error>;
2020
pub enum Error {
2121
#[error(transparent)]
2222
UrlParse(#[from] gix_url::parse::Error),
23-
#[error("The 'url' field must be set when performing a 'get/fill' action")]
23+
#[error("Either 'url' field or both 'protocol' and 'host' fields must be provided")]
2424
UrlMissing,
2525
#[error(transparent)]
2626
ContextDecode(#[from] context::decode::Error),

‎gix-credentials/tests/program/main_validation.rs‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
use gix_credentials::program::main;
2+
use std::io::Cursor;
3+
4+
#[derive(Debug, thiserror::Error)]
5+
#[error("Test error")]
6+
struct TestError;
7+
8+
#[test]
9+
fn protocol_and_host_without_url_is_valid() {
10+
let input = b"protocol=https\nhost=github.com\n";
11+
let mut output = Vec::new();
12+
13+
let result = main(
14+
["get".into()],
15+
Cursor::new(input),
16+
&mut output,
17+
|_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
18+
// Verify the context has the expected fields
19+
assert_eq!(context.protocol.as_deref(), Some("https"));
20+
assert_eq!(context.host.as_deref(), Some("github.com"));
21+
// Should return None to simulate no credentials found (which is expected in test)
22+
Ok(None)
23+
},
24+
);
25+
26+
// This should fail because our mock helper returned None (no credentials found)
27+
// but it should NOT fail because of missing URL
28+
match result {
29+
Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
30+
// This is the expected error - credentials missing, not URL missing
31+
}
32+
other => panic!("Expected CredentialsMissing error, got: {:?}", other),
33+
}
34+
}
35+
36+
#[test]
37+
fn missing_protocol_with_host_fails() {
38+
let input = b"host=github.com\n";
39+
let mut output = Vec::new();
40+
41+
let result = main(
42+
["get".into()],
43+
Cursor::new(input),
44+
&mut output,
45+
|_action, _context| -> Result<Option<gix_credentials::protocol::Context>, TestError> { Ok(None) },
46+
);
47+
48+
match result {
49+
Err(gix_credentials::program::main::Error::UrlMissing) => {
50+
// This is expected
51+
}
52+
other => panic!("Expected UrlMissing error, got: {:?}", other),
53+
}
54+
}
55+
56+
#[test]
57+
fn missing_host_with_protocol_fails() {
58+
let input = b"protocol=https\n";
59+
let mut output = Vec::new();
60+
61+
let result = main(
62+
["get".into()],
63+
Cursor::new(input),
64+
&mut output,
65+
|_action, _context| -> Result<Option<gix_credentials::protocol::Context>, TestError> { Ok(None) },
66+
);
67+
68+
match result {
69+
Err(gix_credentials::program::main::Error::UrlMissing) => {
70+
// This is expected
71+
}
72+
other => panic!("Expected UrlMissing error, got: {:?}", other),
73+
}
74+
}
75+
76+
#[test]
77+
fn url_alone_is_still_valid() {
78+
let input = b"url=https://github.com\n";
79+
let mut output = Vec::new();
80+
81+
let result = main(
82+
["get".into()],
83+
Cursor::new(input),
84+
&mut output,
85+
|_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
86+
// Verify the context has the expected fields
87+
assert_eq!(context.url.as_deref().map(|b| &**b), Some("https://github.com".as_bytes()));
88+
// Should return None to simulate no credentials found (which is expected in test)
89+
Ok(None)
90+
},
91+
);
92+
93+
// This should fail because our mock helper returned None (no credentials found)
94+
// but it should NOT fail because of missing URL
95+
match result {
96+
Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
97+
// This is the expected error - credentials missing, not URL missing
98+
}
99+
other => panic!("Expected CredentialsMissing error, got: {:?}", other),
100+
}
101+
}

‎gix-credentials/tests/program/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
mod from_custom_definition;
2+
mod main_validation;

‎gix-credentials/tests/protocol/context.rs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,55 @@ mod destructure_url_in_place {
7272
true,
7373
);
7474
}
75+
76+
#[test]
77+
fn protocol_and_host_without_url_constructs_url_first() {
78+
let mut ctx = Context {
79+
protocol: Some("https".into()),
80+
host: Some("github.com".into()),
81+
..Default::default()
82+
};
83+
ctx.destructure_url_in_place(false).expect("should work with protocol and host");
84+
85+
// URL should be constructed from protocol and host
86+
assert_eq!(ctx.url.as_deref().map(|b| &**b), Some("https://github.com".as_bytes()));
87+
// Original fields should be preserved
88+
assert_eq!(ctx.protocol.as_deref(), Some("https"));
89+
assert_eq!(ctx.host.as_deref(), Some("github.com"));
90+
}
91+
92+
#[test]
93+
fn protocol_and_host_with_path_without_url_constructs_full_url() {
94+
let mut ctx = Context {
95+
protocol: Some("https".into()),
96+
host: Some("github.com".into()),
97+
path: Some("org/repo".into()),
98+
..Default::default()
99+
};
100+
ctx.destructure_url_in_place(false).expect("should work with protocol, host and path");
101+
102+
// URL should be constructed from all provided fields
103+
assert_eq!(ctx.url.as_deref().map(|b| &**b), Some("https://github.com/org/repo".as_bytes()));
104+
// Original fields should be preserved
105+
assert_eq!(ctx.protocol.as_deref(), Some("https"));
106+
assert_eq!(ctx.host.as_deref(), Some("github.com"));
107+
assert_eq!(ctx.path.as_deref().map(|b| &**b), Some("org/repo".as_bytes()));
108+
}
109+
110+
#[test]
111+
fn missing_protocol_or_host_without_url_fails() {
112+
let mut ctx_no_protocol = Context {
113+
host: Some("github.com".into()),
114+
..Default::default()
115+
};
116+
assert!(ctx_no_protocol.destructure_url_in_place(false).is_err());
117+
118+
let mut ctx_no_host = Context {
119+
protocol: Some("https".into()),
120+
..Default::default()
121+
};
122+
assert!(ctx_no_host.destructure_url_in_place(false).is_err());
123+
}
75124
}
76125

77126
mod to_prompt {

0 commit comments

Comments
 (0)