Skip to content

Commit 033ce8e

Browse files
authored
Merge pull request #2191 from GitoxideLabs/copilot/fix-b53588ea-1fea-485f-82a8-505a9101514e
Fix `gix credential fill` to accept protocol+host without URL field
2 parents a80a150 + 2cc63ca commit 033ce8e

File tree

7 files changed

+161
-11
lines changed

7 files changed

+161
-11
lines changed

gitoxide-core/src/repository/credential.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,14 @@ 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 (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(),
20-
)?)?;
18+
let url = context
19+
.url
20+
.clone()
21+
.or_else(|| context.to_url())
22+
.ok_or(Error::Protocol(gix::credentials::protocol::Error::UrlMissing))?;
23+
let (mut cascade, _action, prompt_options) = repo
24+
.config_snapshot()
25+
.credential_helpers(gix::url::parse(url.as_ref())?)?;
2126
cascade
2227
.invoke(
2328
match action {

gix-credentials/src/program/main.rs

Lines changed: 10 additions & 6 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,19 @@ 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
}
95-
let res = credentials(action, ctx).map_err(|err| Error::Helper { source: Box::new(err) })?;
95+
let res = credentials(action, ctx.clone()).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 = ctx;
99+
let url = ctx_for_error
100+
.url
101+
.clone()
102+
.or_else(|| ctx_for_error.to_url())
103+
.expect("URL is available either directly or via protocol+host which we checked for");
104+
return Err(Error::CredentialsMissing { url });
101105
}
102106
(Action::Get, Some(ctx)) => ctx.write_to(stdout)?,
103107
(Action::Erase | Action::Store, None) => {}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ 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+
self.url = Some(self.to_url().ok_or(protocol::Error::UrlMissing)?);
97+
}
98+
99+
let url = gix_url::parse(self.url.as_ref().expect("URL is present after check above").as_ref())?;
96100
self.protocol = Some(url.scheme.as_str().into());
97101
self.username = url.user().map(ToOwned::to_owned);
98102
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),
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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 mut called = false;
14+
let result = main(
15+
["get".into()],
16+
Cursor::new(input),
17+
&mut output,
18+
|_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
19+
assert_eq!(context.protocol.as_deref(), Some("https"));
20+
assert_eq!(context.host.as_deref(), Some("github.com"));
21+
assert_eq!(context.url, None, "the URL isn't automatically populated");
22+
called = true;
23+
24+
Ok(None)
25+
},
26+
);
27+
28+
// This should fail because our mock helper returned None (no credentials found)
29+
// but it should NOT fail because of missing URL
30+
match result {
31+
Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
32+
assert!(
33+
called,
34+
"The helper gets called, but as nothing is provided in the function it ulimately fails"
35+
);
36+
}
37+
other => panic!("Expected CredentialsMissing error, got: {other:?}"),
38+
}
39+
}
40+
41+
#[test]
42+
fn missing_protocol_with_only_host_or_protocol_fails() {
43+
for input in ["host=github.com\n", "protocol=https\n"] {
44+
let mut output = Vec::new();
45+
46+
let mut called = false;
47+
let result = main(
48+
["get".into()],
49+
Cursor::new(input),
50+
&mut output,
51+
|_action, _context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
52+
called = true;
53+
Ok(None)
54+
},
55+
);
56+
57+
match result {
58+
Err(gix_credentials::program::main::Error::UrlMissing) => {
59+
assert!(!called, "the context is lacking, hence nothing gets called");
60+
}
61+
other => panic!("Expected UrlMissing error, got: {other:?}"),
62+
}
63+
}
64+
}
65+
66+
#[test]
67+
fn url_alone_is_valid() {
68+
let input = b"url=https://github.com\n";
69+
let mut output = Vec::new();
70+
71+
let mut called = false;
72+
let result = main(
73+
["get".into()],
74+
Cursor::new(input),
75+
&mut output,
76+
|_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
77+
called = true;
78+
assert_eq!(context.url.unwrap(), "https://github.com");
79+
assert_eq!(context.host, None, "not auto-populated");
80+
assert_eq!(context.protocol, None, "not auto-populated");
81+
82+
Ok(None)
83+
},
84+
);
85+
86+
// This should fail because our mock helper returned None (no credentials found)
87+
// but it should NOT fail because of missing URL
88+
match result {
89+
Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
90+
assert!(called);
91+
}
92+
other => panic!("Expected CredentialsMissing error, got: {other:?}"),
93+
}
94+
}
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;

gix-credentials/tests/protocol/context.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,48 @@ mod destructure_url_in_place {
7272
true,
7373
);
7474
}
75+
76+
#[test]
77+
fn protocol_and_host_with_path_without_url_constructs_full_url() {
78+
let mut ctx = Context {
79+
protocol: Some("https".into()),
80+
host: Some("github.com".into()),
81+
path: Some("org/repo".into()),
82+
username: Some("user".into()),
83+
password: Some("pass-to-be-ignored".into()),
84+
..Default::default()
85+
};
86+
ctx.destructure_url_in_place(false)
87+
.expect("should work with protocol, host and path");
88+
89+
assert_eq!(
90+
ctx.url.unwrap(),
91+
"https://[email protected]/org/repo",
92+
"URL should be constructed from all provided fields, except password"
93+
);
94+
// Original fields should be preserved
95+
assert_eq!(ctx.protocol.as_deref(), Some("https"));
96+
assert_eq!(ctx.host.as_deref(), Some("github.com"));
97+
assert_eq!(ctx.path.unwrap(), "org/repo");
98+
}
99+
100+
#[test]
101+
fn missing_protocol_or_host_without_url_fails() {
102+
let mut ctx_no_protocol = Context {
103+
host: Some("github.com".into()),
104+
..Default::default()
105+
};
106+
assert_eq!(
107+
ctx_no_protocol.destructure_url_in_place(false).unwrap_err().to_string(),
108+
"Either 'url' field or both 'protocol' and 'host' fields must be provided"
109+
);
110+
111+
let mut ctx_no_host = Context {
112+
protocol: Some("https".into()),
113+
..Default::default()
114+
};
115+
assert!(ctx_no_host.destructure_url_in_place(false).is_err());
116+
}
75117
}
76118

77119
mod to_prompt {

0 commit comments

Comments
 (0)