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

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

[Nelson Benitez Leon]

Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
---
 http.c |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

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 */
+			char *ask_str, *proxyuser, *proxypass;
+			int len;
+			struct strbuf pbuf = STRBUF_INIT;
+			len = at - cp;
+			proxyuser = xmalloc(len + 1);
+			memcpy(proxyuser, cp, len);
+			proxyuser[len] = '\0';
+			
+			strbuf_addf(&pbuf, "Enter password for proxy %s...", at+1);
+			ask_str = strbuf_detach(&pbuf, NULL);
+			proxypass = xstrdup(git_getpass(ask_str));
+			
+			strbuf_insert(&pbuf, 0, curl_http_proxy, cp - curl_http_proxy);
+			strbuf_addf(&pbuf, "%s:%s", proxyuser, proxypass);
+			strbuf_add(&pbuf, at, strlen(at));
+			curl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&pbuf, NULL));
+			
+			free(ask_str);
+			free(proxyuser);
+			free(proxypass);
+		} else {
+			curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+		}
 		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
 	}
 
-- 
1.7.7.6

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

[Jeff King]

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).

> +			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?

> +			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.

-Peff

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

[Nelson Benitez Leon]

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).