delta/git.git/http.c, branch cn/thin-push-capability

Merge branch 'jk/http-auth-redirects'

2013-10-30T19:09:53+00:00

Handle the case where http transport gets redirected during the
authorization request better.

* jk/http-auth-redirects:
  http.c: Spell the null pointer as NULL
  remote-curl: rewrite base url from info/refs redirects
  remote-curl: store url as a strbuf
  remote-curl: make refs_url a strbuf
  http: update base URLs when we see redirects
  http: provide effective url to callers
  http: hoist credential request out of handle_curl_result
  http: refactor options to http_get_*
  http_request: factor out curlinfo_strbuf
  http_get_file: style fixes

Merge branch 'ew/keepalive'

2013-10-28T17:43:24+00:00

* ew/keepalive:
  http: use curl's tcp keepalive if available
  http: enable keepalive on TCP sockets

http.c: Spell the null pointer as NULL

2013-10-24T21:42:26+00:00

Commit 1bbcc224 ("http: refactor options to http_get_*", 28-09-2013)
changed the type of final 'options' argument of the http_get_file()
function from an int to an 'struct http_get_options' pointer.
However, it neglected to update the (single) call site. Since this
call was passing '0' to that argument, it was (correctly) being
interpreted as a null pointer. Change to argument to NULL.

Noticed by sparse. ("Using plain integer as NULL pointer")

Signed-off-by: Ramsay Jones 
Acked-by: Jeff King 
Reviewed-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano

http: use curl's tcp keepalive if available

2013-10-16T18:26:09+00:00

Commit a15d069 taught git to use curl's SOCKOPTFUNCTION hook
to turn on TCP keepalives. However, modern versions of curl
have a TCP_KEEPALIVE option, which can do this for us. As an
added bonus, the curl code knows how to turn on keepalive
for a much wider variety of platforms. The only downside to
using this option is that not everybody has a new enough curl.
Let's split our keepalive options into three conditionals:

  1. With curl 7.25.0 and newer, we rely on curl to do it
     right.

  2. With older curl that still knows SOCKOPTFUNCTION, we
     use the code from a15d069.

  3. Otherwise, we are out of luck, and the call is a no-op.

Signed-off-by: Jeff King 
Signed-off-by: Junio C Hamano

http: update base URLs when we see redirects

2013-10-14T23:56:47+00:00

If a caller asks the http_get_* functions to go to a
particular URL and we end up elsewhere due to a redirect,
the effective_url field can tell us where we went.

It would be nice to remember this redirect and short-cut
further requests for two reasons:

  1. It's more efficient. Otherwise we spend an extra http
     round-trip to the server for each subsequent request,
     just to get redirected.

  2. If we end up with an http 401 and are going to ask for
     credentials, it is to feed them to the redirect target.
     If the redirect is an http->https upgrade, this means
     our credentials may be provided on the http leg, just
     to end up redirected to https. And if the redirect
     crosses server boundaries, then curl will drop the
     credentials entirely as it follows the redirect.

However, it, it is not enough to simply record the effective
URL we saw and use that for subsequent requests. We were
originally fed a "base" url like:

   http://example.com/foo.git

and we want to figure out what the new base is, even though
the URLs we see may be:

     original: http://example.com/foo.git/info/refs
    effective: http://example.com/bar.git/info/refs

Subsequent requests will not be for "info/refs", but for
other paths relative to the base. We must ask the caller to
pass in the original base, and we must pass the redirected
base back to the caller (so that it can generate more URLs
from it). Furthermore, we need to feed the new base to the
credential code, so that requests to credential helpers (or
to the user) match the URL we will be requesting.

This patch teaches http_request_reauth to do this munging.
Since it is the caller who cares about making more URLs, it
seems at first glance that callers could simply check
effective_url themselves and handle it. However, since we
need to update the credential struct before the second
re-auth request, we have to do it inside http_request_reauth.

Signed-off-by: Jeff King 
Signed-off-by: Jonathan Nieder

http: provide effective url to callers

2013-10-14T23:55:23+00:00

When we ask curl to access a URL, it may follow one or more
redirects to reach the final location. We have no idea
this has happened, as curl takes care of the details and
simply returns the final content to us.

The final URL that we ended up with can be accessed via
CURLINFO_EFFECTIVE_URL. Let's make that optionally available
to callers of http_get_*, so that they can make further
decisions based on the redirection.

Signed-off-by: Jeff King 
Signed-off-by: Jonathan Nieder

http: hoist credential request out of handle_curl_result

2013-10-14T23:55:13+00:00

When we are handling a curl response code in http_request or
in the remote-curl RPC code, we use the handle_curl_result
helper to translate curl's response into an easy-to-use
code. When we see an HTTP 401, we do one of two things:

  1. If we already had a filled-in credential, we mark it as
     rejected, and then return HTTP_NOAUTH to indicate to
     the caller that we failed.

  2. If we didn't, then we ask for a new credential and tell
     the caller HTTP_REAUTH to indicate that they may want
     to try again.

Rejecting in the first case makes sense; it is the natural
result of the request we just made. However, prompting for
more credentials in the second step does not always make
sense. We do not know for sure that the caller is going to
make a second request, and nor are we sure that it will be
to the same URL. Logically, the prompt belongs not to the
request we just finished, but to the request we are (maybe)
about to make.

In practice, it is very hard to trigger any bad behavior.
Currently, if we make a second request, it will always be to
the same URL (even in the face of redirects, because curl
handles the redirects internally). And we almost always
retry on HTTP_REAUTH these days. The one exception is if we
are streaming a large RPC request to the server (e.g., a
pushed packfile), in which case we cannot restart. It's
extremely unlikely to see a 401 response at this stage,
though, as we would typically have seen it when we sent a
probe request, before streaming the data.

This patch drops the automatic prompt out of case 2, and
instead requires the caller to do it. This is a few extra
lines of code, and the bug it fixes is unlikely to come up
in practice. But it is conceptually cleaner, and paves the
way for better handling of credentials across redirects.

Signed-off-by: Jeff King 
Signed-off-by: Jonathan Nieder

http: enable keepalive on TCP sockets

2013-10-14T14:03:59+00:00

This is a follow up to commit e47a8583 (enable SO_KEEPALIVE for
connected TCP sockets, 2011-12-06).

Sockets may never receive notification of some link errors,
causing "git fetch" or similar processes to hang forever.
Enabling keepalive messages allows hung processes to error out
after a few minutes/hours depending on the keepalive settings of
the system.

I noticed this problem with some non-interactive cronjobs getting
hung when talking to HTTP servers.

Signed-off-by: Eric Wong 
Signed-off-by: Jonathan Nieder

http: refactor options to http_get_*

2013-10-01T00:21:59+00:00

Over time, the http_get_strbuf function has grown several
optional parameters. We now have a bitfield with multiple
boolean options, as well as an optional strbuf for returning
the content-type of the response. And a future patch in this
series is going to add another strbuf option.

Treating these as separate arguments has a few downsides:

  1. Most call sites need to add extra NULLs and 0s for the
     options they aren't interested in.

  2. The http_get_* functions are actually wrappers around
     2 layers of low-level implementation functions. We have
     to pass these options through individually.

  3. The http_get_strbuf wrapper learned these options, but
     nobody bothered to do so for http_get_file, even though
     it is backed by the same function that does understand
     the options.

Let's consolidate the options into a single struct. For the
common case of the default options, we'll allow callers to
simply pass a NULL for the options struct.

The resulting code is often a few lines longer, but it ends
up being easier to read (and to change as we add new
options, since we do not need to update each call site).

Signed-off-by: Jeff King 
Signed-off-by: Jonathan Nieder

http_request: factor out curlinfo_strbuf

2013-09-30T20:04:45+00:00

When we retrieve the content-type of an http response, curl
gives us a pointer to internal storage, which we then copy
into a strbuf. Let's factor out the get-and-copy routine,
which can be used for getting other curl info.

Signed-off-by: Jeff King 
Signed-off-by: Jonathan Nieder