Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set

From: Junio C Hamano <gitster@pobox.com>
Date: May 4, 2012, 15:36 UTC
Message-ID: <7v4nrwc61e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120504073913.GA22388@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 35 quoted lines

> On Fri, May 04, 2012 at 09:27:16AM +0200, Daniel Stenberg wrote:
>
>> On Fri, 4 May 2012, Jeff King wrote:
>> 
>> >I don't see any way around it besides implementing curl's full
>> >tokenizing and matching algorithm, which is about a page of code.
>> >I'd really prefer not to re-implement bits of curl (especially
>> >because they may change later), but AFAIK there is no way to ask
>> >curl "is there a proxy configured, and if so, what is it?".
>> 
>> Sorry for being thick, but I lost track on this thread. Why does it
>> need this info again?
>
> We need to know information about the proxy in order to look up the
> username and password in our credential database. Before the request is
> made in some cases, and in others, after we see a 407. If we fed the
> proxy to curl via CURLOPT_PROXY, it's easy. But if the proxy came from
> the environment, we have to replicate curl's lookup rules.
>
>> Or perhaps put another way: if there was an ideal way to get this
>> done or provide this to libcurl other than the current way, how would
>> you suggest it would be done from a git internal point of view?
>
> The absolute simplest way for us would be to stop using
> CURLOPT_PROXYUSERNAME/PASSWORD to set it ahead of time, and instead
> provide a callback that curl would call on a 407. That callback would
> just need the URL of the proxy, and would return the username/password
> (or even just set them on the curl object via
> CURLOPT_PROXYUSERNAME/PASSWORD).
>
> For that matter, it would simplify our code to do the same for regular
> http auth, too. And though we usually know our URL in that case, we
> might not if we got a 302 with FOLLOWLOCATION set.
>
> -Peff

Thanks for a nice summary, and I agree with your list of what we wish we had from the cURL library. With such a change, it becomes irrelevant how the user fed cURL provisional (partial) authentication information, either in http.proxy (which we turn into CURLOPT_PROXY), or from the environment (without Git having to know anything about it), and a lot of complexity that led to bugs in this series will become unnecessary.

I am tempted to suggest that the current series should be rerolled without all the guessing and preauth tricks until such an update to the cURL library materializes.