Re: [PATCH v2] http: honnor empty http.proxy option to bypass proxy
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 11, 2017, 10:09 UTC
- Message-ID
- <CACBZZX4QfeiL7kB2+3pPbKjPGnQRXrGooQ-QrpgUtPnZRF=o8A@mail.gmail.com>
- In-Reply-To
- <20170411092050.15867-1-ryazanov.s.a@gmail.com>
Since nobody pointed this out already, some grammar/spelling fixes. Also CC-ing Knut who wrote the commit you're referencing.
- http: honnor empty http.proxy option to bypass proxy + http: honor empty http.proxy option to bypass proxy
On Tue, Apr 11, 2017 at 11:20 AM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> Curl distinguish between empty proxy address and NULL proxy address. In
...distinguishes between an empty proxy address and a NULL proxy address...
> the first case it completly disable proxy usage, but if proxy address
it completely disables ... but if the proxy ....
> option is NULL then curl attempt to determine proxy address from
...attempts to determine the proxy ...
> http_proxy environment variable.
... the http_....
> According to documentation, if http.proxy configured to empty string > then git should bypass proxy and connects to the server directly:
...to the documentation, if http_proxy is set to an empty string, git should bypass the proxy and connect to the server directly.
Show 13 quoted lines
> > 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, which makes previous call a noop:
[Using the format we usually cite commits with, see SubmittingPatches]
Commit 372370f167 ("http: use credential API to handle proxy authentication", 2016-01-26) parses the proxy option, then extracts the proxy host address and updates the curl configuration, making the previous call a noop.
Show 7 quoted lines
> > 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.
But if the proxy option is empty then the proxy host field becomes NULL. This forces curl to fall back to detecting the proxy configuration from the environment, causing the http.rpoxy option to not work anymore.
> > Fix this issue by explicitly handling empty http.proxy option. This also > makes code a bit more clear and should help us avoid such regressions in > the future.
Fix this issue by explicitly handling http.proxy being set to the empty string. This also makes the code a ....
Show 34 quoted lines
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> ---
>
> Changes since v1:
> - explicitly handle this case instead of mangling the common code
>
> http.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/http.c b/http.c
> index 96d84bb..8be75b2 100644
> --- a/http.c
> +++ b/http.c
> @@ -836,8 +836,14 @@ static CURL *get_curl_handle(void)
> }
> }
>
> - if (curl_http_proxy) {
> - curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> + if (curl_http_proxy && curl_http_proxy[0] == '\0') {
> + /*
> + * Handle case with the empty http.proxy value here to keep
> + * common code clean.
> + * NB: empty option disables proxying at all.
> + */
> + curl_easy_setopt(result, CURLOPT_PROXY, "");
> + } else if (curl_http_proxy) {
> #if LIBCURL_VERSION_NUM >= 0x071800
> if (starts_with(curl_http_proxy, "socks5h"))
> curl_easy_setopt(result,
> --
> 2.10.2
>