git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergey Ryazanov <ryazanov.s.a@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2] http: honnor empty http.proxy option to bypass proxy
Date: Tue, 11 Apr 2017 12:20:50 +0300	[thread overview]
Message-ID: <20170411092050.15867-1-ryazanov.s.a@gmail.com> (raw)

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, which makes 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.

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.

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


             reply	other threads:[~2017-04-11  9:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-11  9:20 Sergey Ryazanov [this message]
2017-04-11 10:09 ` [PATCH v2] http: honnor empty http.proxy option to bypass proxy Ævar Arnfjörð Bjarmason
2017-04-11 10:21   ` Sergey Ryazanov
2017-04-11 13:06 ` Jeff King
2017-04-11 14:56   ` Sergey Ryazanov
2017-04-11 14:59     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170411092050.15867-1-ryazanov.s.a@gmail.com \
    --to=ryazanov.s.a@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).