Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password

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

On 02/28/2012 08:31 PM, Jeff King wrote:
> On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:
> 
>> diff --git a/http.c b/http.c
>> index 79cbe50..68e3f7d 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)
>>  		}
>>  	}
>>  	if (curl_http_proxy) {
>> -		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>> +		char *at, *colon, *proxyuser;
>> +		const char *cp;
>> +		cp = strstr(curl_http_proxy, "://");
>> +		if (cp == NULL) {
>> +			cp = curl_http_proxy;
>> +		} else {
>> +			cp += 3;
>> +		}
>> +		at = strchr(cp, '@');
>> +		colon = strchr(cp, ':');
>> +		if (at && (!colon || at < colon)) {
>> +			/* proxy string has username but no password, ask for password */
> 
> Don't parse the URL by hand. Use credential_from_url, which will do it
> for you (and will properly handle things like unquoting the various
> components).

Will do that

>> +			char *ask_str, *proxyuser, *proxypass;
> 
> Shouldn't these be static globals? If we have multiple curl handles, you
> would want them to share the authentication information we collect here,
> and not have to ask the user again, no?

I didn't think about multiple curl handles, will look at make those static..

>> +			strbuf_addf(&pbuf, "Enter password for proxy %s...", at+1);
>> +			ask_str = strbuf_detach(&pbuf, NULL);
>> +			proxypass = xstrdup(git_getpass(ask_str));
> 
> And this should be using credential_fill(), which will let it use
> credential helpers to save passwords, give it the same type of prompt as
> elsewhere, etc.
> 
> See Documentation/technical/api-credential.txt, and see how regular http
> auth is handled for an example.

I will try the credential api, I did my patch based on the fedora 16 git
version, which didn't have the credential api (I couldn't git clone at
that moment for the proxy problem so I had to use the source rpm from
fedora).