Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password
- From
- Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es>
- Date
- Feb 29, 2012, 10:46 UTC
- Message-ID
- <4F4E01F0.8050503@seap.minhap.es>
- In-Reply-To
- <20120228193125.GA11725@sigill.intra.peff.net>
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).