Jeff King [off-list ref] writes:
So the ideal logic is:
1. look in netrc
2. If we have a username and no password, ask for password
3. Otherwise, try it and see if we get a 401.
But we can't do that, because (1) and (3) happen atomically inside of
curl.
The simplest thing is to just drop the behavior in (2), and let it drop
to a 401. The extra round trip probably isn't that big a deal.
That is essentially what Stefan's fix is about.
The cases we have "extra" roundtrip are:
- when you have username@ in URL but no password is stored in .netrc;
- when you have username@ in URL and no $HOME/.netrc file.
and in such a case using URL without username@ in it as a workaround would
save the roundtrip but forces you to type your username@ over and over
again, which is _not_ a real workaround.
A workaround for people who want ultimate convenience is to use .netrc to
have both username:password, but that is at the cost of potentially
reduced security. Having username@ in URL and typing password
interactively, if it worked properly, would have been the best of both
worlds.
The other option is to start parsing netrc ourselves, or do the extra
round trip if we detect ~/.netrc or something. But that last one is
getting pretty hackish.
I tend to agree that we wouldn't want to parse netrc ourselves (that is
what library support e.g. CURLOPT_NETRC is for). The latter is hackish but
on the other hand it is a cheap, simple and useful hack.
How would the upcoming keystore support fit in this picture, by the way?
On Wed, Nov 02, 2011 at 12:13:48PM -0700, Junio C Hamano wrote:
quoted
The simplest thing is to just drop the behavior in (2), and let it drop
to a 401. The extra round trip probably isn't that big a deal.
That is essentially what Stefan's fix is about.
Right. I think it may be the sanest thing to do.
The cases we have "extra" roundtrip are:
- when you have username@ in URL but no password is stored in .netrc;
- when you have username@ in URL and no $HOME/.netrc file.
and in such a case using URL without username@ in it as a workaround would
save the roundtrip but forces you to type your username@ over and over
again, which is _not_ a real workaround.
Yeah. There's no way for us to know before we hand off to curl what you
have in netrc. So these netrc cases will always be at odds with the
no-netrc case.
Normally I would say to implement in favor of the no-netrc case, as it
is probably more common (and will hopefully be more so after the auth
helpers are finished). But the problem is that the penalties are
different. On the one hand, we have the extra http round-trip. Which is
annoying, but mostly invisible to the user. But on the other, we have
git prompting the user unnecessarily, which is just awful.
quoted
The other option is to start parsing netrc ourselves, or do the extra
round trip if we detect ~/.netrc or something. But that last one is
getting pretty hackish.
I tend to agree that we wouldn't want to parse netrc ourselves (that is
what library support e.g. CURLOPT_NETRC is for). The latter is hackish but
on the other hand it is a cheap, simple and useful hack.
Note that it's not always right, of course. You might have a .netrc but
no entry for that host. But at least it lets the common case people
(i.e., people who never heard of or touched netrc) to avoid the round
trip.
How would the upcoming keystore support fit in this picture, by the way?
Any time we would call getpass(), we ask the helper for the credential.
So for user@host, we would call out to the helper for the password
proactively, and otherwise wait for a 401.
We _could_ be proactive and actually ask the helpers for a username and
password even for "https://host/repo", which would save a round-trip to
get the 401 in some cases. But that assumes that asking the helper is
cheap. It might actually require the user inputting a password to unlock
the keystore, which would be annoying if the remote doesn't require
auth at all.
We could try to be clever and use a heuristic that fetch probably
doesn't need auth, but push does. Then fetch gets the extra round-trip
but push doesn't. But that just seems needlessly complex to save one
http round-trip on push.
-Peff