* HTTP NTLM Authentication @ 2009-10-02 17:28 gsky 2009-04-14 21:56 ` [PATCH v2] Add an option for using any HTTP authentication scheme, not only basic Martin Storsjö 2009-10-02 19:04 ` [PATCH] Use the best HTTP authentication method supported by the server Nicholas Miell 0 siblings, 2 replies; 28+ messages in thread From: gsky @ 2009-10-02 17:28 UTC (permalink / raw) To: git Is is possible for me to pass arguments to the curl calls that git uses to access a repository hosted over HTTP? I am having a problem accessing the repository as it is authenticated using NTLM, I can curl the repository using curl --ntlm http://username:pass@machine.domain/git How can I do the same for the git clone of the repository? Is it possible easily, or do I have to modify the source and recompile? gsky -- View this message in context: http://www.nabble.com/HTTP-NTLM-Authentication-tp25718488p25718488.html Sent from the git mailing list archive at Nabble.com. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] Add an option for using any HTTP authentication scheme, not only basic @ 2009-04-14 21:56 ` Martin Storsjö 2009-04-14 20:52 ` [PATCH] " Martin Storsjö 2009-11-27 15:41 ` [PATCH 0/2] http: allow multi-pass authentication Tay Ray Chuan 0 siblings, 2 replies; 28+ messages in thread From: Martin Storsjö @ 2009-04-14 21:56 UTC (permalink / raw) To: git This adds the configuration option http.authAny (overridable with the environment variable GIT_HTTP_AUTH_ANY), for instructing curl to allow any HTTP authentication scheme, not only basic (which sends the password in plaintext). When this is enabled, curl has to do double requests most of the time, in order to discover which HTTP authentication method to use, which lowers the performance slightly. Therefore this isn't enabled by default. One example of another authentication scheme to use is digest, which doesn't send the password in plaintext, but uses a challenge-response mechanism instead. Using digest authentication in practice requires at least curl 7.18.1, due to bugs in the digest handling in earlier versions of curl. Signed-off-by: Martin Storsjo <martin@martin•st> --- Repost with the curl version checked in only one place. Documentation/config.txt | 7 +++++++ http.c | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f3ebd2f..1515d77 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1011,6 +1011,13 @@ http.noEPSV:: support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV' environment variable. Default is false (curl will use EPSV). +http.authAny:: + Allow any HTTP authentication method, not only basic. Enabling + this lowers the performance slightly, by having to do requests + without any authentication to discover the authentication method + to use. Can be overridden by the 'GIT_HTTP_AUTH_ANY' + environment variable. Default is false. + i18n.commitEncoding:: Character encoding the commit messages are stored in; git itself does not care per se, but this information is necessary e.g. when diff --git a/http.c b/http.c index 2e3d649..49b8441 100644 --- a/http.c +++ b/http.c @@ -3,6 +3,10 @@ int data_received; int active_requests; +#if LIBCURL_VERSION_NUM >= 0x070a06 +#define LIBCURL_CAN_HANDLE_AUTH_ANY +#endif + #ifdef USE_CURL_MULTI static int max_requests = -1; static CURLM *curlm; @@ -26,6 +30,9 @@ static long curl_low_speed_time = -1; static int curl_ftp_no_epsv; static const char *curl_http_proxy; static char *user_name, *user_pass; +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY +static int curl_http_auth_any = 0; +#endif static struct curl_slist *pragma_header; @@ -150,6 +157,12 @@ static int http_options(const char *var, const char *value, void *cb) } if (!strcmp("http.proxy", var)) return git_config_string(&curl_http_proxy, var, value); +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (!strcmp("http.authany", var)) { + curl_http_auth_any = git_config_bool(var, value); + return 0; + } +#endif /* Fall back on the default ones */ return git_default_config(var, value, cb); @@ -184,6 +197,10 @@ static CURL *get_curl_handle(void) #if LIBCURL_VERSION_NUM >= 0x070907 curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); #endif +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (curl_http_auth_any) + curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY); +#endif init_curl_http_auth(result); @@ -329,6 +346,11 @@ void http_init(struct remote *remote) if (getenv("GIT_CURL_FTP_NO_EPSV")) curl_ftp_no_epsv = 1; +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (getenv("GIT_HTTP_AUTH_ANY")) + curl_http_auth_any = 1; +#endif + if (remote && remote->url && remote->url[0]) http_auth_init(remote->url[0]); -- 1.6.0.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH] Add an option for using any HTTP authentication scheme, not only basic @ 2009-04-14 20:52 ` Martin Storsjö 2009-04-14 21:08 ` Johannes Schindelin 0 siblings, 1 reply; 28+ messages in thread From: Martin Storsjö @ 2009-04-14 20:52 UTC (permalink / raw) To: git This adds the configuration option http.authAny (overridable with the environment variable GIT_HTTP_AUTH_ANY), for instructing curl to allow any HTTP authentication scheme, not only basic (which sends the password in plaintext). When this is enabled, curl has to do double requests most of the time, in order to discover which HTTP authentication method to use, which lowers the performance slightly. Therefore this isn't enabled by default. One example of another authentication scheme to use is digest, which doesn't send the password in plaintext, but uses a challenge-response mechanism instead. Using digest authentication in practice requires at least curl 7.18.2, due to bugs in the digest handling in earlier versions of curl. Signed-off-by: Martin Storsjo <martin@martin•st> --- This is a resend of a patch I sent a few weeks ago. Now this functionality is configurable and disabled by default. Documentation/config.txt | 7 +++++++ http.c | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f3ebd2f..1515d77 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1011,6 +1011,13 @@ http.noEPSV:: support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV' environment variable. Default is false (curl will use EPSV). +http.authAny:: + Allow any HTTP authentication method, not only basic. Enabling + this lowers the performance slightly, by having to do requests + without any authentication to discover the authentication method + to use. Can be overridden by the 'GIT_HTTP_AUTH_ANY' + environment variable. Default is false. + i18n.commitEncoding:: Character encoding the commit messages are stored in; git itself does not care per se, but this information is necessary e.g. when diff --git a/http.c b/http.c index 2e3d649..0b18c64 100644 --- a/http.c +++ b/http.c @@ -26,6 +26,9 @@ static long curl_low_speed_time = -1; static int curl_ftp_no_epsv; static const char *curl_http_proxy; static char *user_name, *user_pass; +#if LIBCURL_VERSION_NUM >= 0x070a06 +static int curl_http_auth_any = 0; +#endif static struct curl_slist *pragma_header; @@ -150,6 +153,12 @@ static int http_options(const char *var, const char *value, void *cb) } if (!strcmp("http.proxy", var)) return git_config_string(&curl_http_proxy, var, value); +#if LIBCURL_VERSION_NUM >= 0x070a06 + if (!strcmp("http.authany", var)) { + curl_http_auth_any = git_config_bool(var, value); + return 0; + } +#endif /* Fall back on the default ones */ return git_default_config(var, value, cb); @@ -184,6 +193,10 @@ static CURL *get_curl_handle(void) #if LIBCURL_VERSION_NUM >= 0x070907 curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); #endif +#if LIBCURL_VERSION_NUM >= 0x070a06 + if (curl_http_auth_any) + curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY); +#endif init_curl_http_auth(result); @@ -329,6 +342,11 @@ void http_init(struct remote *remote) if (getenv("GIT_CURL_FTP_NO_EPSV")) curl_ftp_no_epsv = 1; +#if LIBCURL_VERSION_NUM >= 0x070a06 + if (getenv("GIT_HTTP_AUTH_ANY")) + curl_http_auth_any = 1; +#endif + if (remote && remote->url && remote->url[0]) http_auth_init(remote->url[0]); -- 1.6.0.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] Add an option for using any HTTP authentication scheme, not only basic 2009-04-14 20:52 ` [PATCH] " Martin Storsjö @ 2009-04-14 21:08 ` Johannes Schindelin 2009-04-14 21:15 ` Martin Storsjö 0 siblings, 1 reply; 28+ messages in thread From: Johannes Schindelin @ 2009-04-14 21:08 UTC (permalink / raw) To: Martin Storsjö; +Cc: git [-- Attachment #1: Type: TEXT/PLAIN, Size: 610 bytes --] Hi, On Tue, 14 Apr 2009, Martin Storsjö wrote: > diff --git a/http.c b/http.c > index 2e3d649..0b18c64 100644 > --- a/http.c > +++ b/http.c > @@ -26,6 +26,9 @@ static long curl_low_speed_time = -1; > static int curl_ftp_no_epsv; > static const char *curl_http_proxy; > static char *user_name, *user_pass; > +#if LIBCURL_VERSION_NUM >= 0x070a06 > +static int curl_http_auth_any = 0; > +#endif In six months from now, it might be easier to read #if LIBCURL_VERSION_NUM >= 0x070a06 #define LIBCURL_CAN_HANDLE_ANY_AUTH #endif [...] #ifdef LIBCURL_CAN_HANDLE_ANY_AUTH [...] Don't you agree? Thanks, Dscho ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add an option for using any HTTP authentication scheme, not only basic 2009-04-14 21:08 ` Johannes Schindelin @ 2009-04-14 21:15 ` Martin Storsjö 2009-04-14 21:42 ` Johannes Schindelin 0 siblings, 1 reply; 28+ messages in thread From: Martin Storsjö @ 2009-04-14 21:15 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git Hi, On Tue, 14 Apr 2009, Johannes Schindelin wrote: > In six months from now, it might be easier to read > > #if LIBCURL_VERSION_NUM >= 0x070a06 > #define LIBCURL_CAN_HANDLE_ANY_AUTH > #endif > > [...] > > #ifdef LIBCURL_CAN_HANDLE_ANY_AUTH > [...] > > Don't you agree? Yeah, absolutely. Any comments on the naming of options and env variables? Otherwise, I'll resend an updated version soon. // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add an option for using any HTTP authentication scheme, not only basic 2009-04-14 21:15 ` Martin Storsjö @ 2009-04-14 21:42 ` Johannes Schindelin 0 siblings, 0 replies; 28+ messages in thread From: Johannes Schindelin @ 2009-04-14 21:42 UTC (permalink / raw) To: Martin Storsjö; +Cc: git [-- Attachment #1: Type: TEXT/PLAIN, Size: 534 bytes --] Hi, On Wed, 15 Apr 2009, Martin Storsjö wrote: > On Tue, 14 Apr 2009, Johannes Schindelin wrote: > > > In six months from now, it might be easier to read > > > > #if LIBCURL_VERSION_NUM >= 0x070a06 > > #define LIBCURL_CAN_HANDLE_ANY_AUTH > > #endif > > > > [...] > > > > #ifdef LIBCURL_CAN_HANDLE_ANY_AUTH > > [...] > > > > Don't you agree? > > Yeah, absolutely. > > Any comments on the naming of options and env variables? Otherwise, I'll > resend an updated version soon. They sound sensible enough to me! Thanks, Dscho ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/2] http: allow multi-pass authentication 2009-04-14 21:56 ` [PATCH v2] Add an option for using any HTTP authentication scheme, not only basic Martin Storsjö 2009-04-14 20:52 ` [PATCH] " Martin Storsjö @ 2009-11-27 15:41 ` Tay Ray Chuan 2009-11-27 15:42 ` [PATCH 1/2] http: maintain curl sessions Tay Ray Chuan 2009-12-01 10:28 ` [PATCH 0/2] http: allow multi-pass authentication Martin Storsjö 1 sibling, 2 replies; 28+ messages in thread From: Tay Ray Chuan @ 2009-11-27 15:41 UTC (permalink / raw) To: git Cc: Martin Storsjö, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin This patch series applies on top of master. It enables fetching and pushing over http with the most suitable authentication scheme chosen by curl when the http.authAny or GIT_HTTP_AUTH_ANY is set. Authorization headers can also be preserved across requests, with at least 1 curl session being preserved by default. This is especially useful for the smart http protocol, where it is hard to rewind and re- send a request. Nicholas, Martin's patch should lead to similar functionality as your patch (dated Oct 3rd) would. However, unlike your patch, CURLOPT_HTTPAUTH is set even if the user name is not specified explicitly in the remote url, since it's conditional on http.c::user_name being set. I've tested this with Digest, and I believe this should work with NTLM too. gsky, could you try this out with NTLM? =?ISO-8859-15?Q?Martin_Storsj=F6?= (1): Add an option for using any HTTP authentication scheme, not only basic Tay Ray Chuan (1): http: maintain curl sessions Documentation/config.txt | 13 +++++++++++++ http.c | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/2] http: maintain curl sessions 2009-11-27 15:41 ` [PATCH 0/2] http: allow multi-pass authentication Tay Ray Chuan @ 2009-11-27 15:42 ` Tay Ray Chuan 2009-11-27 15:43 ` [PATCH 2/2] Add an option for using any HTTP authentication scheme, not only basic Tay Ray Chuan 2009-12-01 10:28 ` [PATCH 0/2] http: allow multi-pass authentication Martin Storsjö 1 sibling, 1 reply; 28+ messages in thread From: Tay Ray Chuan @ 2009-11-27 15:42 UTC (permalink / raw) To: git Allow curl sessions to be kept alive (ie. not ended with curl_easy_cleanup()) even after the request is completed, the number of which is determined by the configuration setting http.minSessions. Add a count for curl sessions, and update it, across slots, when starting and ending curl sessions. Signed-off-by: Tay Ray Chuan <rctay89@gmail•com> --- Documentation/config.txt | 6 ++++++ http.c | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index a8e0876..b77d66d 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1132,6 +1132,12 @@ http.maxRequests:: How many HTTP requests to launch in parallel. Can be overridden by the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5. +http.minSessions:: + The number of curl sessions (counted across slots) to be kept across + requests. They will not be ended with curl_easy_cleanup() until + http_cleanup() is invoked. If USE_CURL_MULTI is not defined, this + value will be capped at 1. Defaults to 1. + http.postBuffer:: Maximum size in bytes of the buffer used by smart HTTP transports when POSTing data to the remote system. diff --git a/http.c b/http.c index ed6414a..fb0a97b 100644 --- a/http.c +++ b/http.c @@ -7,6 +7,8 @@ int active_requests; int http_is_verbose; size_t http_post_buffer = 16 * LARGE_PACKET_MAX; +static int min_curl_sessions = 1; +static int curl_session_count; #ifdef USE_CURL_MULTI static int max_requests = -1; static CURLM *curlm; @@ -152,6 +154,14 @@ static int http_options(const char *var, const char *value, void *cb) ssl_cert_password_required = 1; return 0; } + if (!strcmp("http.minsessions", var)) { + min_curl_sessions = git_config_int(var, value); +#ifndef USE_CURL_MULTI + if (min_curl_sessions > 1) + min_curl_sessions = 1; +#endif + return 0; + } #ifdef USE_CURL_MULTI if (!strcmp("http.maxrequests", var)) { max_requests = git_config_int(var, value); @@ -372,6 +382,7 @@ void http_init(struct remote *remote) if (curl_ssl_verify == -1) curl_ssl_verify = 1; + curl_session_count = 0; #ifdef USE_CURL_MULTI if (max_requests < 1) max_requests = DEFAULT_MAX_REQUESTS; @@ -480,6 +491,7 @@ struct active_request_slot *get_active_slot(void) #else slot->curl = curl_easy_duphandle(curl_default); #endif + curl_session_count++; } active_requests++; @@ -558,9 +570,11 @@ void fill_active_slots(void) } while (slot != NULL) { - if (!slot->in_use && slot->curl != NULL) { + if (!slot->in_use && slot->curl != NULL + && curl_session_count > min_curl_sessions) { curl_easy_cleanup(slot->curl); slot->curl = NULL; + curl_session_count--; } slot = slot->next; } @@ -633,12 +647,13 @@ static void closedown_active_slot(struct active_request_slot *slot) void release_active_slot(struct active_request_slot *slot) { closedown_active_slot(slot); - if (slot->curl) { + if (slot->curl && curl_session_count > min_curl_sessions) { #ifdef USE_CURL_MULTI curl_multi_remove_handle(curlm, slot->curl); #endif curl_easy_cleanup(slot->curl); slot->curl = NULL; + curl_session_count--; } #ifdef USE_CURL_MULTI fill_active_slots(); -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/2] Add an option for using any HTTP authentication scheme, not only basic 2009-11-27 15:42 ` [PATCH 1/2] http: maintain curl sessions Tay Ray Chuan @ 2009-11-27 15:43 ` Tay Ray Chuan 0 siblings, 0 replies; 28+ messages in thread From: Tay Ray Chuan @ 2009-11-27 15:43 UTC (permalink / raw) To: git From: =?ISO-8859-15?Q?Martin_Storsj=F6?= <martin@martin•st> This adds the configuration option http.authAny (overridable with the environment variable GIT_HTTP_AUTH_ANY), for instructing curl to allow any HTTP authentication scheme, not only basic (which sends the password in plaintext). When this is enabled, curl has to do double requests most of the time, in order to discover which HTTP authentication method to use, which lowers the performance slightly. Therefore this isn't enabled by default. One example of another authentication scheme to use is digest, which doesn't send the password in plaintext, but uses a challenge-response mechanism instead. Using digest authentication in practice requires at least curl 7.18.1, due to bugs in the digest handling in earlier versions of curl. Signed-off-by: Martin Storsjo <martin@martin•st> Signed-off-by: Tay Ray Chuan <rctay89@gmail•com> --- Martin, I've not made any changes, except to make it apply cleanly. Documentation/config.txt | 7 +++++++ http.c | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index b77d66d..a54ede3 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1158,6 +1158,13 @@ http.noEPSV:: support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV' environment variable. Default is false (curl will use EPSV). +http.authAny:: + Allow any HTTP authentication method, not only basic. Enabling + this lowers the performance slightly, by having to do requests + without any authentication to discover the authentication method + to use. Can be overridden by the 'GIT_HTTP_AUTH_ANY' + environment variable. Default is false. + i18n.commitEncoding:: Character encoding the commit messages are stored in; git itself does not care per se, but this information is necessary e.g. when diff --git a/http.c b/http.c index fb0a97b..aeb69b3 100644 --- a/http.c +++ b/http.c @@ -7,6 +7,10 @@ int active_requests; int http_is_verbose; size_t http_post_buffer = 16 * LARGE_PACKET_MAX; +#if LIBCURL_VERSION_NUM >= 0x070a06 +#define LIBCURL_CAN_HANDLE_AUTH_ANY +#endif + static int min_curl_sessions = 1; static int curl_session_count; #ifdef USE_CURL_MULTI @@ -36,6 +40,9 @@ static long curl_low_speed_time = -1; static int curl_ftp_no_epsv; static const char *curl_http_proxy; static char *user_name, *user_pass; +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY +static int curl_http_auth_any = 0; +#endif #if LIBCURL_VERSION_NUM >= 0x071700 /* Use CURLOPT_KEYPASSWD as is */ @@ -190,6 +197,12 @@ static int http_options(const char *var, const char *value, void *cb) http_post_buffer = LARGE_PACKET_MAX; return 0; } +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (!strcmp("http.authany", var)) { + curl_http_auth_any = git_config_bool(var, value); + return 0; + } +#endif /* Fall back on the default ones */ return git_default_config(var, value, cb); @@ -240,6 +253,10 @@ static CURL *get_curl_handle(void) #if LIBCURL_VERSION_NUM >= 0x070907 curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); #endif +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (curl_http_auth_any) + curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY); +#endif init_curl_http_auth(result); @@ -391,6 +408,11 @@ void http_init(struct remote *remote) if (getenv("GIT_CURL_FTP_NO_EPSV")) curl_ftp_no_epsv = 1; +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY + if (getenv("GIT_HTTP_AUTH_ANY")) + curl_http_auth_any = 1; +#endif + if (remote && remote->url && remote->url[0]) { http_auth_init(remote->url[0]); if (!ssl_cert_password_required && -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 0/2] http: allow multi-pass authentication 2009-11-27 15:41 ` [PATCH 0/2] http: allow multi-pass authentication Tay Ray Chuan 2009-11-27 15:42 ` [PATCH 1/2] http: maintain curl sessions Tay Ray Chuan @ 2009-12-01 10:28 ` Martin Storsjö 2009-12-01 10:33 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer Martin Storsjö 2009-12-01 10:37 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time Martin Storsjö 1 sibling, 2 replies; 28+ messages in thread From: Martin Storsjö @ 2009-12-01 10:28 UTC (permalink / raw) To: Tay Ray Chuan Cc: git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Fri, 27 Nov 2009, Tay Ray Chuan wrote: > This patch series applies on top of master. It enables fetching and > pushing over http with the most suitable authentication scheme chosen > by curl when the http.authAny or GIT_HTTP_AUTH_ANY is set. I also tested this, and things generally seem to work fine. Thanks to the "maintain curl sessions" patch, only the first request needs to be redone after getting the 401 error containing the authentication challenge, the later ones work fine on the first try. However, theoretically, I guess we can't be certain that the curl session really is initialized for the later requests (we could be given a new fresh curl session for some reason), or the first request could perhaps be a large, (currently) non-rewindable POST. Avoiding redoing large POST requests is generally accomplished by adding a Expect: 100-continue header, and then waiting for a reply (either 100 continue or 401 unauthorized) to that header before actually sending the POST body data. If the server doesn't support the Expect header (e.g. Lighttpd doesn't support it), the client starts sending the POST body after a timeout (1 second in libcurl). (As a side note, chunked POST requests without a content-length header isn't supported by lighttpd at all at the moment, neither in the stable 1.4 version nor in the new upcoming 1.5 branch.) Normally, libcurl should add the Expect: 100-continue header automatically, but for some reason (http://article.gmane.org/gmane.comp.web.curl.library/25992) it doesn't, so that's probably why we're manually adding that header in remote-curl.c:371 at the moment. libcurl doesn't detect this at the moment (http://article.gmane.org/gmane.comp.web.curl.library/25991) so it won't wait for the 100 continue response before starting to send the body data. So, with a server supporting Expect, the 401 error response may come after sending a few KB of POST data (corresponding to the roundtrip delay for the server to respond to the header) - if the server doesn't support Expect at all, the whole request will be sent and may need to be rewound. To clarify - this only happens if the curl authentication isn't initialized yet, for the first request of every curl session. The "maintain curl sessions" patch makes sure this isn't needed in the normal case. I've experimented with two solutions to this, which add partial and full rewind solutions to the chunked POST requests - I'll send them as follow-ups to this mail. // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 10:28 ` [PATCH 0/2] http: allow multi-pass authentication Martin Storsjö @ 2009-12-01 10:33 ` Martin Storsjö 2009-12-01 16:01 ` Shawn O. Pearce 2009-12-01 17:49 ` Junio C Hamano 2009-12-01 10:37 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time Martin Storsjö 1 sibling, 2 replies; 28+ messages in thread From: Martin Storsjö @ 2009-12-01 10:33 UTC (permalink / raw) To: Tay Ray Chuan Cc: git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin When using multi-pass authentication methods, the curl library may need to rewind the read buffers used for providing data to HTTP POST, if data has been output before a 401 error is received. This is needed only when the first request (when the multi-pass authentication method isn't initialized and hasn't received its challenge yet) for a certain curl session is a chunked HTTP POST. As long as the current rpc read buffer is the first one, we're able to rewind without need for additional buffering. The curl library currently starts sending data without waiting for a response to the Expect: 100-continue header, due to a bug in curl that exists up to curl version 7.19.7. If the HTTP server doesn't handle Expect: 100-continue headers properly (e.g. Lighttpd), the library has to start sending data without knowing if the request will be successfully authenticated. In this case, this rewinding solution is not sufficient - the whole request will be sent before the 401 error is received. Signed-off-by: Martin Storsjo <martin@martin•st> --- The curl bug is yet unconfirmed upstream, discussed here: http://article.gmane.org/gmane.comp.web.curl.library/25991 remote-curl.c | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index a331bae..28b2a31 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -290,6 +290,7 @@ struct rpc_state { int out; struct strbuf result; unsigned gzip_request : 1; + unsigned initial_buffer : 1; }; static size_t rpc_out(void *ptr, size_t eltsize, @@ -300,6 +301,7 @@ static size_t rpc_out(void *ptr, size_t eltsize, size_t avail = rpc->len - rpc->pos; if (!avail) { + rpc->initial_buffer = 0; avail = packet_read_line(rpc->out, rpc->buf, rpc->alloc); if (!avail) return 0; @@ -314,6 +316,29 @@ static size_t rpc_out(void *ptr, size_t eltsize, return avail; } +#ifndef NO_CURL_IOCTL +curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp) +{ + struct rpc_state *rpc = clientp; + + switch (cmd) { + case CURLIOCMD_NOP: + return CURLIOE_OK; + + case CURLIOCMD_RESTARTREAD: + if (rpc->initial_buffer) { + rpc->pos = 0; + return CURLIOE_OK; + } + fprintf(stderr, "Unable to rewind rpc post data - try increasing http.postBuffer\n"); + return CURLIOE_FAILRESTART; + + default: + return CURLIOE_UNKNOWNCMD; + } +} +#endif + static size_t rpc_in(const void *ptr, size_t eltsize, size_t nmemb, void *buffer_) { @@ -370,8 +395,13 @@ static int post_rpc(struct rpc_state *rpc) */ headers = curl_slist_append(headers, "Expect: 100-continue"); headers = curl_slist_append(headers, "Transfer-Encoding: chunked"); + rpc->initial_buffer = 1; curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out); curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc); +#ifndef NO_CURL_IOCTL + curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl); + curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc); +#endif if (options.verbosity > 1) { fprintf(stderr, "POST %s (chunked)\n", rpc->service_name); fflush(stderr); -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 10:33 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer Martin Storsjö @ 2009-12-01 16:01 ` Shawn O. Pearce 2009-12-01 16:12 ` Tay Ray Chuan 2009-12-01 16:51 ` Martin Storsjö 2009-12-01 17:49 ` Junio C Hamano 1 sibling, 2 replies; 28+ messages in thread From: Shawn O. Pearce @ 2009-12-01 16:01 UTC (permalink / raw) To: Martin Storsj? Cc: Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Martin Storsj? <martin@martin•st> wrote: > When using multi-pass authentication methods, the curl library may need > to rewind the read buffers used for providing data to HTTP POST, if data > has been output before a 401 error is received. In theory, since the cURL session stays active, we would have received the 401 authentication error during the initial "GET $GIT_DIR/info/refs?service=git-$service" request, and the subsequent "POST $GIT_DIR/git-$service" requests would automatically include the authentication data. That's theory. Reality doesn't always agree with my theories. :-) > remote-curl.c | 30 ++++++++++++++++++++++++++++++ > 1 files changed, 30 insertions(+), 0 deletions(-) Acked-by: Shawn O. Pearce <spearce@spearce•org> -- Shawn. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 16:01 ` Shawn O. Pearce @ 2009-12-01 16:12 ` Tay Ray Chuan 2009-12-01 16:16 ` Shawn O. Pearce 2009-12-01 16:51 ` Martin Storsjö 1 sibling, 1 reply; 28+ messages in thread From: Tay Ray Chuan @ 2009-12-01 16:12 UTC (permalink / raw) To: Shawn O. Pearce Cc: Martin Storsj?, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Hi, On Wed, Dec 2, 2009 at 12:01 AM, Shawn O. Pearce <spearce@spearce•org> wrote: > In theory, since the cURL session stays active, we would have > received the 401 authentication error during the initial > "GET $GIT_DIR/info/refs?service=git-$service" request, and the subsequent > "POST $GIT_DIR/git-$service" requests would automatically include the > authentication data. > > That's theory. Reality doesn't always agree with my theories. :-) that's because the curl session where the 401 was received (and thus successful authentication takes place) is closed. I sent out a patch series recently which contains a patch to maintain at least one curl session throughout a http session (from http_init() to http_cleanup()), you can see this here: http://www.spinics.net/lists/git/msg118190.html -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 16:12 ` Tay Ray Chuan @ 2009-12-01 16:16 ` Shawn O. Pearce 0 siblings, 0 replies; 28+ messages in thread From: Shawn O. Pearce @ 2009-12-01 16:16 UTC (permalink / raw) To: Tay Ray Chuan Cc: Martin Storsj?, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Tay Ray Chuan <rctay89@gmail•com> wrote: > On Wed, Dec 2, 2009 at 12:01 AM, Shawn O. Pearce <spearce@spearce•org> wrote: > > In theory, since the cURL session stays active, we would have > > received the 401 authentication error during the initial > > "GET $GIT_DIR/info/refs?service=git-$service" request, and the subsequent > > "POST $GIT_DIR/git-$service" requests would automatically include the > > authentication data. > > > > That's theory. ?Reality doesn't always agree with my theories. ?:-) > > that's because the curl session where the 401 was received (and thus > successful authentication takes place) is closed. > > I sent out a patch series recently which contains a patch to maintain > at least one curl session throughout a http session (from http_init() > to http_cleanup()), you can see this here: > > http://www.spinics.net/lists/git/msg118190.html Right, this patch looked sane to me. It didn't touch code I recently have touched myself, so I didn't bother to ACK, but if it helps, Acked-by: Shawn O. Pearce <spearce@spearce•org>. -- Shawn. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 16:01 ` Shawn O. Pearce 2009-12-01 16:12 ` Tay Ray Chuan @ 2009-12-01 16:51 ` Martin Storsjö 1 sibling, 0 replies; 28+ messages in thread From: Martin Storsjö @ 2009-12-01 16:51 UTC (permalink / raw) To: Shawn O. Pearce Cc: Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Tue, 1 Dec 2009, Shawn O. Pearce wrote: > Martin Storsj? <martin@martin•st> wrote: > > When using multi-pass authentication methods, the curl library may need > > to rewind the read buffers used for providing data to HTTP POST, if data > > has been output before a 401 error is received. > > In theory, since the cURL session stays active, we would have > received the 401 authentication error during the initial > "GET $GIT_DIR/info/refs?service=git-$service" request, and the subsequent > "POST $GIT_DIR/git-$service" requests would automatically include the > authentication data. > > That's theory. Reality doesn't always agree with my theories. :-) As Tay said - his "maintain curl sessions" patch should make this redundant in most cases. But in case request pattern gets changed or if the curl session for some other reason isn't able to authenticate on the first try, this is a quite non-intrusive way of ensuring that these requests can be restarted. // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 10:33 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer Martin Storsjö 2009-12-01 16:01 ` Shawn O. Pearce @ 2009-12-01 17:49 ` Junio C Hamano 2009-12-02 2:32 ` Tay Ray Chuan 1 sibling, 1 reply; 28+ messages in thread From: Junio C Hamano @ 2009-12-01 17:49 UTC (permalink / raw) To: Martin Storsjö Cc: Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Martin Storsjö <martin@martin•st> writes: > As long as the current rpc read buffer is the first one, we're able to > rewind without need for additional buffering. ... and if the current buffer isn't the first one, what do we do? > +#ifndef NO_CURL_IOCTL > +curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp) > +{ > + struct rpc_state *rpc = clientp; > + > + switch (cmd) { > + case CURLIOCMD_NOP: > + return CURLIOE_OK; > + > + case CURLIOCMD_RESTARTREAD: > + if (rpc->initial_buffer) { > + rpc->pos = 0; > + return CURLIOE_OK; > + } > + fprintf(stderr, "Unable to rewind rpc post data - try increasing http.postBuffer\n"); > + return CURLIOE_FAILRESTART; > + > + default: > + return CURLIOE_UNKNOWNCMD; > + } > +} > +#endif What will this result in? A failed request, then the user increases http.postBuffer, and re-runs the entire command? I am not suggesting the code should do it differently (e.g. retry with a larger buffer without having the user to help it). At least not yet. That is why my first question above was "what do we do?" and not "what should we do?". I am primarily interested in _documenting_ the expected user experience in the failure case, so that people can notice the message, run "git grep" to find the above line and then run "git blame" to find the commit to read its log message to understand what is going on. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-01 17:49 ` Junio C Hamano @ 2009-12-02 2:32 ` Tay Ray Chuan 2009-12-02 7:45 ` Martin Storsjö 0 siblings, 1 reply; 28+ messages in thread From: Tay Ray Chuan @ 2009-12-02 2:32 UTC (permalink / raw) To: Junio C Hamano Cc: Martin Storsjö, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Hi, On Wed, Dec 2, 2009 at 1:49 AM, Junio C Hamano <gitster@pobox•com> wrote: > ... and if the current buffer isn't the first one, what do we do? > [snip] > What will this result in? A failed request, then the user increases > http.postBuffer, and re-runs the entire command? I am not suggesting the > code should do it differently (e.g. retry with a larger buffer without > having the user to help it). At least not yet. That is why my first > question above was "what do we do?" and not "what should we do?". I guess that by "we" you're referring to the "normal" users of git? > I am primarily interested in _documenting_ the expected user experience in > the failure case, so that people can notice the message, run "git grep" to > find the above line and then run "git blame" to find the commit to read > its log message to understand what is going on. Yes, the code will just fail. As you might suspect, the code won't attempt to mitigate the failure by doing anything, and would require intervention on the part of the user. What the user could do to make this work: 1. Turn off multi-pass authentication and just go with Basic. 2. Allow for persistent curl sessions. In theory, we get a 401 the first time when we send a GET for info/refs; subsequently, curl knows what authentication to use, so the POST request *should* take place without the need for rewinding. In theory. 3. Increase http.postBuffer size in the config. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer 2009-12-02 2:32 ` Tay Ray Chuan @ 2009-12-02 7:45 ` Martin Storsjö 0 siblings, 0 replies; 28+ messages in thread From: Martin Storsjö @ 2009-12-02 7:45 UTC (permalink / raw) To: Tay Ray Chuan Cc: Junio C Hamano, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin [-- Attachment #1: Type: TEXT/PLAIN, Size: 2715 bytes --] On Wed, 2 Dec 2009, Tay Ray Chuan wrote: > > What will this result in? A failed request, then the user increases > > http.postBuffer, and re-runs the entire command? I am not suggesting the > > code should do it differently (e.g. retry with a larger buffer without > > having the user to help it). At least not yet. That is why my first > > question above was "what do we do?" and not "what should we do?". > > I guess that by "we" you're referring to the "normal" users of git? > > > I am primarily interested in _documenting_ the expected user experience in > > the failure case, so that people can notice the message, run "git grep" to > > find the above line and then run "git blame" to find the commit to read > > its log message to understand what is going on. > > Yes, the code will just fail. As you might suspect, the code won't > attempt to mitigate the failure by doing anything, and would require > intervention on the part of the user. > > What the user could do to make this work: > > 1. Turn off multi-pass authentication and just go with Basic. > > 2. Allow for persistent curl sessions. In theory, we get a 401 the > first time when we send a GET for info/refs; subsequently, curl knows > what authentication to use, so the POST request *should* take place > without the need for rewinding. In theory. I'd actually put this as number 1 - if this error message pops up for some reason, the first thing would be to find out why reusing the previous curl sessions didn't work. Other options are: - Switch to a HTTP server that handles Expect: 100-continue properly - Try pushing the data in smaller chunks, e.g. if populating a new repo from scratch, don't push the whole history in one single run, or populate through some other mechanism and just do the incremental pushs over HTTP. And possibly: Update curl to a version post 7.19.7, which detects the Expect header set by git and tries to await a response from the server before proceeding. (The problem that would solve is if we start sending and manage to send the whole initial 1 MB buffer before the 401 reply from the server is received. But it doesn't solve the case if the server doesn't understand the Expect header at all.) > 3. Increase http.postBuffer size in the config. As Shawn pointed out, if the whole request would have to be buffered, the needed size may be prohibitively large, so I guess this isn't a good hint to include in the error message after all. But if the request is sensibly sized (e.g. on the order of tens of MBs), this may be a stopgap solution. So, should we change the error message to something a bit more descriptive, and add this discussion into the commit message? // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 10:28 ` [PATCH 0/2] http: allow multi-pass authentication Martin Storsjö 2009-12-01 10:33 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer Martin Storsjö @ 2009-12-01 10:37 ` Martin Storsjö 2009-12-01 16:14 ` Shawn O. Pearce 1 sibling, 1 reply; 28+ messages in thread From: Martin Storsjö @ 2009-12-01 10:37 UTC (permalink / raw) To: Tay Ray Chuan Cc: git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin When using multi-pass authentication methods, the curl library may need to rewind the read buffers used for providing data to HTTP POST, if data has been output before a 401 error is received. This solution buffers all data read by the curl library, in order to allow it to rewind the reading buffer at any time later. If communicating with a HTTP server that doesn't support the Expect: 100-continue headers, all HTTP POST data will be sent before the server replies with the 401 error containing the authentication challenge. The buffering is enabled only if the rpc_service function allocates a buffer - this should perhaps be limited to the cases where http.authAny is enabled. Signed-off-by: Martin Storsjo <martin@martin•st> --- remote-curl.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 53 insertions(+), 0 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index a331bae..c1c5ccd 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -286,6 +286,10 @@ struct rpc_state { size_t alloc; size_t len; size_t pos; + char *rewind_buf; + size_t rewind_buf_size; + size_t rewind_buf_write_pos; + size_t rewind_buf_read_pos; int in; int out; struct strbuf result; @@ -299,12 +303,28 @@ static size_t rpc_out(void *ptr, size_t eltsize, struct rpc_state *rpc = buffer_; size_t avail = rpc->len - rpc->pos; + if (rpc->rewind_buf && rpc->rewind_buf_read_pos < rpc->rewind_buf_write_pos) { + avail = rpc->rewind_buf_write_pos - rpc->rewind_buf_read_pos; + if (max < avail) + avail = max; + memcpy(ptr, rpc->rewind_buf + rpc->rewind_buf_read_pos, avail); + rpc->rewind_buf_read_pos += avail; + return avail; + } + if (!avail) { avail = packet_read_line(rpc->out, rpc->buf, rpc->alloc); if (!avail) return 0; rpc->pos = 0; rpc->len = avail; + + if (rpc->rewind_buf) { + ALLOC_GROW(rpc->rewind_buf, rpc->rewind_buf_write_pos + avail, rpc->rewind_buf_size); + memcpy(rpc->rewind_buf + rpc->rewind_buf_write_pos, rpc->buf, avail); + rpc->rewind_buf_write_pos += avail; + rpc->rewind_buf_read_pos += avail; + } } if (max < avail) @@ -314,6 +334,26 @@ static size_t rpc_out(void *ptr, size_t eltsize, return avail; } +#ifndef NO_CURL_IOCTL +curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp) +{ + struct rpc_state *rpc = clientp; + + switch (cmd) { + case CURLIOCMD_NOP: + return CURLIOE_OK; + + case CURLIOCMD_RESTARTREAD: + rpc->rewind_buf_read_pos = 0; + rpc->pos = rpc->len; + return CURLIOE_OK; + + default: + return CURLIOE_UNKNOWNCMD; + } +} +#endif + static size_t rpc_in(const void *ptr, size_t eltsize, size_t nmemb, void *buffer_) { @@ -370,8 +410,18 @@ static int post_rpc(struct rpc_state *rpc) */ headers = curl_slist_append(headers, "Expect: 100-continue"); headers = curl_slist_append(headers, "Transfer-Encoding: chunked"); + if (rpc->rewind_buf) { + ALLOC_GROW(rpc->rewind_buf, rpc->rewind_buf_write_pos + rpc->len, rpc->rewind_buf_size); + memcpy(rpc->rewind_buf, rpc->buf, rpc->len); + rpc->rewind_buf_read_pos = rpc->len; + rpc->rewind_buf_write_pos = rpc->len; + } curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out); curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc); +#ifndef NO_CURL_IOCTL + curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl); + curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc); +#endif if (options.verbosity > 1) { fprintf(stderr, "POST %s (chunked)\n", rpc->service_name); fflush(stderr); @@ -472,6 +522,8 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads) rpc->buf = xmalloc(rpc->alloc); rpc->in = client.in; rpc->out = client.out; + rpc->rewind_buf_size = 100; + rpc->rewind_buf = xmalloc(rpc->rewind_buf_size); strbuf_init(&rpc->result, 0); strbuf_addf(&buf, "%s/%s", url, svc); @@ -503,6 +555,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads) free(rpc->hdr_content_type); free(rpc->hdr_accept); free(rpc->buf); + free(rpc->rewind_buf); strbuf_release(&buf); return err; } -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 10:37 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time Martin Storsjö @ 2009-12-01 16:14 ` Shawn O. Pearce 2009-12-01 16:59 ` Martin Storsjö 2009-12-01 18:18 ` Daniel Stenberg 0 siblings, 2 replies; 28+ messages in thread From: Shawn O. Pearce @ 2009-12-01 16:14 UTC (permalink / raw) To: Martin Storsj? Cc: Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Martin Storsj? <martin@martin•st> wrote: > When using multi-pass authentication methods, the curl library may > need to rewind the read buffers used for providing data to HTTP POST, > if data has been output before a 401 error is received. > > This solution buffers all data read by the curl library, in order to allow > it to rewind the reading buffer at any time later. NAK. In the case of git-upload-pack requests, we should fit into 1 MiB almost all of the time, and thus not need to grow the http.postBuffer to support a rewind. The state data plus current have list isn't all that large. A 1 MiB request means we have over 20,900 commits in common with the remote and still haven't been able to find a sufficient cut point. Or the remote has 20,000 active, unrelated branches we are trying to fetch. Either way, this is a really sick and twisted situation. In the case of git-receive-pack requests, we might be uploading an entire project to an empty repository on the remote side. This could be 8 GiB worth of data if the project was something huge like KDE. We can't assume that we should malloc 8 GiB of memory to buffer the payload. The *correct* way to support an arbitrary rewind is to modify the outgoing channel from remote-curl to its protocol engine (client.in within the rpc_service method) to somehow request the protocol engine (aka git-send-pack or git-fetch-pack) to stop and regenerate the current request. Another approach would be to modify http-backend (and the protocol) to support an "auth ping" request prior to spooling out the entire payload if its more than an http.postBuffer size. Basically we do what the "Expect: 100-continue" protocol is supposed to do, but in the application layer rather than the HTTP/1.1 layer, so our CGI actually gets invoked. This unfortunately still relies on the underlying libcurl to not discard the authentication data after that initial "auth ping". But to be honest, I think that is a reasonable expectation. The #@!*@!* library should be able to generate two requests back-to-back to the same URL without needing to rewind the 2nd request. -- Shawn. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 16:14 ` Shawn O. Pearce @ 2009-12-01 16:59 ` Martin Storsjö 2009-12-02 3:15 ` Tay Ray Chuan 2009-12-01 18:18 ` Daniel Stenberg 1 sibling, 1 reply; 28+ messages in thread From: Martin Storsjö @ 2009-12-01 16:59 UTC (permalink / raw) To: Shawn O. Pearce Cc: Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Tue, 1 Dec 2009, Shawn O. Pearce wrote: > In the case of git-receive-pack requests, we might be uploading an > entire project to an empty repository on the remote side. This could > be 8 GiB worth of data if the project was something huge like KDE. > We can't assume that we should malloc 8 GiB of memory to buffer > the payload. True, fair enough. This was mostly a proof of concept of how this could be implemented, but with these comments for you, it's clear that this isn't a feasible solution at all. There's no acute need for it either. > The *correct* way to support an arbitrary rewind is to modify the > outgoing channel from remote-curl to its protocol engine (client.in > within the rpc_service method) to somehow request the protocol engine > (aka git-send-pack or git-fetch-pack) to stop and regenerate the > current request. That's a good idea! > Another approach would be to modify http-backend (and the protocol) > to support an "auth ping" request prior to spooling out the entire > payload if its more than an http.postBuffer size. Basically we > do what the "Expect: 100-continue" protocol is supposed to do, > but in the application layer rather than the HTTP/1.1 layer, so > our CGI actually gets invoked. That's also quite a good idea, especially if it would be done in a way so that it's certain that the same curl session will be reused, instead of getting a potentially new curl session when using get_active_slot(). > This unfortunately still relies on the underlying libcurl to not > discard the authentication data after that initial "auth ping". > But to be honest, I think that is a reasonable expectation. The > #@!*@!* library should be able to generate two requests back-to-back > to the same URL without needing to rewind the 2nd request. Yeah, as long as the same curl session is preserved, this should be no problem. // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 16:59 ` Martin Storsjö @ 2009-12-02 3:15 ` Tay Ray Chuan 0 siblings, 0 replies; 28+ messages in thread From: Tay Ray Chuan @ 2009-12-02 3:15 UTC (permalink / raw) To: Martin Storsjö Cc: Shawn O. Pearce, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Hi, On Wed, Dec 2, 2009 at 12:59 AM, Martin Storsjö <martin@martin•st> wrote: > On Tue, 1 Dec 2009, Shawn O. Pearce wrote: >> The *correct* way to support an arbitrary rewind is to modify the >> outgoing channel from remote-curl to its protocol engine (client.in >> within the rpc_service method) to somehow request the protocol engine >> (aka git-send-pack or git-fetch-pack) to stop and regenerate the >> current request. > > That's a good idea! > >> Another approach would be to modify http-backend (and the protocol) >> to support an "auth ping" request prior to spooling out the entire >> payload if its more than an http.postBuffer size. Basically we >> do what the "Expect: 100-continue" protocol is supposed to do, >> but in the application layer rather than the HTTP/1.1 layer, so >> our CGI actually gets invoked. > > That's also quite a good idea, especially if it would be done in a way so > that it's certain that the same curl session will be reused, instead of > getting a potentially new curl session when using get_active_slot(). I think restarting the read by killing the protocol engine/client and starting again would be the easier of the two. Not just that, it would be neater than storing everything that the protocol engine has spewed out, like Martin's patch does. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 16:14 ` Shawn O. Pearce 2009-12-01 16:59 ` Martin Storsjö @ 2009-12-01 18:18 ` Daniel Stenberg 2009-12-02 2:03 ` Tay Ray Chuan 1 sibling, 1 reply; 28+ messages in thread From: Daniel Stenberg @ 2009-12-01 18:18 UTC (permalink / raw) To: Shawn O. Pearce Cc: Martin Storsj?, Tay Ray Chuan, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Tue, 1 Dec 2009, Shawn O. Pearce wrote: > The #@!*@!* library should be able to generate two requests back-to-back to > the same URL without needing to rewind the 2nd request. If '#@!*@!*' is your pattern for matching libcurl or curl, then sure libcurl certainly has no problem at all to send as many requests you like back-to-back. The rewinding business is only really necessary for multipass authentication when Expect: 100-continue doesn't work (and thus libcurl has started to send data that the server will discard and thus is needed to get sent again). And that's not something you can blame "the #@!*@!* library" for, but rather your server end and/or how HTTP is defined to work. -- / daniel.haxx.se ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-01 18:18 ` Daniel Stenberg @ 2009-12-02 2:03 ` Tay Ray Chuan 2009-12-02 9:19 ` Daniel Stenberg 0 siblings, 1 reply; 28+ messages in thread From: Tay Ray Chuan @ 2009-12-02 2:03 UTC (permalink / raw) To: Daniel Stenberg Cc: Shawn O. Pearce, Martin Storsj?, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin Hi, On Wed, Dec 2, 2009 at 2:18 AM, Daniel Stenberg <daniel@haxx•se> wrote: > If '#@!*@!*' is your pattern for matching libcurl or curl, then sure libcurl > certainly has no problem at all to send as many requests you like > back-to-back. I have a feeling Shawn's referring to the git http library on top of that. ;) > The rewinding business is only really necessary for multipass authentication > when Expect: 100-continue doesn't work (and thus libcurl has started to send > data that the server will discard and thus is needed to get sent again). And > that's not something you can blame "the #@!*@!* library" for, but rather > your server end and/or how HTTP is defined to work. According to Martin, Expect: 100-continue is not working due to libcurl. I quote him: Date: Tue, 1 Dec 2009 12:28:26 +0200 (EET) Subject: Re: [PATCH 0/2] http: allow multi-pass authentication On Tue, Dec 1, 2009 at 6:28 PM, Martin Storsjö <martin@martin•st> wrote: > Normally, libcurl should add the Expect: 100-continue header > automatically, but for some reason > (http://article.gmane.org/gmane.comp.web.curl.library/25992) it doesn't, > so that's probably why we're manually adding that header in > remote-curl.c:371 at the moment. libcurl doesn't detect this at the moment > (http://article.gmane.org/gmane.comp.web.curl.library/25991) so it won't > wait for the 100 continue response before starting to send the body data. But, again, don't read my blaming of libcurl for this 100 business as a criticism of curl. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-02 2:03 ` Tay Ray Chuan @ 2009-12-02 9:19 ` Daniel Stenberg 2009-12-02 9:32 ` Martin Storsjö 0 siblings, 1 reply; 28+ messages in thread From: Daniel Stenberg @ 2009-12-02 9:19 UTC (permalink / raw) To: Tay Ray Chuan Cc: Shawn O. Pearce, Martin Storsj?, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Wed, 2 Dec 2009, Tay Ray Chuan wrote: > According to Martin, Expect: 100-continue is not working due to libcurl. Right, that is/was a bug in how libcurl behaves when the application itself has set the "Expect: 100-continue" header. Martin has provided a fix for that for the next libcurl version though, but that won't make a lot of existing users happy. Thinking about this particular problem, what is the motivation for git to forcily add that header in the first place? I mean, libcurl does add the header by itself when it thinks it is necessary and then it handles it correctly. I'm just suggesting (and speculating widely since I don't know git internals) that a possible way to work around this particular bug may be to reconsider how git adds the Expect header. It's just an idea. Please ignore it if it is totally crazy. -- / daniel.haxx.se ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-02 9:19 ` Daniel Stenberg @ 2009-12-02 9:32 ` Martin Storsjö 2009-12-02 10:04 ` Daniel Stenberg 0 siblings, 1 reply; 28+ messages in thread From: Martin Storsjö @ 2009-12-02 9:32 UTC (permalink / raw) To: Daniel Stenberg Cc: Tay Ray Chuan, Shawn O. Pearce, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin On Wed, 2 Dec 2009, Daniel Stenberg wrote: > On Wed, 2 Dec 2009, Tay Ray Chuan wrote: > > > According to Martin, Expect: 100-continue is not working due to libcurl. > > Right, that is/was a bug in how libcurl behaves when the application itself > has set the "Expect: 100-continue" header. Martin has provided a fix for that > for the next libcurl version though, but that won't make a lot of existing > users happy. > > Thinking about this particular problem, what is the motivation for git to > forcily add that header in the first place? I mean, libcurl does add the > header by itself when it thinks it is necessary and then it handles it > correctly. As far as I saw, the reason for it being manually added is that curl actually didn't add it automatically in that case. That was the reason for the second patch/rfc thread that I sent to curl-library (where postsize == 0, as in unknown, didn't trigger the addition of any Expect header). // Martin ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time 2009-12-02 9:32 ` Martin Storsjö @ 2009-12-02 10:04 ` Daniel Stenberg 0 siblings, 0 replies; 28+ messages in thread From: Daniel Stenberg @ 2009-12-02 10:04 UTC (permalink / raw) To: Martin Storsjö Cc: Tay Ray Chuan, Shawn O. Pearce, git, Nicholas Miell, gsky51, Clemens Buchacher, Mark Lodato, Johannes Schindelin [-- Attachment #1: Type: TEXT/PLAIN, Size: 692 bytes --] On Wed, 2 Dec 2009, Martin Storsjö wrote: >> Thinking about this particular problem, what is the motivation for git to >> forcily add that header in the first place? I mean, libcurl does add the >> header by itself when it thinks it is necessary and then it handles it >> correctly. > > As far as I saw, the reason for it being manually added is that curl > actually didn't add it automatically in that case. That was the reason for > the second patch/rfc thread that I sent to curl-library (where postsize == > 0, as in unknown, didn't trigger the addition of any Expect header). Ah right, thanks for the clarification. An unfortunate combination then... :-( -- / daniel.haxx.se ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] Use the best HTTP authentication method supported by the server 2009-10-02 17:28 HTTP NTLM Authentication gsky 2009-04-14 21:56 ` [PATCH v2] Add an option for using any HTTP authentication scheme, not only basic Martin Storsjö @ 2009-10-02 19:04 ` Nicholas Miell 1 sibling, 0 replies; 28+ messages in thread From: Nicholas Miell @ 2009-10-02 19:04 UTC (permalink / raw) To: gsky51; +Cc: git, Nicholas Miell Currently, libcurl is limited to using HTTP Basic authentication if a username and password are specified. HTTP Basic passes the username and password to the server as plaintext, which is obviously suboptimal. Furthermore, some servers are configured to require a more secure authentication method (e.g. Digest or NTLM), which means that git can't talk to them at all. This is easily solved by telling libcurl to use any HTTP authentication method it pleases. I leave the decision as to whether HTTP Basic (i.e. completely insecure) should be allowed at all to somebody else. This can be easily changed in the future by using CURLAUTH_ANYSAFE instead of CURLAUTH_ANY. Signed-off-by: Nicholas Miell <nmiell@gmail•com> --- http.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) This passes make test; but I haven't actually tested it on a real HTTP server. diff --git a/http.c b/http.c index 23b2a19..1937b45 100644 --- a/http.c +++ b/http.c @@ -185,6 +185,7 @@ static void init_curl_http_auth(CURL *result) if (!user_pass) user_pass = xstrdup(getpass("Password: ")); strbuf_addf(&up, "%s:%s", user_name, user_pass); + curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY); curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL)); } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 28+ messages in thread
end of thread, other threads:[~2009-12-02 10:04 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-10-02 17:28 HTTP NTLM Authentication gsky 2009-04-14 21:56 ` [PATCH v2] Add an option for using any HTTP authentication scheme, not only basic Martin Storsjö 2009-04-14 20:52 ` [PATCH] " Martin Storsjö 2009-04-14 21:08 ` Johannes Schindelin 2009-04-14 21:15 ` Martin Storsjö 2009-04-14 21:42 ` Johannes Schindelin 2009-11-27 15:41 ` [PATCH 0/2] http: allow multi-pass authentication Tay Ray Chuan 2009-11-27 15:42 ` [PATCH 1/2] http: maintain curl sessions Tay Ray Chuan 2009-11-27 15:43 ` [PATCH 2/2] Add an option for using any HTTP authentication scheme, not only basic Tay Ray Chuan 2009-12-01 10:28 ` [PATCH 0/2] http: allow multi-pass authentication Martin Storsjö 2009-12-01 10:33 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer Martin Storsjö 2009-12-01 16:01 ` Shawn O. Pearce 2009-12-01 16:12 ` Tay Ray Chuan 2009-12-01 16:16 ` Shawn O. Pearce 2009-12-01 16:51 ` Martin Storsjö 2009-12-01 17:49 ` Junio C Hamano 2009-12-02 2:32 ` Tay Ray Chuan 2009-12-02 7:45 ` Martin Storsjö 2009-12-01 10:37 ` [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time Martin Storsjö 2009-12-01 16:14 ` Shawn O. Pearce 2009-12-01 16:59 ` Martin Storsjö 2009-12-02 3:15 ` Tay Ray Chuan 2009-12-01 18:18 ` Daniel Stenberg 2009-12-02 2:03 ` Tay Ray Chuan 2009-12-02 9:19 ` Daniel Stenberg 2009-12-02 9:32 ` Martin Storsjö 2009-12-02 10:04 ` Daniel Stenberg 2009-10-02 19:04 ` [PATCH] Use the best HTTP authentication method supported by the server Nicholas Miell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox