Jeff King [off-list ref] writes:
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.
On Thu, Apr 12, 2012 at 02:25:12PM -0700, Junio C Hamano wrote:
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.
Hmm. Does the distinction between http and https actually matter to
curl? My reading of the code and documentation is that only "http" is
meaningful (actually, anything besides socks*:// gets converted to
http).
So as far as I can tell, these are equivalent:
http_proxy=http://127.0.0.1:1080
http_proxy=https://127.0.0.1:1080
http_proxy=foobar://127.0.0.1:1080
And furthermore, the decision to use http_proxy versus https_proxy is
about what the _target_ connection wants to do. So if you see this:
HTTPS_PROXY=127.0.0.1:1080
it is still an http proxy; it is just that it is used for requests going
to https:// servers, and it will ask to tunnel via CONNECT instead of
GET. But in either case, the conversation with the proxy is over
straight http.
So the value should always be "http" in that case. This is a credential
we are handing to the proxy, not to the target server, and it is done
over http, not https.
I can't see that there is a way to tell curl to speak SSL to the proxy
itself. Maybe I am missing it, but I couldn't find anything in the
code, nor make it work with "curl -x" to an "openssl s_server" instance.
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.
No, it should ask for the credential for 127.0.0.1:1080 in the "http"
domain, per the above discussion.
Not splitting "http" and "http-proxy" does have a slight confusion, as
the default proxy port is "1080". So a proxy of "http://127.0.0.1" would
mean "http://127.0.0.1:1080", whereas a regular request would mean
"http://127.0.0.1:80". The credential code includes the port as part of
the unique hostname, but since the default-port magic happens inside
curl, we have no access to it (short of re-implementing it ourselves).
In practice, I doubt it matters much; do people really have different
credentials for proxies and regular servers on the same host? And if so,
there is already a workaround by using the port number in the proxy
specification.
I really wish curl's credential-handling was implemented as a callback;
this would be much simpler if could let curl decipher the request and
come to us with the complete request (protocol, host, port, path, etc).
But even if we got such a feature in curl, we are stuck supporting the
old way for a while anyway.
-Peff