On Mon, Apr 10, 2017 at 06:15:56PM +0300, Sergey Ryazanov wrote: > Curl distinguish between empty proxy address and NULL proxy address. In > the first case it completly disable proxy usage, but if proxy address > option is NULL then curl attempt to determine proxy address from > http_proxy environment variable. > > According to documentation, if http.proxy configured to empty string > then git should bypass proxy and connects to the server directly: > > export http_proxy=http://network-proxy/ > cd ~/foobar-project > git config remote.origin.proxy "" > git fetch > > Previously, proxy host was configured by one line: > > curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy); > > Commit 372370f (http: use credential API to handle proxy auth...) parses > proxy option, extracts proxy host address and additionaly updates curl > configuration: > > credential_from_url(&proxy_auth, curl_http_proxy); > curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host); > > But if proxy option is empty then proxy host field become NULL this > force curl to fallback to proxy configuration detection from > environment. This caused empty http.proxy option not working any more. That makes sense. And if I understand correctly, this was a regression in 372370f; before that we fed curl_http_proxy directly, and it was either NULL or not, depending on whether we had seen the config option. It looks like we _still_ set CURLOPT_PROXY to curl_http_proxy, and then immediately afterward set it to proxy_auth.host. That should make the first one always a noop, I would think, and it should be removed. But... > diff --git a/http.c b/http.c > index 96d84bb..bf0e709 100644 > --- a/http.c > +++ b/http.c > @@ -861,7 +861,12 @@ static CURL *get_curl_handle(void) > strbuf_release(&url); > } > > - curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host); > + /* > + * Avoid setting CURLOPT_PROXY to NULL if empty http.proxy > + * option configured. > + */ > + if (proxy_auth.host) > + curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host); Here that second one becomes conditional, and we rely on the earlier setting (but only sometimes). I would think this whole thing would be more clear if we dropped the first CURLOPT_PROXY call entirely, and just did: /* * If we parsed a null host from the URL, we must convert that * back into an empty string so that curl knows we want no proxy at * all (not to find the default). */ curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host ? proxy_auth.host : ""); -Peff