Re: [PATCH 8/8] http: prompt for credentials on failed POST

On Mon, Aug 27, 2012 at 10:48:28AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

Most of the time, this is not a big deal; for both fetching
and pushing, we make a GET request before doing any POSTs,
so typically we figure out the credentials during the first
request, then reuse them during the POST. However, some
servers may allow a client to get the list of refs from
receive-pack without authentication, and then require
authentication when the client actually tries to POST the
pack.

A silly question.  Does the initial GET request when we push look
any different from the initial GET request when we fetch?  Can we
make them look different in an updated client, so that the server
side can say "this GET is about pushing into us, and we require
authentication"?

Yes, they are already different. A fetch asks for

  info/refs?service=git-upload-pack

and a push asks for

  info/refs?service-git-receive-pack

And I definitely think the optimal server config will authenticate the
client at that first GET step, because the client may do a significant
amount of work for the POST (due to the probe_rpc, it won't actually
_send_ a large pack, but it will do the complete delta-compression phase
before generating any output, which can be slow).

But doing it this way has been advertised in our manpage for so long, I
assume some people are using it. And given that it used to work for
older clients (prior to v1.7.8), and that the person who upgraded their
client is not always in charge of telling the person running the server
to fix their server, I think it's worth un-breaking it.

And we should definitely tweak what git-http-backend advertises on top
so that eventually this sub-optimal config dies out.

quoted

  1. If we are using gzip. However, we only do so when
     calling git-upload-pack, so it does not apply to
     pushes.

  2. If we have a large request, the probe succeeds, but
     then the real POST wants authentication. This is an
     extremely unlikely configuration and not worth worrying
     about.

While it might be nice to cover those instances, doing so
would be significantly more complex for very little
real-world gain. In the long run, we will be much better off
when curl learns to internally handle authentication as a
callback, and we can cleanly handle all cases that way.

I suspect that in real life, almost nobody runs smart HTTP server
that allows anonymous push.

How much usability penalty would it be if we always fill credential
before pushing?

It would reintroduce the problem that 986bbc0 was fixing: we would
prompt even when curl would end up pulling the credential from .netrc.
I find that somewhat less compelling a problem now that we have
credential helpers, though. And of course it does not fix (1) or (2)
above, either.

Alternatively, how much latency penalty would it incur if we always
send a probe request regardless of the request size when we try to
push without having an authentication material?

It would be one http round-trip and no-op invocation of request-pack on
the server. If we did it only on push, that would probably not be too
bad, as we would hit it only when we were actually pushing something.

But that would still suffer from (1) and (2) above, so I don't see it as
a real advantage. You _could_ fix both cases by buffering the input data
and restarting the request. I just didn't think it was worth doing,
since they are unlikely configurations and the code complexity is much
higher.

-Peff