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

From: Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es>
Date: Mar 2, 2012, 14:05 UTC
Message-ID: <4F50D39D.5040806@seap.minhap.es>
In-Reply-To: <20120302124538.GA10637@sigill.intra.peff.net>

On 03/02/2012 01:45 PM, Jeff King wrote:

Show 34 quoted lines

> On Fri, Mar 02, 2012 at 02:33:53PM +0100, Nelson Benitez Leon wrote:
> 
>>> So there's the history lesson. What should proxy auth do?
>>>
>>>   1. Definitely respond to HTTP 407 by prompting on the fly; this code
>>>      should go along-side the HTTP 401 code in http.c.
>>>
>>>   2. Definitely do the pre-prompt thing when http_proactive_auth is set
>>>      (which is used only by http-push). Unless somebody really feels
>>>      like re-writing http-push to handle retries for authentication.
>>>
>>>   3. Consider doing the pre-prompt thing when http_proactive_auth is not
>>>      set. This can save a round-trip, but we should not do it if there
>>>      is a good reason not to. The two possible reasons I can think of
>>>      are:
>>>
>>>        a. Like http auth, if curl will read the proxy credentials from
>>>           .netrc, then we should not do it for the same reasons
>>>           mentioned in 986bbc0.
>>>
>>>        b. If people realistically have proxy URLs with usernames but do
>>>           _not_ want to ask for a password, then the prompt will be
>>>           annoying. I'm not sure that anybody expects that.
>>
>> So, trying to sum up, I will try to redo patch-set as follows:
>> - Ignore PATCH 2/3 , that is, we won't read any env var.
>> - Let cURL try to connect and if that fails with 407 , then do a credential_fill
>> and try to reconnect.
>>
>> Is that ok? or do I need to do something more?
> 
> I think you'll still need to read the env var, because you'll need to
> know the proxy URL when getting the password (to ask credential helpers
> properly, and to prompt the user).

Ok, but I can read it after receiving the 407 (and in case we were not using http.proxy) so discarding PATCH 2/3 still applies, ok? or we need to read it first-hand for the http_proactive_auth you mention below?

> 
> Also, I think you'll need to call credential_fill() when
> http_proactive_auth is set. Otherwise http-push will not be able to do
> proxy auth.

I still don't get what proactive_auth is about, will ask you when I get to that part of the patch.

Thank you,

Show 5 quoted lines

> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html