Re: [PATCH v3 3/4] http: handle proxy proactive authentication

From: Jeff King <peff@peff.net>
Date: Mar 6, 2012, 08:30 UTC
Message-ID: <20120306083052.GD21199@sigill.intra.peff.net>
In-Reply-To: <4F54D98C.2070909@seap.minhap.es>

On Mon, Mar 05, 2012 at 04:19:40PM +0100, Nelson Benitez Leon wrote:

Show 24 quoted lines

> diff --git a/http.c b/http.c
> index 8932da5..b0b4362 100644
> --- a/http.c
> +++ b/http.c
> @@ -43,6 +43,7 @@ static int curl_ftp_no_epsv;
>  static const char *curl_http_proxy;
>  static const char *curl_cookie_file;
>  static struct credential http_auth = CREDENTIAL_INIT;
> +static struct credential proxy_auth = CREDENTIAL_INIT;
>  static int http_proactive_auth;
>  static const char *user_agent;
>  
> @@ -303,6 +304,17 @@ static CURL *get_curl_handle(void)
>  		}
>  	}
>  	if (curl_http_proxy) {
> +		credential_from_url(&proxy_auth, curl_http_proxy);
> +		if (http_proactive_auth && proxy_auth.username && !proxy_auth.password) {
> +			/* proxy string has username but no password, ask for password */
> +			struct strbuf pbuf = STRBUF_INIT;
> +			credential_fill(&proxy_auth);
> +			strbuf_addf(&pbuf, "%s://%s:%s@%s",proxy_auth.protocol,
> +			    	proxy_auth.username, proxy_auth.password,
> +			    	proxy_auth.host);

Can we pull this out into a helper function, since the next patch will need to do the exact same thing in the 407 case?

Also, when turning it back into a URL to hand to curl, should we be percent-encoding the items we put in? If my password has an "@" in it, wouldn't we generate a bogus URL? Although looking at how the http auth code handles this, we set CURLOPT_USERPWD directly. Should you be setting CURLOPT_PROXYUSERPWD instead of munging the proxy URL?

> +			free ((void *)curl_http_proxy);

Please don't cast to void. This is C, not C++, and casts to void pointers are implicit. They can never help, and might cover up an actual type error (e.g., casting a non-pointer type).

-Peff