# [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set 6 messages from 2012-03-01 to 2012-03-01. Participants: Nelson Benitez Leon, Sam Vilain, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/29799 ## Sam Vilain, 2012-03-01 17:45 Subject: Re: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <4F4FB5BF.8000904@vilain.net> URL: https://gitlist.dev/e/4F4FB5BF.8000904%40vilain.net In-Reply-To: <4F4FBE0F.6020004@seap.minhap.es> ``` On 3/1/12 10:21 AM, Nelson Benitez Leon wrote: > CuRL already reads it, but if $http_proxy has username but no password > curl will not ask you for the password.. so we read it ourselves to > detect that and ask for the password. That's not what this change does. This change explicitly loads from the environment the 'http_proxy' variable and sets up curl to use it. As Junio said, this is (on its own) a regression. Sam > Signed-off-by: Nelson Benitez Leon > --- > http.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/http.c b/http.c > index 8ac8eb6..8932da5 100644 > --- a/http.c > +++ b/http.c > @@ -295,6 +295,13 @@ static CURL *get_curl_handle(void) > if (curl_ftp_no_epsv) > curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0); > > + if (!curl_http_proxy) { > + const char *env_proxy; > + env_proxy = getenv("http_proxy"); > + if (env_proxy) { > + curl_http_proxy = xstrdup(env_proxy); > + } > + } > if (curl_http_proxy) { > curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy); > curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY); ``` ## Nelson Benitez Leon, 2012-03-01 18:21 Subject: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <4F4FBE0F.6020004@seap.minhap.es> URL: https://gitlist.dev/e/4F4FBE0F.6020004%40seap.minhap.es ``` CuRL already reads it, but if $http_proxy has username but no password curl will not ask you for the password.. so we read it ourselves to detect that and ask for the password. Signed-off-by: Nelson Benitez Leon --- http.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/http.c b/http.c index 8ac8eb6..8932da5 100644 --- a/http.c +++ b/http.c @@ -295,6 +295,13 @@ static CURL *get_curl_handle(void) if (curl_ftp_no_epsv) curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0); + if (!curl_http_proxy) { + const char *env_proxy; + env_proxy = getenv("http_proxy"); + if (env_proxy) { + curl_http_proxy = xstrdup(env_proxy); + } + } if (curl_http_proxy) { curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy); curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY); -- 1.7.7.6 ``` ## Junio C Hamano, 2012-03-01 18:33 Subject: Re: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <7v7gz4npby.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7gz4npby.fsf%40alter.siamese.dyndns.org In-Reply-To: <4F4FB5BF.8000904@vilain.net> ``` Sam Vilain writes: > On 3/1/12 10:21 AM, Nelson Benitez Leon wrote: >> CuRL already reads it, but if $http_proxy has username but no password >> curl will not ask you for the password.. so we read it ourselves to >> detect that and ask for the password. > > That's not what this change does. This change explicitly loads from > the environment the 'http_proxy' variable and sets up curl to use it. > As Junio said, this is (on its own) a regression. Just to make sure there is no understanding down the road, I only expressed a concern that this _might_ be a regression. That Mac OS X behaviour is not something I observed first-hand. ``` ## Junio C Hamano, 2012-03-01 19:10 Subject: Re: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <7vy5rkm91t.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy5rkm91t.fsf%40alter.siamese.dyndns.org In-Reply-To: <4F4FBE0F.6020004@seap.minhap.es> ``` Nelson Benitez Leon writes: > CuRL already reads it, but if $http_proxy has username but no password > curl will not ask you for the password.. so we read it ourselves to > detect that and ask for the password. Please stop the double-dot. Also your capitalization for cURL is screwed up. More importantly, please describe what happens after "will not ask". "will not ask you for the password and the connection fails"? "will not ask you for the password and the gives an error message saying 'authentication failure'"? The logic in the patch, needless to say, seems OK, though. Thanks. > > Signed-off-by: Nelson Benitez Leon > --- > http.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/http.c b/http.c > index 8ac8eb6..8932da5 100644 > --- a/http.c > +++ b/http.c > @@ -295,6 +295,13 @@ static CURL *get_curl_handle(void) > if (curl_ftp_no_epsv) > curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0); > > + if (!curl_http_proxy) { > + const char *env_proxy; > + env_proxy = getenv("http_proxy"); > + if (env_proxy) { > + curl_http_proxy = xstrdup(env_proxy); > + } > + } > if (curl_http_proxy) { > curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy); > curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY); ``` ## Jeff King, 2012-03-01 21:01 Subject: Re: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <20120301210129.GD17631@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120301210129.GD17631%40sigill.intra.peff.net In-Reply-To: <7vy5rkm91t.fsf@alter.siamese.dyndns.org> ``` On Thu, Mar 01, 2012 at 11:10:38AM -0800, Junio C Hamano wrote: > Nelson Benitez Leon writes: > > > CuRL already reads it, but if $http_proxy has username but no password > > curl will not ask you for the password.. so we read it ourselves to > > detect that and ask for the password. > > Please stop the double-dot. Also your capitalization for cURL is screwed > up. > > More importantly, please describe what happens after "will not ask". > "will not ask you for the password and the connection fails"? > "will not ask you for the password and the gives an error message saying > 'authentication failure'"? When we need to authenticate for the destination webserver, we detect an HTTP 401, _then_ ask for the credentials, and retry the request. I'm curious what the error condition is for the authentication failure, and if we can do the same here (from a brief skim of rfc2616, it looks like it should be a 407, but I do not even have a proxy set up to try). -Peff ``` ## Junio C Hamano, 2012-03-01 21:38 Subject: Re: [PATCH v2 2/3] http: try http_proxy env var when http.proxy config option is not set Message-ID: <7vty28knmz.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vty28knmz.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120301210129.GD17631@sigill.intra.peff.net> ``` Jeff King writes: > When we need to authenticate for the destination webserver, we detect an > HTTP 401, _then_ ask for the credentials, and retry the request. I'm > curious what the error condition is for the authentication failure, and > if we can do the same here. Yeah, that would be ideal. ```