Re: [PATCH 2/6] http: handle proxy proactive authentication

[Jeff King <peff@peff.net>]

On Thu, May 03, 2012 at 06:39:54PM +0200, Nelson Benitez Leon wrote:

> If http_proactive_auth flag is set and there is a username
> but no password in the proxy url, then interactively ask for
> the password.
> 
> This makes possible to not have the password written down in
> http_proxy env var or in http.proxy config option.
> 
> Also take care that CURLOPT_PROXY don't include username or
> password, as we now set them in the new set_proxy_auth() function
> where we use their specific cURL options.

Do we actually need to do that? If we set CURLOPT_PROXYUSERNAME, will
curl ignore it in favor of what's in the URL? I ask, because there is a
bug here:

> @@ -351,8 +366,19 @@ static CURL *get_curl_handle(const char *url)
>  	}
>  	
>  	if (curl_http_proxy) {
> -		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		struct strbuf proxyhost = STRBUF_INIT;
> +
> +		if (!proxy_auth.host) /* check to parse only once */
> +			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 */
> +			credential_fill(&proxy_auth);
> +
> +		strbuf_addf(&proxyhost, "%s://%s", proxy_auth.protocol, proxy_auth.host);
> +		curl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&proxyhost, NULL));

When you parse the URL via credential_from_url, the components you get
will have any URL-encoding removed. So when you regenerate the URL in
the proxyhost variable, you would need to re-encode.

But if we can stop doing this regeneration at all, then the problem goes
away.

Also, newer versions of curl will copy the string instead of taking
ownership of the pointer. Unfortunately we have to deal with both old
and new, but you can get around it by using a static strbuf (so we leak,
but we only leak once per program, not once per get_curl_handle call).
This issue would also go away if we stop regenerating the URL.

-Peff