Re: [PATCH v3 4/4] http: handle proxy authentication failure (error 407)

[Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es>]

On 03/06/2012 09:54 AM, Jeff King wrote:
> On Mon, Mar 05, 2012 at 04:21:28PM +0100, Nelson Benitez Leon wrote:
> 
>> Handle http 407 error code by asking for credentials and
>> retrying request in case credentials were not present, or
>> marking credentials as rejected if they were already provided.
>>
>> Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
>> ---
>>  http.c |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> No tests. I wonder how hard it would be to set up an apache proxy for
> automated testing, just as we set one up as a git server in
> t/lib-httpd.sh. It should be basically the same process, but with a
> different config file, I would think.

I cannot help with this, I didn't even know you could setup a proxy
with solely apache, I thought you needed special software like Squid..
anyway I'm testing all these patches with my employers proxy which uses
NTLM autentication and they're performing well..

> [snip]
> 
> We need to actually loop, retrying if we get reauth (and arguably REAUTH
> should simply be called RETRY). Like this:
> 
> diff --git a/http.c b/http.c
> index e4afbe5..c325b00 100644
> --- a/http.c
> +++ b/http.c
> @@ -810,10 +810,13 @@ static int http_request(const char *url, curl_write_callback cb, void *result,
>  static int http_request_reauth(const char *url, curl_write_callback cb,
>  			       void *result, unsigned long offset, int options)
>  {
> -	int ret = http_request(url, cb, result, offset, options);
> -	if (ret != HTTP_REAUTH)
> -		return ret;
> -	return http_request(url, cb, result, offset, options);
> +	int ret;
> +
> +	do {
> +		ret = http_request(url, cb, result, offset, options);
> +	} while (ret == HTTP_REAUTH);
> +
> +	return ret;
>  }
>  
>  int http_get_strbuf(const char *url, struct strbuf *result, int options)

Yes, giving the case you mention (proxy auth + http auth) your patch does
the right thing.