threads / patch / 29772

patch, 3 partshttp: when proxy url has username but no password, ask for password

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

## tl;dr

3 messages between Feb 28, 2012 and Feb 29, 2012. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Nelson Benitez Leon· Feb 28, 2012, 12:56 UTC · lore
Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
---
 http.c |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)
Show changes to http.c +35 −1
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
Jeff King· Feb 28, 2012, 19:31 UTC · re: Nelson Benitez Leon · lore

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

On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:
Show 21 quoted lines
> 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
Nelson Benitez Leon· Feb 29, 2012, 10:46 UTC · re: Jeff King · lore

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

On 02/28/2012 08:31 PM, Jeff King wrote:
Show 27 quoted lines
> 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
Show 5 quoted lines
>> +			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..
Show 10 quoted lines
>> +			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).

← back to recent threads