Граф коммитов

160 Коммитов

Автор SHA1 Сообщение Дата
Ramsay Jones 70900eda4a http.c: Spell the null pointer as NULL
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 <ramsay@ramsay1.demon.co.uk>
Acked-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-10-24 14:42:26 -07:00
Jeff King c93c92f309 http: update base URLs when we see redirects
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14 16:56:47 -07:00
Jeff King 78868962c0 http: provide effective url to callers
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14 16:55:23 -07:00
Jeff King 2501aff8b7 http: hoist credential request out of handle_curl_result
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14 16:55:13 -07:00
Jeff King 1bbcc224cc http: refactor options to http_get_*
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-30 17:21:59 -07:00
Jeff King 132b70a2ed http_request: factor out curlinfo_strbuf
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 <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-30 13:04:45 -07:00
Jeff King 3d1fb769b2 http_get_file: style fixes
Besides being ugly, the extra parentheses are idiomatic for
suppressing compiler warnings when we are assigning within a
conditional. We aren't doing that here, and they just
confuse the reader.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-30 13:04:31 -07:00
Junio C Hamano dc2ed04c23 Merge branch 'bc/http-keep-memory-given-to-curl'
Older cURL wanted piece of memory we call it with to be stable, but
we updated the auth material after handing it to a call.

* bc/http-keep-memory-given-to-curl:
  http.c: don't rewrite the user:passwd string multiple times
2013-06-27 14:29:49 -07:00
Brandon Casey a94cf2cb7e http.c: don't rewrite the user:passwd string multiple times
Curl older than 7.17 (RHEL 4.X provides 7.12 and RHEL 5.X provides
7.15) requires that we manage any strings that we pass to it as
pointers.  So, we really shouldn't be modifying this strbuf after we
have passed it to curl.

Our interaction with curl is currently safe (before or after this
patch) since the pointer that is passed to curl is never invalidated;
it is repeatedly rewritten with the same sequence of characters but
the strbuf functions never need to allocate a larger string, so the
same memory buffer is reused.

This "guarantee" of safety is somewhat subtle and could be overlooked
by someone who may want to add a more complex handling of the username
and password.  So, let's stop modifying this strbuf after we have
passed it to curl, but also leave a note to describe the assumptions
that have been made about username/password lifetime and to draw
attention to the code.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-06-19 10:00:26 -07:00
Junio C Hamano 574d51b575 Merge branch 'mv/ssl-ftp-curl'
Does anybody really use commit walkers over (s)ftp?

* mv/ssl-ftp-curl:
  Support FTP-over-SSL/TLS for regular FTP
2013-04-19 13:31:08 -07:00
Jeff King b793acf14c http: set curl FAILONERROR each time we select a handle
Because we reuse curl handles for multiple requests, the
setup of a handle happens in two stages: stable, global
setup and per-request setup. The lifecycle of a handle is
something like:

  1. get_curl_handle; do basic global setup that will last
     through the whole program (e.g., setting the user
     agent, ssl options, etc)

  2. get_active_slot; set up a per-request baseline (e.g.,
     clearing the read/write functions, making it a GET
     request, etc)

  3. perform the request with curl_*_perform functions

  4. goto step 2 to perform another request

Breaking it down this way means we can avoid doing global
setup from step (1) repeatedly, but we still finish step (2)
with a predictable baseline setup that callers can rely on.

Until commit 6d052d7 (http: add HTTP_KEEP_ERROR option,
2013-04-05), setting curl's FAILONERROR option was a global
setup; we never changed it. However, 6d052d7 introduced an
option where some requests might turn off FAILONERROR. Later
requests using the same handle would have the option
unexpectedly turned off, which meant they would not notice
http failures at all.

This could easily be seen in the test-suite for the
"half-auth" cases of t5541 and t5551. The initial requests
turned off FAILONERROR, which meant it was erroneously off
for the rpc POST. That worked fine for a successful request,
but meant that we failed to react properly to the HTTP 401
(instead, we treated whatever the server handed us as a
successful message body).

The solution is simple: now that FAILONERROR is a
per-request setting, we move it to get_active_slot to make
sure it is reset for each request.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-16 10:13:46 -07:00
Modestas Vainius 4bc444eb64 Support FTP-over-SSL/TLS for regular FTP
Add a boolean http.sslTry option which allows to enable AUTH SSL/TLS and
encrypted data transfers when connecting via regular FTP protocol.

Default is false since it might trigger certificate verification errors on
misconfigured servers.

Signed-off-by: Modestas Vainius <modestas@vainius.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-12 08:52:23 -07:00
Jeff King 4df13f69e9 http: drop http_error function
This function is a single-liner and is only called from one
place. Just inline it, which makes the code more obvious.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06 18:56:46 -07:00
Jeff King 39a570f26c http: re-word http error message
When we report an http error code, we say something like:

  error: The requested URL reported failure: 403 Forbidden while accessing http://example.com/repo.git

Everything between "error:" and "while" is written by curl,
and the resulting sentence is hard to read (especially
because there is no punctuation between curl's sentence and
the remainder of ours). Instead, let's re-order this to give
better flow:

  error: unable to access 'http://example.com/repo.git: The requested URL reported failure: 403 Forbidden

This is still annoyingly long, but at least reads more
clearly left to right.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06 18:56:45 -07:00
Jeff King 67d2a7b5c5 http: simplify http_error helper function
This helper function should really be a one-liner that
prints an error message, but it has ended up unnecessarily
complicated:

  1. We call error() directly when we fail to start the curl
     request, so we must later avoid printing a duplicate
     error in http_error().

     It would be much simpler in this case to just stuff the
     error message into our usual curl_errorstr buffer
     rather than printing it ourselves. This means that
     http_error does not even have to care about curl's exit
     value (the interesting part is in the errorstr buffer
     already).

  2. We return the "ret" value passed in to us, but none of
     the callers actually cares about our return value. We
     can just drop this entirely.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06 18:56:44 -07:00
Jeff King 6d052d78d7 http: add HTTP_KEEP_ERROR option
We currently set curl's FAILONERROR option, which means that
any http failures are reported as curl errors, and the
http body content from the server is thrown away.

This patch introduces a new option to http_get_strbuf which
specifies that the body content from a failed http response
should be placed in the destination strbuf, where it can be
accessed by the caller.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06 18:56:41 -07:00
Jeff King 047ec60205 pkt-line: move LARGE_PACKET_MAX definition from sideband
Having the packet sizes defined near the packet read/write
functions makes more sense.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-20 13:42:22 -08:00
Jeff King 3443db51a0 http_request: reset "type" strbuf before adding
Callers may pass us a strbuf which we use to record the
content-type of the response. However, we simply appended to
it rather than overwriting its contents, meaning that cruft
in the strbuf gave us a bogus type. E.g., the multiple
requests triggered by http_request could yield a type like
"text/plainapplication/x-git-receive-pack-advertisement".

Reported-by: Michael Schubert <mschub@elegosoft.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-06 07:50:56 -08:00
Shawn Pearce 4656bf47fc Verify Content-Type from smart HTTP servers
Before parsing a suspected smart-HTTP response verify the returned
Content-Type matches the standard. This protects a client from
attempting to process a payload that smells like a smart-HTTP
server response.

JGit has been doing this check on all responses since the dawn of
time. I mistakenly failed to include it in git-core when smart HTTP
was introduced. At the time I didn't know how to get the Content-Type
from libcurl. I punted, meant to circle back and fix this, and just
plain forgot about it.

Signed-off-by: Shawn Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-04 10:22:36 -08:00
Junio C Hamano 8bc714b408 Merge branch 'rb/http-cert-cred-no-username-prompt' into maint
* rb/http-cert-cred-no-username-prompt:
  http.c: Avoid username prompt for certifcate credentials
2013-01-10 14:03:54 -08:00
Rene Bredlau 75e9a405d4 http.c: Avoid username prompt for certifcate credentials
If sslCertPasswordProtected is set to true do not ask for username to decrypt rsa key. This question is pointless, the key is only protected by a password. Internaly the username is simply set to "".

Signed-off-by: Rene Bredlau <git@unrelated.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-21 10:19:40 -08:00
Jeff King 23a50a1fb1 Merge branch 'sz/maint-curl-multi-timeout'
Sometimes curl_multi_timeout() function suggested a wrong timeout
value when there is no file descriptors to wait on and the http
transport ended up sleeping for minutes in select(2) system call.
Detect this and reduce the wait timeout in such a case.

* sz/maint-curl-multi-timeout:
  Fix potential hang in https handshake
2012-11-09 12:50:56 -05:00
Jeff King 58f3f9893d Merge branch 'jk/maint-http-init-not-in-result-handler'
Further clean-up to the http codepath that picks up results after
cURL library is done with one request slot.

* jk/maint-http-init-not-in-result-handler:
  http: do not set up curl auth after a 401
  remote-curl: do not call run_slot repeatedly
2012-10-29 04:13:09 -04:00
Stefan Zager 7202b81ffc Fix potential hang in https handshake
It has been observed that curl_multi_timeout may return a very long
timeout value (e.g., 294 seconds and some usec) just before
curl_multi_fdset returns no file descriptors for reading.  The
upshot is that select() will hang for a long time -- long enough for
an https handshake to be dropped.  The observed behavior is that
the git command will hang at the terminal and never transfer any
data.

This patch is a workaround for a probable bug in libcurl.  The bug
only seems to manifest around a very specific set of circumstances:

- curl version (from curl/curlver.h):

 #define LIBCURL_VERSION_NUM 0x071307

- git-remote-https running on an ubuntu-lucid VM.
- Connecting through squid proxy running on another VM.

Interestingly, the problem doesn't manifest if a host connects
through squid proxy running on localhost; only if the proxy is on
a separate VM (not sure if the squid host needs to be on a separate
physical machine).  That would seem to suggest that this issue
is timing-sensitive.

This patch is more or less in line with a recommendation in the
curl docs about how to behave when curl_multi_fdset doesn't return
and file descriptors:

http://curl.haxx.se/libcurl/c/curl_multi_fdset.html

Signed-off-by: Stefan Zager <szager@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-19 14:15:17 -07:00
Junio C Hamano e98fa647aa Merge branch 'jk/maint-http-half-auth-push' into maint
* jk/maint-http-half-auth-push:
  http: fix segfault in handle_curl_result
2012-10-17 10:29:24 -07:00
Junio C Hamano 053a08f5bb Merge branch 'jk/maint-http-half-auth-push'
Fixes a regression in maint-1.7.11 (v1.7.11.7), maint (v1.7.12.1)
and master (v1.8.0-rc0).

* jk/maint-http-half-auth-push:
  http: fix segfault in handle_curl_result
2012-10-16 11:44:37 -07:00
Jeff King 1960897ebc http: do not set up curl auth after a 401
When we get an http 401, we prompt for credentials and put
them in our global credential struct. We also feed them to
the curl handle that produced the 401, with the intent that
they will be used on a retry.

When the code was originally introduced in commit 42653c0,
this was a necessary step. However, since dfa1725, we always
feed our global credential into every curl handle when we
initialize the slot with get_active_slot. So every further
request already feeds the credential to curl.

Moreover, accessing the slot here is somewhat dubious. After
the slot has produced a response, we don't actually control
it any more.  If we are using curl_multi, it may even have
been re-initialized to handle a different request.

It just so happens that we will reuse the curl handle within
the slot in such a case, and that because we only keep one
global credential, it will be the one we want.  So the
current code is not buggy, but it is misleading.

By cleaning it up, we can remove the slot argument entirely
from handle_curl_result, making it much more obvious that
slots should not be accessed after they are marked as
finished.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-12 09:45:15 -07:00
Jeff King 188923f0d1 http: fix segfault in handle_curl_result
When we create an http active_request_slot, we can set its
"results" pointer back to local storage. The http code will
fill in the details of how the request went, and we can
access those details even after the slot has been cleaned
up.

Commit 8809703 (http: factor out http error code handling)
switched us from accessing our local results struct directly
to accessing it via the "results" pointer of the slot. That
means we're accessing the slot after it has been marked as
finished, defeating the whole purpose of keeping the results
storage separate.

Most of the time this doesn't matter, as finishing the slot
does not actually clean up the pointer. However, when using
curl's multi interface with the dumb-http revision walker,
we might actually start a new request before handing control
back to the original caller. In that case, we may reuse the
slot, zeroing its results pointer, and leading the original
caller to segfault while looking for its results inside the
slot.

Instead, we need to pass a pointer to our local results
storage to the handle_curl_result function, rather than
relying on the pointer in the slot struct. This matches what
the original code did before the refactoring (which did not
use a separate function, and therefore just accessed the
results struct directly).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-12 09:42:31 -07:00
Shawn O. Pearce aa90b9697f Enable info/refs gzip decompression in HTTP client
Some HTTP servers try to use gzip compression on the /info/refs
request to save transfer bandwidth. Repositories with many tags
may find the /info/refs request can be gzipped to be 50% of the
original size due to the few but often repeated bytes used (hex
SHA-1 and commonly digits in tag names).

For most HTTP requests enable "Accept-Encoding: gzip" ensuring
the /info/refs payload can use this encoding format.

Only request gzip encoding from servers. Although deflate is
supported by libcurl, most servers have standardized on gzip
encoding for compression as that is what most browsers support.
Asking for deflate increases request sizes by a few bytes, but is
unlikely to ever be used by a server.

Disable the Accept-Encoding header on probe RPCs as response bodies
are supposed to be exactly 4 bytes long, "0000". The HTTP headers
requesting and indicating compression use more space than the data
transferred in the body.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-09-20 10:26:50 -07:00
Junio C Hamano 3503e9ab32 Merge branch 'maint-1.7.11' into maint 2012-09-12 14:08:05 -07:00
Junio C Hamano 7d9483c299 Merge branch 'jk/maint-http-half-auth-push' into maint-1.7.11
Pushing to smart HTTP server with recent Git fails without having
the username in the URL to force authentication, if the server is
configured to allow GET anonymously, while requiring authentication
for POST.

* jk/maint-http-half-auth-push:
  http: prompt for credentials on failed POST
  http: factor out http error code handling
  t: test http access to "half-auth" repositories
  t: test basic smart-http authentication
  t/lib-httpd: recognize */smart/* repos as smart-http
  t/lib-httpd: only route auth/dumb to dumb repos
  t5550: factor out http auth setup
  t5550: put auth-required repo in auth/dumb
2012-09-12 13:58:23 -07:00
Jeff King 8809703072 http: factor out http error code handling
Most of our http requests go through the http_request()
interface, which does some nice post-processing on the
results. In particular, it handles prompting for missing
credentials as well as approving and rejecting valid or
invalid credentials. Unfortunately, it only handles GET
requests. Making it handle POSTs would be quite complex, so
let's pull result handling code into its own function so
that it can be reused from the POST code paths.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-27 10:49:09 -07:00
Joachim Schmitz 4246b0bd90 http.c: don't use curl_easy_strerror prior to curl-7.12.0
Reverts be22d92 (http: avoid empty error messages for some curl
errors, 2011-09-05) on platforms with older versions of libcURL
where the function is not available.

Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-23 14:23:18 -07:00
Jeff King 745c7c8e62 http: get default user-agent from git_user_agent
This means we will respect the GIT_USER_AGENT build-time
configuration and run-time environment variable.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-03 13:11:54 -07:00
Pete Wyckoff 82247e9bd5 remove superfluous newlines in error messages
The error handling routines add a newline.  Remove
the duplicate ones in error messages.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-30 15:45:51 -07:00
Jeff King 6f4c347ca1 http: use newer curl options for setting credentials
We give the username and password to curl by sticking them
in a buffer of the form "user:pass" and handing the result
to CURLOPT_USERPWD. Since curl 7.19.1, there is a split
mechanism, where you can specify each element individually.

This has the advantage that a username can contain a ":"
character. It also is less code for us, since we can hand
our strings over to curl directly. And since curl 7.17.0 and
higher promise to copy the strings for us, we we don't even
have to worry about memory ownership issues.

Unfortunately, we have to keep the ugly code for old curl
around, but as it is now nicely #if'd out, we can easily get
rid of it when we decide that 7.19.1 is "old enough".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-14 16:04:25 -07:00
Jeff King aa0834a04e http: clean up leak in init_curl_http_auth
When we have a credential to give to curl, we must copy it
into a "user:pass" buffer and then hand the buffer to curl.
Old versions of curl did not copy the buffer, and we were
expected to keep it valid. Newer versions of curl will copy
the buffer.

Our solution was to use a strbuf and detach it, giving
ownership of the resulting buffer to curl. However, this
meant that we were leaking the buffer on newer versions of
curl, since curl was just copying it and throwing away the
string we passed. Furthermore, when we replaced a
credential (e.g., because our original one was rejected), we
were also leaking on both old and new versions of curl.

This got even worse in the last patch, which started
replacing the credential (and thus leaking) on every http
request.

Instead, let's use a static buffer to make the ownership
more clear and less leaky.  We already keep a static "struct
credential", so we are only handling a single credential at
a time, anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-14 16:04:24 -07:00
Jeff King dfa1725a3e fix http auth with multiple curl handles
HTTP authentication is currently handled by get_refs and fetch_ref, but
not by fetch_object, fetch_pack or fetch_alternates. In the
single-threaded case, this is not an issue, since get_refs is always
called first. It recognigzes the 401 and prompts the user for
credentials, which will then be used subsequently.

If the curl multi interface is used, however, only the multi handle used
by get_refs will have credentials configured. Requests made by other
handles fail with an authentication error.

Fix this by setting CURLOPT_USERPWD whenever a slot is requested.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-10 09:12:13 -07:00
Jim Meyering a7793a7491 correct spelling: an URL -> a URL
Signed-off-by: Jim Meyering <meyering@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-03-28 08:47:23 -07:00
Nelson Benitez Leon dd6139971a http: support proxies that require authentication
When the proxy server specified by the http.proxy configuration or the
http_proxy environment variable requires authentication, git failed to
connect to the proxy, because we did not configure the cURL handle with
CURLOPT_PROXYAUTH.

When a proxy is in use, and you tell git that the proxy requires
authentication by having username in the http.proxy configuration, an
extra request needs to be made to the proxy to find out what
authentication method it supports, as this patch uses CURLAUTH_ANY to let
the library pick the most secure method supported by the proxy server.

The extra round-trip adds extra latency, but relieves the user from the
burden to configure a specific authentication method.  If it becomes
problem, a later patch could add a configuration option to specify what
method to use, but let's start simple for the time being.

Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-03-02 14:40:14 -08:00
Junio C Hamano 1d3a035d6d Merge branch 'jk/maint-push-over-dav'
* jk/maint-push-over-dav:
  http-push: enable "proactive auth"
  t5540: test DAV push with authentication

Conflicts:
	http.c
2011-12-19 16:05:59 -08:00
Junio C Hamano 367d20ec6b Merge branch 'jk/credentials'
* jk/credentials:
  t: add test harness for external credential helpers
  credentials: add "store" helper
  strbuf: add strbuf_add*_urlencode
  Makefile: unix sockets may not available on some platforms
  credentials: add "cache" helper
  docs: end-user documentation for the credential subsystem
  credential: make relevance of http path configurable
  credential: add credential.*.username
  credential: apply helper config
  http: use credential API to get passwords
  credential: add function for parsing url components
  introduce credentials API
  t5550: fix typo
  test-lib: add test_config_global variant

Conflicts:
	strbuf.c
2011-12-19 16:05:16 -08:00
Jeff King a4ddbc33d7 http-push: enable "proactive auth"
Before commit 986bbc08, git was proactive about asking for
http passwords. It assumed that if you had a username in
your URL, you would also want a password, and asked for it
before making any http requests.

However, this could interfere with the use of .netrc (see
986bbc08 for details). And it was also unnecessary, since
the http fetching code had learned to recognize an HTTP 401
and prompt the user then. Furthermore, the proactive prompt
could interfere with the usage of .netrc (see 986bbc08 for
details).

Unfortunately, the http push-over-DAV code never learned to
recognize HTTP 401, and so was broken by this change. This
patch does a quick fix of re-enabling the "proactive auth"
strategy only for http-push, leaving the dumb http fetch and
smart-http as-is.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-12-13 16:34:44 -08:00
Jeff King 148bb6a7b4 http: use credential API to get passwords
This patch converts the http code to use the new credential
API, both for http authentication as well as for getting
certificate passwords.

Most of the code change is simply variable naming (the
passwords are now contained inside the credential struct)
or deletion of obsolete code (the credential code handles
URL parsing and prompting for us).

The behavior should be the same, with one exception: the
credential code will prompt with a description based on the
credential components. Therefore, the old prompt of:

  Username for 'example.com':
  Password for 'example.com':

now looks like:

  Username for 'https://example.com/repo.git':
  Password for 'https://user@example.com/repo.git':

Note that we include more information in each line,
specifically:

  1. We now include the protocol. While more noisy, this is
     an important part of knowing what you are accessing
     (especially if you care about http vs https).

  2. We include the username in the password prompt. This is
     not a big deal when you have just been prompted for it,
     but the username may also come from the remote's URL
     (and after future patches, from configuration or
     credential helpers).  In that case, it's a nice
     reminder of the user for which you're giving the
     password.

  3. We include the path component of the URL. In many
     cases, the user won't care about this and it's simply
     noise (i.e., they'll use the same credential for a
     whole site). However, that is part of a larger
     question, which is whether path components should be
     part of credential context, both for prompting and for
     lookup by storage helpers. That issue will be addressed
     as a whole in a future patch.

Similarly, for unlocking certificates, we used to say:

  Certificate Password for 'example.com':

and we now say:

  Password for 'cert:///path/to/certificate':

Showing the path to the client certificate makes more sense,
as that is what you are unlocking, not "example.com".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-12-11 23:16:24 -08:00
Junio C Hamano c4c9a63b54 Merge branch 'mf/curl-select-fdset'
* mf/curl-select-fdset:
  http: drop "local" member from request struct
  http.c: Rely on select instead of tracking whether data was received
  http.c: Use timeout suggested by curl instead of fixed 50ms timeout
  http.c: Use curl_multi_fdset to select on curl fds instead of just sleeping
2011-12-05 15:10:28 -08:00
Ramkumar Ramachandra 620771c83e http: remove unused function hex()
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-15 16:08:48 -08:00
Jeff King 093c44a360 http: drop "local" member from request struct
This is a FILE pointer in the case that we are sending our
output to a file. We originally used it to run ftell() to
determine whether data had been written to our file during
our last call to curl. However, as of the last patch, we no
longer care about that flag anymore. All uses of this struct
member are now just book-keeping that can go away.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-04 12:05:01 -07:00
Mika Fischer df26c47127 http.c: Rely on select instead of tracking whether data was received
Since now select is used with the file descriptors of the http connections,
tracking whether data was received recently (and trying to read more in
that case) is no longer necessary. Instead, always call select and rely on
it to return as soon as new data can be read.

Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-04 10:47:13 -07:00
Mika Fischer eb56c82163 http.c: Use timeout suggested by curl instead of fixed 50ms timeout
Recent versions of curl can suggest a period of time the library user
should sleep and try again, when curl is blocked on reading or writing
(or connecting). Use this timeout instead of always sleeping for 50ms.

Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
Helped-by: Daniel Stenberg <daniel@haxx.se>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-04 10:46:56 -07:00
Mika Fischer 6f9dd67ffe http.c: Use curl_multi_fdset to select on curl fds instead of just sleeping
Instead of sleeping unconditionally for a 50ms, when no data can be read
from the http connection(s), use curl_multi_fdset() to obtain the actual
file descriptors of the open connections and use them in the select call.
This way, the 50ms sleep is interrupted when new data arrives.

Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
Helped-by: Daniel Stenberg <daniel@haxx.se>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-11-04 10:46:25 -07:00