Web lists-archives.com

Re: [PATCH v2] http: honnor empty http.proxy option to bypass proxy




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@xxxxxxxxx> 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.


>
>     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.

>
>     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 ....

>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>
> ---
>
> 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
>