From: Nelson Benitez Leon <nelsonjesus.benitez@seap•minhap.es>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, peff@peff•net, sam@vilain•net
Subject: Re: [PATCH v3 2/4] http: try http_proxy env var when http.proxy config option is not set
Date: Tue, 06 Mar 2012 13:22:46 +0100 [thread overview]
Message-ID: <4F560196.8070500@seap.minhap.es> (raw)
In-Reply-To: <7v4nu32bwp.fsf@alter.siamese.dyndns.org>
On 03/05/2012 06:30 PM, Junio C Hamano wrote:
> Nelson Benitez Leon <nelsonjesus.benitez@seap•minhap.es> 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 <nbenitezl@gmail•com>
>> ---
>
> 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://me@proxy.example.com
> 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?
As I said in the commit message, in that cases a 407 error will be
returned from those applications, because curl will not ask you for
the password, I tested that myself, maybe my message is not clear
enough, you can suggest me a better wording or feel free to amend it
yourself :-).
An $http_proxy without password means it needs support from the application,
I expect users who put a proxy url without password to be confident that
the application they're using will ask them for the password (git in this
case), if they're not sure they have to go with the standard approach of
setting both username and password in $http_proxy to obtain full
compatibility amongst any programs (at the cost of having the password
written down somewhere).
>
> 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?
Ok I completely miss this, can this be added in a later patch?
>
>> 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);
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger•kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-03-06 11:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-05 15:17 [PATCH v3 2/4] http: try http_proxy env var when http.proxy config option is not set Nelson Benitez Leon
2012-03-05 17:30 ` Junio C Hamano
2012-03-06 12:22 ` Nelson Benitez Leon [this message]
2012-03-06 11:27 ` Jeff King
2012-03-06 14:08 ` Nelson Benitez Leon
2012-03-06 15:58 ` Junio C Hamano
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=4F560196.8070500@seap.minhap.es \
--to=nelsonjesus.benitez@seap$(echo .)minhap.es \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=sam@vilain$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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