Subject: Re: [PATCH v3 2/4] http: try http_proxy env var when http.proxy config option is not set

From: Junio C Hamano <[email-masked]>
Date: Mon, 05 Mar 2012 09:30:30 -0800

Nelson Benitez Leon <[email-masked]> writes:

> cURL already reads it, but if $http_proxy has username but no password
> cURL will not ask you for the password and so failed to authenticate
> returning a 407 error code. So we read it ourselves to detect that and
> ask for the password. Also we read it prior to connection to be able to
> make a proactive authentication in case the flag http_proactive_auth is
> set.
>
> Signed-off-by: Nelson Benitez Leon <[email-masked]>
> ---

The above does not address my earlier worries about compatibility
across curl applications (it does not even say "it does not matter;
we do not care about other's application"), so let's spell it out
again.  When the user has

	http_proxy=http://[email-masked]
	export http_proxy

with your patch, git might do whatever we desire to do, and the end
result may be better, but how would the user experience be for the
user when using other curl based programs on the same system?

Also I thought the conclusion from the other thread was that even if
we were to do this, we should apply the http_proxy environment only
when we are talking to http:// and for https:// we would instead
read HTTPS_PROXY or something?

>  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);