more faithfully parse http.followRedirect

Author: Byron

git-repository/src/repository/config/transport.rs

@@ -48,10 +48,7 @@ impl crate::Repository {
                     use git_transport::client::http;
                     use git_transport::client::http::options::ProxyAuthMethod;
 
-                    use crate::{
-                        bstr::ByteVec,
-                        config::cache::util::{ApplyLeniency, ApplyLeniencyDefault},
-                    };
+                    use crate::{bstr::ByteVec, config::cache::util::ApplyLeniency};
                     fn try_cow_to_string(
                         v: Cow<'_, BStr>,
                         lenient: bool,
@@ -185,24 +182,28 @@ impl crate::Repository {
                         headers
                     };
 
-                    if let Some(follow_redirects) =
-                        config.string_filter("http", None, "followRedirects", &mut trusted_only)
+                    let redirects_key = "http.followRedirects";
+                    opts.follow_redirects = if config
+                        .string_filter_by_key(redirects_key, &mut trusted_only)
+                        .map_or(false, |v| v.as_ref() == "initial")
                     {
-                        opts.follow_redirects = if follow_redirects.as_ref() == "initial" {
-                            http::options::FollowRedirects::Initial
-                        } else if git_config::Boolean::try_from(follow_redirects)
-                            .map_err(|err| crate::config::transport::Error::ConfigValue {
+                        http::options::FollowRedirects::Initial
+                    } else if let Some(val) = config
+                        .boolean_filter_by_key(redirects_key, &mut trusted_only)
+                        .map(|res| {
+                            res.map_err(|err| crate::config::transport::Error::ConfigValue {
                                 source: err,
-                                key: "http.followRedirects",
+                                key: redirects_key,
                             })
-                            .with_lenient_default(lenient)?
-                            .0
-                        {
-                            http::options::FollowRedirects::All
-                        } else {
-                            http::options::FollowRedirects::None
-                        };
-                    }
+                        })
+                        .transpose()
+                        .with_leniency(lenient)?
+                    {
+                        val.then(|| http::options::FollowRedirects::All)
+                            .unwrap_or(http::options::FollowRedirects::None)
+                    } else {
+                        http::options::FollowRedirects::Initial
+                    };
 
                     opts.low_speed_time_seconds =
                         integer(config, lenient, "http.lowSpeedTime", "u64", trusted_only, 0)?;

git-repository/tests/fixtures/generated-archives/make_config_repos.tar.xz

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:83f5270e487f0364e4a63f82cf4bfb5cb302bee744e9d3df521b7b3f850f3046
-size 10772
+oid sha256:3687c843a39942197ab8a31896b3a6c453295de7bb8d031642eb34b292a272da
+size 10816

git-repository/tests/fixtures/make_config_repos.sh

@@ -6,7 +6,7 @@ git init http-config
   git config --add http.extraHeader ""
   git config --add http.extraHeader "ExtraHeader: value2"
   git config --add http.extraHeader "ExtraHeader: value3"
-  git config http.followRedirects initial
+  git config http.followRedirects true
   git config http.lowSpeedLimit 5k
   git config http.lowSpeedTime 10
   git config http.postBuffer 8k
@@ -19,6 +19,8 @@ git init http-config
 git clone --shared http-config http-remote-override
 (cd http-remote-override
 
+  git config http.followRedirects initial
+
   git config http.proxy http://localhost:9090
   git config http.proxyAuthMethod basic
 
@@ -28,6 +30,8 @@ git clone --shared http-config http-remote-override
 
 git init http-proxy-empty
 (cd http-proxy-empty
+  git config http.followRedirects false
+
   git config http.proxy localhost:9090
   git config --add http.proxy "" # a value override disabling it later
 )
@@ -40,4 +44,8 @@ git init http-proxy-auto-prefix
 git init http-proxy-authenticated
 (cd http-proxy-authenticated
   git config http.proxy user@localhost:9090
+  cat <<EOF >> .git/config
+[http]
+  followRedirects
+EOF
 )

git-repository/tests/repository/config/transport_options.rs

@@ -27,11 +27,13 @@ mod http {
         let git_transport::client::http::Options {
             proxy,
             proxy_auth_method,
+            follow_redirects,
             ..
         } = http_options(&repo, Some("origin"));
 
         assert_eq!(proxy_auth_method, ProxyAuthMethod::Negotiate);
         assert_eq!(proxy.as_deref(), Some("http://overridden"));
+        assert_eq!(follow_redirects, FollowRedirects::Initial);
     }
 
     #[test]
@@ -54,7 +56,7 @@ mod http {
             &["ExtraHeader: value2", "ExtraHeader: value3"],
             "it respects empty values to clear prior values"
         );
-        assert_eq!(follow_redirects, FollowRedirects::Initial);
+        assert_eq!(follow_redirects, FollowRedirects::All);
         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"),);
@@ -84,7 +86,12 @@ mod http {
         assert!(
             opts.proxy_authenticate.is_some(),
             "…and credential-helpers are used to do that. This could be overridden in remotes one day"
-        )
+        );
+        assert_eq!(
+            opts.follow_redirects,
+            FollowRedirects::All,
+            "an empty value is true, so we can't take shortcuts for these"
+        );
     }
 
     #[test]
@@ -97,6 +104,7 @@ mod http {
             Some(""),
             "empty strings indicate that the proxy is to be unset by the transport"
         );
+        assert_eq!(opts.follow_redirects, FollowRedirects::None);
     }
 
     #[test]
@@ -105,5 +113,6 @@ mod http {
 
         let opts = http_options(&repo, None);
         assert_eq!(opts.proxy.as_deref(), Some("http://localhost:9090"));
+        assert_eq!(opts.follow_redirects, FollowRedirects::Initial);
     }
 }