threads / patch / 29774

patch, 3 partshttp: authenticate on NTLM proxies and others suppported, by CuRL

Subject: [PATCH 1/3] http: authenticate on NTLM proxies and others suppported, by CuRL

## tl;dr

4 messages between Feb 28, 2012 and Feb 28, 2012. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Nelson Benitez Leon· Feb 28, 2012, 12:52 UTC · lore
Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
---
 http.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to http.c +3 −1
diff --git a/http.c b/http.c
index 0ffd79c..8ac8eb6 100644
--- a/http.c
+++ b/http.c
@@ -295,8 +295,10 @@ static CURL *get_curl_handle(void)
 	if (curl_ftp_no_epsv)
 		curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
 
-	if (curl_http_proxy)
+	if (curl_http_proxy) {
 		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
+	}
 
 	return result;
 }

-- 
1.7.7.6
Thomas Rast· Feb 28, 2012, 14:32 UTC · re: Nelson Benitez Leon · lore

Re: [PATCH 1/3] http: authenticate on NTLM proxies and others suppported, by CuRL

Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
Show 5 quoted lines
> -	if (curl_http_proxy)
> +	if (curl_http_proxy) {
>  		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> +	}
There was another attempt at doing the same very recently:
  http://thread.gmane.org/gmane.comp.version-control.git/191140

I could swear there was a second one, but apparently that was you. Neither you nor Marco submitter have so far answered the question I raised in

  http://thread.gmane.org/gmane.comp.version-control.git/191155

which can be summarized as: please make a case -- and put it in the message! -- for or against making this configurable. Is there a speed tradeoff? (However, you could steal some of Daniel Stenberg's reasoning!)

Note that Marco (the submitter in the first one) has told me privately that he could not pursue this further right now, so I suppose he'd be happy if you could finish it.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Nelson Benitez Leon· Feb 28, 2012, 18:15 UTC · re: Thomas Rast · lore

Re: [PATCH 1/3] http: authenticate on NTLM proxies and others suppported, by CuRL

On 02/28/2012 03:32 PM, Thomas Rast wrote:
Show 22 quoted lines
> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
> 
>> -	if (curl_http_proxy)
>> +	if (curl_http_proxy) {
>>  		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>> +		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>> +	}
> 
> There was another attempt at doing the same very recently:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/191140
> 
> I could swear there was a second one, but apparently that was you.
> Neither you nor Marco submitter have so far answered the question I
> raised in
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/191155
> 
> which can be summarized as: please make a case -- and put it in the
> message! -- for or against making this configurable.  Is there a speed
> tradeoff?  (However, you could steal some of Daniel Stenberg's
> reasoning!)

I don't see any reason to make this configurable, CuRL people made this cool CURLAUTH_ANY option that automatically chooses the best auth method from among those the server supports, that means you don't have to investigate if your proxy is using Basic, Digest or NTLM methods, and use a specific curl option for each of them, instead curl will ask the proxy and use the appropiate, and it will only do that if you are using a proxy (i.e. you've set CURLOPT_PROXY or you have http_proxy env var), also curl will not try to authenticate if you've not provided username or password in the proxy string, as I've been told here[1]..

so, setting CURLOPT_PROXYAUTH = CURLAUTH_ANY will not affect the speed of normal curl use, only if 1) you are using a proxy and 2) your proxy requires authentication, only then curl will just make two or three roundtrips to find out the auth method the proxy is using, that is a tiny cost compared to having the user find out the proxy auth type and set an specifically config option to enable that type.

So I would call CURLAUTH_ANY as out-of-the-box proxy support, and I don't want it activated from a config option, I want it to still be out-of-the-box in git also..

[1] https://bugzilla.redhat.com/show_bug.cgi?id=769254#c6
Thomas Rast· Feb 28, 2012, 17:30 UTC · re: Nelson Benitez Leon · lore

Re: [PATCH 1/3] http: authenticate on NTLM proxies and others suppported, by CuRL

Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
Show 25 quoted lines
> On 02/28/2012 03:32 PM, Thomas Rast wrote:
>> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
>> 
>>> -	if (curl_http_proxy)
>>> +	if (curl_http_proxy) {
>>>  		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>>> +		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>>> +	}
>> 
>> There was another attempt at doing the same very recently:
>> 
>>   http://thread.gmane.org/gmane.comp.version-control.git/191140
>> 
>> I could swear there was a second one, but apparently that was you.
>> Neither you nor Marco submitter have so far answered the question I
>> raised in
>> 
>>   http://thread.gmane.org/gmane.comp.version-control.git/191155
>> 
>> which can be summarized as: please make a case -- and put it in the
>> message! -- for or against making this configurable.  Is there a speed
>> tradeoff?  (However, you could steal some of Daniel Stenberg's
>> reasoning!)
>
> I don't see any reason to make this configurable, [snip]

Sorry for not making this clear, but you don't have to explain it to me in the list archives; rather, the commit message should sufficiently take care of it. That is, I was objecting to your (lack of a) commit message.

The argument you need to counter here is (Daniel explains this) curl does an extra roundtrip to the proxy to determine which protocols work, which is a little extra cost.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

← back to recent threads