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

Signed-off-by: Nelson Benitez Leon <redacted>
---
 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

On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:

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

On 02/28/2012 08:31 PM, Jeff King wrote:

On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:

quoted

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

quoted

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

quoted

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