Re: [PATCH v5 2/5] http: handle proxy proactive authentication

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2012, 21:25 UTC
Message-ID: <7vpqbc4p8n.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120412205836.GB21018@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> My first instinct was "that is not a URL, and should be handled outside
> this function". In particular, it has no protocol field, and that is an
> important part of the credential-matching process. It would be up to the
> caller to supply something sane in the protocol portion. In this case,
> it would probably be "http"...

Outside git, these actually come from things like these:

	http_proxy=127.0.0.1:1080
        HTTPS_PROXY=127.0.0.1:1080

And http.proxy configuration variable we have is a substitute for http_proxy. So if we want to keep the credentials for destination servers and the credentials for proxies, "http.proxy" codepath should be asking you with "http". If we are looking at HTTPS_PROXY, you should get "https". The patch that broke the unauthenticated proxy access does neither.

> ... (unless we want to distinguish http proxies
> from http end-points in the credential store, but I doubt that is
> useful).

That is something we may want to think carefully about.

If it is better to separate them, then we can easily invent "http-proxy", "https-proxy" etc. for them, e.g.

	HTTPS_PROXY=http://127.0.0.1:1080
	git fetch https://over.there.xz/repo/sito/ry.git

would ask you for a credential to access 127.0.0.1:1080 in "https-proxy" domain, and another to access over.there.xz in "https" domain.

In either case, the last example will not use "http" anywhere, even though the value of the proxy has noiseword "http://" in front of it, which is ignored. So in that sense, even if we ignored the breakage for the proxy specification without noiseword which Shawn noticed, the patch is broken, as it asks credential for http://127.0.0.1:1080 and you parse it for "http" protocol.