public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Nelson Benitez Leon <nelsonjesus.benitez@seap•minhap.es>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, sam@vilain•net
Subject: Re: [PATCH v3 3/4] http: handle proxy proactive authentication
Date: Tue, 06 Mar 2012 11:58:59 +0100	[thread overview]
Message-ID: <4F55EDF3.2030201@seap.minhap.es> (raw)
In-Reply-To: <20120306083052.GD21199@sigill.intra.peff.net>

On 03/06/2012 09:30 AM, Jeff King wrote:
> On Mon, Mar 05, 2012 at 04:19:40PM +0100, Nelson Benitez Leon wrote:
> 
>> diff --git a/http.c b/http.c
>> index 8932da5..b0b4362 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -43,6 +43,7 @@ static int curl_ftp_no_epsv;
>>  static const char *curl_http_proxy;
>>  static const char *curl_cookie_file;
>>  static struct credential http_auth = CREDENTIAL_INIT;
>> +static struct credential proxy_auth = CREDENTIAL_INIT;
>>  static int http_proactive_auth;
>>  static const char *user_agent;
>>  
>> @@ -303,6 +304,17 @@ static CURL *get_curl_handle(void)
>>  		}
>>  	}
>>  	if (curl_http_proxy) {
>> +		credential_from_url(&proxy_auth, curl_http_proxy);
>> +		if (http_proactive_auth && proxy_auth.username && !proxy_auth.password) {
>> +			/* proxy string has username but no password, ask for password */
>> +			struct strbuf pbuf = STRBUF_INIT;
>> +			credential_fill(&proxy_auth);
>> +			strbuf_addf(&pbuf, "%s://%s:%s@%s",proxy_auth.protocol,
>> +			    	proxy_auth.username, proxy_auth.password,
>> +			    	proxy_auth.host);
> 
> Can we pull this out into a helper function, since the next patch will
> need to do the exact same thing in the 407 case?

Ok.

> Also, when turning it back into a URL to hand to curl, should we be
> percent-encoding the items we put in? If my password has an "@" in it,
> wouldn't we generate a bogus URL? Although looking at how the http auth
> code handles this, we set CURLOPT_USERPWD directly. Should you be
> setting CURLOPT_PROXYUSERPWD instead of munging the proxy URL?

Ok, but it seems is CURLOPT_PROXYUSERNAME and CURLOPT_PROXYPASSWORD what
we need here as per documentation[1]

[1] http://curl.haxx.se/libcurl/c/curl_easy_setopt.html#CURLOPTPROXYUSERNAME

>> +			free ((void *)curl_http_proxy);
> 
> Please don't cast to void. This is C, not C++, and casts to void
> pointers are implicit.  They can never help, and might cover up an
> actual type error (e.g., casting a non-pointer type).

Ok, will remove it, I copy/paste it from the http code and I must admit
I didn't understand why this was needed.

  reply	other threads:[~2012-03-06 10:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-05 15:19 [PATCH v3 3/4] http: handle proxy proactive authentication Nelson Benitez Leon
2012-03-06  8:30 ` Jeff King
2012-03-06 10:58   ` Nelson Benitez Leon [this message]
2012-03-06 10:09     ` Jeff King
2012-03-06 10:53       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F55EDF3.2030201@seap.minhap.es \
    --to=nelsonjesus.benitez@seap$(echo .)minhap.es \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    --cc=sam@vilain$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox