Skip to content

Commit

Permalink
Add proxy-prefix and explicitly allow empty proxy values
Browse files Browse the repository at this point in the history
It's how git does it, and we don't have tests to validate it all.
  • Loading branch information
Byron committed Nov 15, 2022
1 parent 717b09f commit 70303c1
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 31 deletions.
10 changes: 9 additions & 1 deletion git-repository/src/repository/config/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,15 @@ impl crate::Repository {
opts.proxy = config
.string_filter("http", None, "proxy", &mut trusted_only)
.and_then(|v| try_cow_to_string(v, lenient, "http.proxy").transpose())
.transpose()?;
.transpose()?
.map(|mut proxy| {
if !proxy.trim().is_empty() && !proxy.contains("://") {
proxy.insert_str(0, "http://");
proxy
} else {
proxy
}
});
opts.user_agent = config
.string_filter("http", None, "userAgent", &mut trusted_only)
.and_then(|v| try_cow_to_string(v, lenient, "http.userAgent").transpose())
Expand Down
Git LFS file not shown
11 changes: 8 additions & 3 deletions git-repository/tests/fixtures/make_config_repos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ git init http-config
git config http.lowSpeedLimit 5k
git config http.lowSpeedTime 10
git config http.postBuffer 8k
git config http.proxy localhost:9090
git config http.proxy http://localhost:9090
git config http.proxyAuthMethod anyauth
git config http.userAgent agentJustForHttp
)

git init http-config-empty-proxy
(cd http-config-empty-proxy
git init http-proxy-empty
(cd http-proxy-empty
git config http.proxy localhost:9090
git config --add http.proxy "" # a value override disabling it later
)

git init http-proxy-auto-prefix
(cd http-proxy-auto-prefix
git config http.proxy localhost:9090 # http:// is prefixed automatically
)
52 changes: 27 additions & 25 deletions git-repository/tests/repository/config/transport_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@ mod http {
git::open_opts(dir.join(name), git::open::Options::isolated()).unwrap()
}

#[test]
fn simple_configuration() {
let repo = repo("http-config");
let http_config = repo
fn http_options(repo: &git::Repository) -> git_transport::client::http::Options {
let opts = repo
.transport_options("https://example.com/does/not/matter")
.expect("valid configuration")
.expect("configuration available for http");
opts.downcast_ref::<git_transport::client::http::Options>()
.expect("http options have been created")
.to_owned()
}

#[test]
fn simple_configuration() {
let repo = repo("http-config");
let git_transport::client::http::Options {
extra_headers,
follow_redirects,
Expand All @@ -24,25 +30,19 @@ mod http {
user_agent,
connect_timeout,
backend,
} = http_config
.downcast_ref::<git_transport::client::http::Options>()
.expect("http options have been created");
} = http_options(&repo);
assert_eq!(
extra_headers,
&["ExtraHeader: value2", "ExtraHeader: value3"],
"it respects empty values to clear prior values"
);
assert_eq!(
*follow_redirects,
follow_redirects,
git_transport::client::http::options::FollowRedirects::Initial
);
assert_eq!(*low_speed_limit_bytes_per_second, 5120);
assert_eq!(*low_speed_time_seconds, 10);
assert_eq!(
proxy.as_deref(),
Some("localhost:9090"),
"TODO: turn it into a URL valid for curl"
);
assert_eq!(low_speed_limit_bytes_per_second, 5120);
assert_eq!(low_speed_time_seconds, 10);
assert_eq!(proxy.as_deref(), Some("http://localhost:9090"),);
assert_eq!(
proxy_auth_method.as_ref(),
// Some(&git_transport::client::http::options::ProxyAuthMethod::AnyAuth)
Expand All @@ -51,7 +51,7 @@ mod http {
);
assert_eq!(user_agent.as_deref(), Some("agentJustForHttp"));
assert_eq!(
*connect_timeout,
connect_timeout,
std::time::Duration::from_secs(20),
"this is an arbitrary default, and it's her to allow adjustments of the default"
);
Expand All @@ -63,19 +63,21 @@ mod http {

#[test]
fn empty_proxy_string_turns_it_off() {
let repo = repo("http-config-empty-proxy");
let repo = repo("http-proxy-empty");

let http_config = repo
.transport_options("https://example.com/does/not/matter")
.expect("valid configuration")
.expect("configuration available for http");
let http_config = http_config
.downcast_ref::<git_transport::client::http::Options>()
.expect("http options have been created");
let opts = http_options(&repo);
assert_eq!(
http_config.proxy.as_deref(),
opts.proxy.as_deref(),
Some(""),
"empty strings indicate that the proxy is to be unset by the transport"
);
}

#[test]
fn proxy_without_protocol_is_defaulted_to_http() {
let repo = repo("http-proxy-auto-prefix");

let opts = http_options(&repo);
assert_eq!(opts.proxy.as_deref(), Some("http://localhost:9090"));
}
}

0 comments on commit 70303c1

Please sign in to comment.