{"thread":{"id":"18873","subject":"[PATCH v2] Add an option for using any HTTP authentication scheme, not only basic","startedAt":"2009-04-14T21:56:23Z","lastAt":"2009-12-02T10:04:17Z","messageCount":22,"participants":["Martin Storsjö","Tay Ray Chuan","Shawn O. Pearce","Junio C Hamano","Daniel Stenberg"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"111329","messageId":"Pine.LNX.4.64.0904150054470.7479@localhost.localdomain","threadId":"18873","inReplyTo":null,"subject":"[PATCH v2] Add an option for using any HTTP authentication scheme, not only basic","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-04-14T21:56:23Z","receivedAt":"2009-04-14T21:56:23Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"This adds the configuration option http.authAny (overridable with\nthe environment variable GIT_HTTP_AUTH_ANY), for instructing curl\nto allow any HTTP authentication scheme, not only basic (which\nsends the password in plaintext).\n\nWhen this is enabled, curl has to do double requests most of the time,\nin order to discover which HTTP authentication method to use, which\nlowers the performance slightly. Therefore this isn't enabled by default.\n\nOne example of another authentication scheme to use is digest, which\ndoesn't send the password in plaintext, but uses a challenge-response\nmechanism instead. Using digest authentication in practice requires\nat least curl 7.18.1, due to bugs in the digest handling in earlier\nversions of curl.\n\nSigned-off-by: Martin Storsjo <martin@martin.st>\n---\n\nRepost with the curl version checked in only one place.\n\n Documentation/config.txt |    7 +++++++\n http.c                   |   22 ++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f3ebd2f..1515d77 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1011,6 +1011,13 @@ http.noEPSV::\n \tsupport EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV'\n \tenvironment variable. Default is false (curl will use EPSV).\n \n+http.authAny::\n+\tAllow any HTTP authentication method, not only basic. Enabling\n+\tthis lowers the performance slightly, by having to do requests\n+\twithout any authentication to discover the authentication method\n+\tto use. Can be overridden by the 'GIT_HTTP_AUTH_ANY'\n+\tenvironment variable. Default is false.\n+\n i18n.commitEncoding::\n \tCharacter encoding the commit messages are stored in; git itself\n \tdoes not care per se, but this information is necessary e.g. when\ndiff --git a/http.c b/http.c\nindex 2e3d649..49b8441 100644\n--- a/http.c\n+++ b/http.c\n@@ -3,6 +3,10 @@\n int data_received;\n int active_requests;\n \n+#if LIBCURL_VERSION_NUM >= 0x070a06\n+#define LIBCURL_CAN_HANDLE_AUTH_ANY\n+#endif\n+\n #ifdef USE_CURL_MULTI\n static int max_requests = -1;\n static CURLM *curlm;\n@@ -26,6 +30,9 @@ static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n static char *user_name, *user_pass;\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+static int curl_http_auth_any = 0;\n+#endif\n \n static struct curl_slist *pragma_header;\n \n@@ -150,6 +157,12 @@ static int http_options(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(\"http.proxy\", var))\n \t\treturn git_config_string(&curl_http_proxy, var, value);\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (!strcmp(\"http.authany\", var)) {\n+\t\tcurl_http_auth_any = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+#endif\n \n \t/* Fall back on the default ones */\n \treturn git_default_config(var, value, cb);\n@@ -184,6 +197,10 @@ static CURL *get_curl_handle(void)\n #if LIBCURL_VERSION_NUM >= 0x070907\n \tcurl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);\n #endif\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (curl_http_auth_any)\n+\t\tcurl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);\n+#endif\n \n \tinit_curl_http_auth(result);\n \n@@ -329,6 +346,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n \n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (getenv(\"GIT_HTTP_AUTH_ANY\"))\n+\t\tcurl_http_auth_any = 1;\n+#endif\n+\n \tif (remote && remote->url && remote->url[0])\n \t\thttp_auth_init(remote->url[0]);\n \n-- \n1.6.0.2\n"},{"id":"128571","messageId":"20091127234110.7b7e9993.rctay89@gmail.com","threadId":"18873","inReplyTo":"Pine.LNX.4.64.0904150054470.7479@localhost.localdomain","subject":"[PATCH 0/2] http: allow multi-pass authentication","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-11-27T15:41:10Z","receivedAt":"2009-11-27T15:41:10Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This patch series applies on top of master. It enables fetching and\npushing over http with the most suitable authentication scheme chosen\nby curl when the http.authAny or GIT_HTTP_AUTH_ANY is set.\n\nAuthorization headers can also be preserved across requests, with at\nleast 1 curl session being preserved by default. This is especially\nuseful for the smart http protocol, where it is hard to rewind and re-\nsend a request.\n\nNicholas, Martin's patch should lead to similar functionality as your\npatch (dated Oct 3rd) would. However, unlike your patch,\nCURLOPT_HTTPAUTH is set even if the user name is not specified\nexplicitly in the remote url, since it's conditional on\nhttp.c::user_name being set.\n\nI've tested this with Digest, and I believe this should work with NTLM\ntoo.\n\ngsky, could you try this out with NTLM?\n\n=?ISO-8859-15?Q?Martin_Storsj=F6?= (1):\n  Add an option for using any HTTP authentication scheme, not only\n    basic\n\nTay Ray Chuan (1):\n  http: maintain curl sessions\n\n Documentation/config.txt |   13 +++++++++++++\n http.c                   |   41 +++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 52 insertions(+), 2 deletions(-)\n\n--\nCheers,\nRay Chuan\n"},{"id":"128572","messageId":"20091127234226.8b158336.rctay89@gmail.com","threadId":"18873","inReplyTo":"20091127234110.7b7e9993.rctay89@gmail.com","subject":"[PATCH 1/2] http: maintain curl sessions","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-11-27T15:42:26Z","receivedAt":"2009-11-27T15:42:26Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Allow curl sessions to be kept alive (ie. not ended with\ncurl_easy_cleanup()) even after the request is completed, the number of\nwhich is determined by the configuration setting http.minSessions.\n\nAdd a count for curl sessions, and update it, across slots, when\nstarting and ending curl sessions.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Documentation/config.txt |    6 ++++++\n http.c                   |   19 +++++++++++++++++--\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a8e0876..b77d66d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1132,6 +1132,12 @@ http.maxRequests::\n \tHow many HTTP requests to launch in parallel. Can be overridden\n \tby the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5.\n\n+http.minSessions::\n+\tThe number of curl sessions (counted across slots) to be kept across\n+\trequests. They will not be ended with curl_easy_cleanup() until\n+\thttp_cleanup() is invoked. If USE_CURL_MULTI is not defined, this\n+\tvalue will be capped at 1. Defaults to 1.\n+\n http.postBuffer::\n \tMaximum size in bytes of the buffer used by smart HTTP\n \ttransports when POSTing data to the remote system.\ndiff --git a/http.c b/http.c\nindex ed6414a..fb0a97b 100644\n--- a/http.c\n+++ b/http.c\n@@ -7,6 +7,8 @@ int active_requests;\n int http_is_verbose;\n size_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n\n+static int min_curl_sessions = 1;\n+static int curl_session_count;\n #ifdef USE_CURL_MULTI\n static int max_requests = -1;\n static CURLM *curlm;\n@@ -152,6 +154,14 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\t\tssl_cert_password_required = 1;\n \t\treturn 0;\n \t}\n+\tif (!strcmp(\"http.minsessions\", var)) {\n+\t\tmin_curl_sessions = git_config_int(var, value);\n+#ifndef USE_CURL_MULTI\n+\t\tif (min_curl_sessions > 1)\n+\t\t\tmin_curl_sessions = 1;\n+#endif\n+\t\treturn 0;\n+\t}\n #ifdef USE_CURL_MULTI\n \tif (!strcmp(\"http.maxrequests\", var)) {\n \t\tmax_requests = git_config_int(var, value);\n@@ -372,6 +382,7 @@ void http_init(struct remote *remote)\n \tif (curl_ssl_verify == -1)\n \t\tcurl_ssl_verify = 1;\n\n+\tcurl_session_count = 0;\n #ifdef USE_CURL_MULTI\n \tif (max_requests < 1)\n \t\tmax_requests = DEFAULT_MAX_REQUESTS;\n@@ -480,6 +491,7 @@ struct active_request_slot *get_active_slot(void)\n #else\n \t\tslot->curl = curl_easy_duphandle(curl_default);\n #endif\n+\t\tcurl_session_count++;\n \t}\n\n \tactive_requests++;\n@@ -558,9 +570,11 @@ void fill_active_slots(void)\n \t}\n\n \twhile (slot != NULL) {\n-\t\tif (!slot->in_use && slot->curl != NULL) {\n+\t\tif (!slot->in_use && slot->curl != NULL\n+\t\t\t&& curl_session_count > min_curl_sessions) {\n \t\t\tcurl_easy_cleanup(slot->curl);\n \t\t\tslot->curl = NULL;\n+\t\t\tcurl_session_count--;\n \t\t}\n \t\tslot = slot->next;\n \t}\n@@ -633,12 +647,13 @@ static void closedown_active_slot(struct active_request_slot *slot)\n void release_active_slot(struct active_request_slot *slot)\n {\n \tclosedown_active_slot(slot);\n-\tif (slot->curl) {\n+\tif (slot->curl && curl_session_count > min_curl_sessions) {\n #ifdef USE_CURL_MULTI\n \t\tcurl_multi_remove_handle(curlm, slot->curl);\n #endif\n \t\tcurl_easy_cleanup(slot->curl);\n \t\tslot->curl = NULL;\n+\t\tcurl_session_count--;\n \t}\n #ifdef USE_CURL_MULTI\n \tfill_active_slots();\n--\n1.6.4.4\n"},{"id":"128573","messageId":"20091127234308.81046118.rctay89@gmail.com","threadId":"18873","inReplyTo":"20091127234226.8b158336.rctay89@gmail.com","subject":"[PATCH 2/2] Add an option for using any HTTP authentication scheme, not only basic","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-11-27T15:43:08Z","receivedAt":"2009-11-27T15:43:08Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From:\t=?ISO-8859-15?Q?Martin_Storsj=F6?= <martin@martin.st>\n\nThis adds the configuration option http.authAny (overridable with\nthe environment variable GIT_HTTP_AUTH_ANY), for instructing curl\nto allow any HTTP authentication scheme, not only basic (which\nsends the password in plaintext).\n\nWhen this is enabled, curl has to do double requests most of the time,\nin order to discover which HTTP authentication method to use, which\nlowers the performance slightly. Therefore this isn't enabled by default.\n\nOne example of another authentication scheme to use is digest, which\ndoesn't send the password in plaintext, but uses a challenge-response\nmechanism instead. Using digest authentication in practice requires\nat least curl 7.18.1, due to bugs in the digest handling in earlier\nversions of curl.\n\nSigned-off-by: Martin Storsjo <martin@martin.st>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\n  Martin, I've not made any changes, except to make it apply cleanly.\n\n Documentation/config.txt |    7 +++++++\n http.c                   |   22 ++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b77d66d..a54ede3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1158,6 +1158,13 @@ http.noEPSV::\n \tsupport EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV'\n \tenvironment variable. Default is false (curl will use EPSV).\n\n+http.authAny::\n+\tAllow any HTTP authentication method, not only basic. Enabling\n+\tthis lowers the performance slightly, by having to do requests\n+\twithout any authentication to discover the authentication method\n+\tto use. Can be overridden by the 'GIT_HTTP_AUTH_ANY'\n+\tenvironment variable. Default is false.\n+\n i18n.commitEncoding::\n \tCharacter encoding the commit messages are stored in; git itself\n \tdoes not care per se, but this information is necessary e.g. when\ndiff --git a/http.c b/http.c\nindex fb0a97b..aeb69b3 100644\n--- a/http.c\n+++ b/http.c\n@@ -7,6 +7,10 @@ int active_requests;\n int http_is_verbose;\n size_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n\n+#if LIBCURL_VERSION_NUM >= 0x070a06\n+#define LIBCURL_CAN_HANDLE_AUTH_ANY\n+#endif\n+\n static int min_curl_sessions = 1;\n static int curl_session_count;\n #ifdef USE_CURL_MULTI\n@@ -36,6 +40,9 @@ static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n static char *user_name, *user_pass;\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+static int curl_http_auth_any = 0;\n+#endif\n\n #if LIBCURL_VERSION_NUM >= 0x071700\n /* Use CURLOPT_KEYPASSWD as is */\n@@ -190,6 +197,12 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\t\thttp_post_buffer = LARGE_PACKET_MAX;\n \t\treturn 0;\n \t}\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (!strcmp(\"http.authany\", var)) {\n+\t\tcurl_http_auth_any = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+#endif\n\n \t/* Fall back on the default ones */\n \treturn git_default_config(var, value, cb);\n@@ -240,6 +253,10 @@ static CURL *get_curl_handle(void)\n #if LIBCURL_VERSION_NUM >= 0x070907\n \tcurl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);\n #endif\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (curl_http_auth_any)\n+\t\tcurl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);\n+#endif\n\n \tinit_curl_http_auth(result);\n\n@@ -391,6 +408,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\tif (getenv(\"GIT_HTTP_AUTH_ANY\"))\n+\t\tcurl_http_auth_any = 1;\n+#endif\n+\n \tif (remote && remote->url && remote->url[0]) {\n \t\thttp_auth_init(remote->url[0]);\n \t\tif (!ssl_cert_password_required &&\n--\n1.6.4.4\n"},{"id":"128862","messageId":"alpine.DEB.2.00.0912011208160.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"20091127234110.7b7e9993.rctay89@gmail.com","subject":"Re: [PATCH 0/2] http: allow multi-pass authentication","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-01T10:28:26Z","receivedAt":"2009-12-01T10:28:26Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Fri, 27 Nov 2009, Tay Ray Chuan wrote:\n\n> This patch series applies on top of master. It enables fetching and\n> pushing over http with the most suitable authentication scheme chosen\n> by curl when the http.authAny or GIT_HTTP_AUTH_ANY is set.\n\nI also tested this, and things generally seem to work fine.\n\nThanks to the \"maintain curl sessions\" patch, only the first request needs \nto be redone after getting the 401 error containing the authentication \nchallenge, the later ones work fine on the first try. However, \ntheoretically, I guess we can't be certain that the curl session really is \ninitialized for the later requests (we could be given a new fresh curl \nsession for some reason), or the first request could perhaps be a large, \n(currently) non-rewindable POST.\n\n\nAvoiding redoing large POST requests is generally accomplished by adding a \nExpect: 100-continue header, and then waiting for a reply (either 100 \ncontinue or 401 unauthorized) to that header before actually sending the \nPOST body data. If the server doesn't support the Expect header (e.g. \nLighttpd doesn't support it), the client starts sending the POST body \nafter a timeout (1 second in libcurl).\n\n(As a side note, chunked POST requests without a content-length header \nisn't supported by lighttpd at all at the moment, neither in the stable \n1.4 version nor in the new upcoming 1.5 branch.)\n\n\nNormally, libcurl should add the Expect: 100-continue header \nautomatically, but for some reason \n(http://article.gmane.org/gmane.comp.web.curl.library/25992) it doesn't, \nso that's probably why we're manually adding that header in \nremote-curl.c:371 at the moment. libcurl doesn't detect this at the moment \n(http://article.gmane.org/gmane.comp.web.curl.library/25991) so it won't \nwait for the 100 continue response before starting to send the body data. \n\nSo, with a server supporting Expect, the 401 error response may come after \nsending a few KB of POST data (corresponding to the roundtrip delay for \nthe server to respond to the header) - if the server doesn't support \nExpect at all, the whole request will be sent and may need to be rewound.\n\nTo clarify - this only happens if the curl authentication isn't \ninitialized yet, for the first request of every curl session. The \n\"maintain curl sessions\" patch makes sure this isn't needed in the normal \ncase.\n\nI've experimented with two solutions to this, which add partial and full \nrewind solutions to the chunked POST requests - I'll send them as \nfollow-ups to this mail.\n\n// Martin\n"},{"id":"128863","messageId":"alpine.DEB.2.00.0912011232450.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011208160.5582@cone.home.martin.st","subject":"[PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-01T10:33:39Z","receivedAt":"2009-12-01T10:33:39Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"When using multi-pass authentication methods, the curl library may need\nto rewind the read buffers used for providing data to HTTP POST, if data\nhas been output before a 401 error is received.\n\nThis is needed only when the first request (when the multi-pass\nauthentication method isn't initialized and hasn't received its challenge\nyet) for a certain curl session is a chunked HTTP POST.\n\nAs long as the current rpc read buffer is the first one, we're able to\nrewind without need for additional buffering.\n\nThe curl library currently starts sending data without waiting for a\nresponse to the Expect: 100-continue header, due to a bug in curl that\nexists up to curl version 7.19.7.\n\nIf the HTTP server doesn't handle Expect: 100-continue headers properly\n(e.g. Lighttpd), the library has to start sending data without knowing\nif the request will be successfully authenticated. In this case, this\nrewinding solution is not sufficient - the whole request will be sent\nbefore the 401 error is received.\n\nSigned-off-by: Martin Storsjo <martin@martin.st>\n---\n\nThe curl bug is yet unconfirmed upstream, discussed here:\nhttp://article.gmane.org/gmane.comp.web.curl.library/25991\n\n remote-curl.c |   30 ++++++++++++++++++++++++++++++\n 1 files changed, 30 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex a331bae..28b2a31 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -290,6 +290,7 @@ struct rpc_state {\n \tint out;\n \tstruct strbuf result;\n \tunsigned gzip_request : 1;\n+\tunsigned initial_buffer : 1;\n };\n \n static size_t rpc_out(void *ptr, size_t eltsize,\n@@ -300,6 +301,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,\n \tsize_t avail = rpc->len - rpc->pos;\n \n \tif (!avail) {\n+\t\trpc->initial_buffer = 0;\n \t\tavail = packet_read_line(rpc->out, rpc->buf, rpc->alloc);\n \t\tif (!avail)\n \t\t\treturn 0;\n@@ -314,6 +316,29 @@ static size_t rpc_out(void *ptr, size_t eltsize,\n \treturn avail;\n }\n \n+#ifndef NO_CURL_IOCTL\n+curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)\n+{\n+\tstruct rpc_state *rpc = clientp;\n+\n+\tswitch (cmd) {\n+\tcase CURLIOCMD_NOP:\n+\t\treturn CURLIOE_OK;\n+\n+\tcase CURLIOCMD_RESTARTREAD:\n+\t\tif (rpc->initial_buffer) {\n+\t\t\trpc->pos = 0;\n+\t\t\treturn CURLIOE_OK;\n+\t\t}\n+\t\tfprintf(stderr, \"Unable to rewind rpc post data - try increasing http.postBuffer\\n\");\n+\t\treturn CURLIOE_FAILRESTART;\n+\n+\tdefault:\n+\t\treturn CURLIOE_UNKNOWNCMD;\n+\t}\n+}\n+#endif\n+\n static size_t rpc_in(const void *ptr, size_t eltsize,\n \t\tsize_t nmemb, void *buffer_)\n {\n@@ -370,8 +395,13 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t */\n \t\theaders = curl_slist_append(headers, \"Expect: 100-continue\");\n \t\theaders = curl_slist_append(headers, \"Transfer-Encoding: chunked\");\n+\t\trpc->initial_buffer = 1;\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);\n+#ifndef NO_CURL_IOCTL\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);\n+#endif\n \t\tif (options.verbosity > 1) {\n \t\t\tfprintf(stderr, \"POST %s (chunked)\\n\", rpc->service_name);\n \t\t\tfflush(stderr);\n-- \n1.6.4.4\n"},{"id":"128864","messageId":"alpine.DEB.2.00.0912011236360.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011208160.5582@cone.home.martin.st","subject":"[PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-01T10:37:26Z","receivedAt":"2009-12-01T10:37:26Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"When using multi-pass authentication methods, the curl library may\nneed to rewind the read buffers used for providing data to HTTP POST,\nif data has been output before a 401 error is received.\n\nThis solution buffers all data read by the curl library, in order to allow\nit to rewind the reading buffer at any time later.\n\nIf communicating with a HTTP server that doesn't support the\nExpect: 100-continue headers, all HTTP POST data will be sent before\nthe server replies with the 401 error containing the authentication\nchallenge.\n\nThe buffering is enabled only if the rpc_service function allocates a\nbuffer - this should perhaps be limited to the cases where http.authAny\nis enabled.\n\nSigned-off-by: Martin Storsjo <martin@martin.st>\n---\n remote-curl.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 53 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex a331bae..c1c5ccd 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -286,6 +286,10 @@ struct rpc_state {\n \tsize_t alloc;\n \tsize_t len;\n \tsize_t pos;\n+\tchar *rewind_buf;\n+\tsize_t rewind_buf_size;\n+\tsize_t rewind_buf_write_pos;\n+\tsize_t rewind_buf_read_pos;\n \tint in;\n \tint out;\n \tstruct strbuf result;\n@@ -299,12 +303,28 @@ static size_t rpc_out(void *ptr, size_t eltsize,\n \tstruct rpc_state *rpc = buffer_;\n \tsize_t avail = rpc->len - rpc->pos;\n \n+\tif (rpc->rewind_buf && rpc->rewind_buf_read_pos < rpc->rewind_buf_write_pos) {\n+\t\tavail = rpc->rewind_buf_write_pos - rpc->rewind_buf_read_pos;\n+\t\tif (max < avail)\n+\t\t\tavail = max;\n+\t\tmemcpy(ptr, rpc->rewind_buf + rpc->rewind_buf_read_pos, avail);\n+\t\trpc->rewind_buf_read_pos += avail;\n+\t\treturn avail;\n+\t}\n+\n \tif (!avail) {\n \t\tavail = packet_read_line(rpc->out, rpc->buf, rpc->alloc);\n \t\tif (!avail)\n \t\t\treturn 0;\n \t\trpc->pos = 0;\n \t\trpc->len = avail;\n+\n+\t\tif (rpc->rewind_buf) {\n+\t\t\tALLOC_GROW(rpc->rewind_buf, rpc->rewind_buf_write_pos + avail, rpc->rewind_buf_size);\n+\t\t\tmemcpy(rpc->rewind_buf + rpc->rewind_buf_write_pos, rpc->buf, avail);\n+\t\t\trpc->rewind_buf_write_pos += avail;\n+\t\t\trpc->rewind_buf_read_pos += avail;\n+\t\t}\n \t}\n \n \tif (max < avail)\n@@ -314,6 +334,26 @@ static size_t rpc_out(void *ptr, size_t eltsize,\n \treturn avail;\n }\n \n+#ifndef NO_CURL_IOCTL\n+curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)\n+{\n+\tstruct rpc_state *rpc = clientp;\n+\n+\tswitch (cmd) {\n+\tcase CURLIOCMD_NOP:\n+\t\treturn CURLIOE_OK;\n+\n+\tcase CURLIOCMD_RESTARTREAD:\n+\t\trpc->rewind_buf_read_pos = 0;\n+\t\trpc->pos = rpc->len;\n+\t\treturn CURLIOE_OK;\n+\n+\tdefault:\n+\t\treturn CURLIOE_UNKNOWNCMD;\n+\t}\n+}\n+#endif\n+\n static size_t rpc_in(const void *ptr, size_t eltsize,\n \t\tsize_t nmemb, void *buffer_)\n {\n@@ -370,8 +410,18 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t */\n \t\theaders = curl_slist_append(headers, \"Expect: 100-continue\");\n \t\theaders = curl_slist_append(headers, \"Transfer-Encoding: chunked\");\n+\t\tif (rpc->rewind_buf) {\n+\t\t\tALLOC_GROW(rpc->rewind_buf, rpc->rewind_buf_write_pos + rpc->len, rpc->rewind_buf_size);\n+\t\t\tmemcpy(rpc->rewind_buf, rpc->buf, rpc->len);\n+\t\t\trpc->rewind_buf_read_pos = rpc->len;\n+\t\t\trpc->rewind_buf_write_pos = rpc->len;\n+\t\t}\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);\n+#ifndef NO_CURL_IOCTL\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);\n+#endif\n \t\tif (options.verbosity > 1) {\n \t\t\tfprintf(stderr, \"POST %s (chunked)\\n\", rpc->service_name);\n \t\t\tfflush(stderr);\n@@ -472,6 +522,8 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \trpc->buf = xmalloc(rpc->alloc);\n \trpc->in = client.in;\n \trpc->out = client.out;\n+\trpc->rewind_buf_size = 100;\n+\trpc->rewind_buf = xmalloc(rpc->rewind_buf_size);\n \tstrbuf_init(&rpc->result, 0);\n \n \tstrbuf_addf(&buf, \"%s/%s\", url, svc);\n@@ -503,6 +555,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \tfree(rpc->hdr_content_type);\n \tfree(rpc->hdr_accept);\n \tfree(rpc->buf);\n+\tfree(rpc->rewind_buf);\n \tstrbuf_release(&buf);\n \treturn err;\n }\n-- \n1.6.4.4\n"},{"id":"128884","messageId":"20091201160150.GB21299@spearce.org","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011232450.5582@cone.home.martin.st","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T16:01:50Z","receivedAt":"2009-12-01T16:01:50Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Martin Storsj? <martin@martin.st> wrote:\n> When using multi-pass authentication methods, the curl library may need\n> to rewind the read buffers used for providing data to HTTP POST, if data\n> has been output before a 401 error is received.\n\nIn theory, since the cURL session stays active, we would have\nreceived the 401 authentication error during the initial\n\"GET $GIT_DIR/info/refs?service=git-$service\" request, and the subsequent\n\"POST $GIT_DIR/git-$service\" requests would automatically include the\nauthentication data.\n\nThat's theory.  Reality doesn't always agree with my theories.  :-)\n \n>  remote-curl.c |   30 ++++++++++++++++++++++++++++++\n>  1 files changed, 30 insertions(+), 0 deletions(-)\n\nAcked-by: Shawn O. Pearce <spearce@spearce.org>\n\n-- \nShawn.\n"},{"id":"128886","messageId":"be6fef0d0912010812i54531ce0n18e4615c3f408569@mail.gmail.com","threadId":"18873","inReplyTo":"20091201160150.GB21299@spearce.org","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-01T16:12:17Z","receivedAt":"2009-12-01T16:12:17Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Dec 2, 2009 at 12:01 AM, Shawn O. Pearce <spearce@spearce.org> wrote:\n> In theory, since the cURL session stays active, we would have\n> received the 401 authentication error during the initial\n> \"GET $GIT_DIR/info/refs?service=git-$service\" request, and the subsequent\n> \"POST $GIT_DIR/git-$service\" requests would automatically include the\n> authentication data.\n>\n> That's theory.  Reality doesn't always agree with my theories.  :-)\n\nthat's because the curl session where the 401 was received (and thus\nsuccessful authentication takes place) is closed.\n\nI sent out a patch series recently which contains a patch to maintain\nat least one curl session throughout a http session (from http_init()\nto http_cleanup()), you can see this here:\n\n  http://www.spinics.net/lists/git/msg118190.html\n\n-- \nCheers,\nRay Chuan\n"},{"id":"128888","messageId":"20091201161428.GC21299@spearce.org","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011236360.5582@cone.home.martin.st","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T16:14:28Z","receivedAt":"2009-12-01T16:14:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Martin Storsj? <martin@martin.st> wrote:\n> When using multi-pass authentication methods, the curl library may\n> need to rewind the read buffers used for providing data to HTTP POST,\n> if data has been output before a 401 error is received.\n> \n> This solution buffers all data read by the curl library, in order to allow\n> it to rewind the reading buffer at any time later.\n\nNAK.\n\n\nIn the case of git-upload-pack requests, we should fit into 1 MiB\nalmost all of the time, and thus not need to grow the http.postBuffer\nto support a rewind.  The state data plus current have list isn't\nall that large.  A 1 MiB request means we have over 20,900 commits\nin common with the remote and still haven't been able to find a\nsufficient cut point.  Or the remote has 20,000 active, unrelated\nbranches we are trying to fetch.  Either way, this is a really sick\nand twisted situation.\n\nIn the case of git-receive-pack requests, we might be uploading an\nentire project to an empty repository on the remote side.  This could\nbe 8 GiB worth of data if the project was something huge like KDE.\nWe can't assume that we should malloc 8 GiB of memory to buffer\nthe payload.\n\nThe *correct* way to support an arbitrary rewind is to modify the\noutgoing channel from remote-curl to its protocol engine (client.in\nwithin the rpc_service method) to somehow request the protocol engine\n(aka git-send-pack or git-fetch-pack) to stop and regenerate the\ncurrent request.\n\n\nAnother approach would be to modify http-backend (and the protocol)\nto support an \"auth ping\" request prior to spooling out the entire\npayload if its more than an http.postBuffer size.  Basically we\ndo what the \"Expect: 100-continue\" protocol is supposed to do,\nbut in the application layer rather than the HTTP/1.1 layer, so\nour CGI actually gets invoked.\n\nThis unfortunately still relies on the underlying libcurl to not\ndiscard the authentication data after that initial \"auth ping\".\nBut to be honest, I think that is a reasonable expectation.  The\n#@!*@!* library should be able to generate two requests back-to-back\nto the same URL without needing to rewind the 2nd request.\n\n-- \nShawn.\n"},{"id":"128889","messageId":"20091201161642.GD21299@spearce.org","threadId":"18873","inReplyTo":"be6fef0d0912010812i54531ce0n18e4615c3f408569@mail.gmail.com","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-12-01T16:16:42Z","receivedAt":"2009-12-01T16:16:42Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Tay Ray Chuan <rctay89@gmail.com> wrote:\n> On Wed, Dec 2, 2009 at 12:01 AM, Shawn O. Pearce <spearce@spearce.org> wrote:\n> > In theory, since the cURL session stays active, we would have\n> > received the 401 authentication error during the initial\n> > \"GET $GIT_DIR/info/refs?service=git-$service\" request, and the subsequent\n> > \"POST $GIT_DIR/git-$service\" requests would automatically include the\n> > authentication data.\n> >\n> > That's theory. ?Reality doesn't always agree with my theories. ?:-)\n> \n> that's because the curl session where the 401 was received (and thus\n> successful authentication takes place) is closed.\n> \n> I sent out a patch series recently which contains a patch to maintain\n> at least one curl session throughout a http session (from http_init()\n> to http_cleanup()), you can see this here:\n> \n>   http://www.spinics.net/lists/git/msg118190.html\n\nRight, this patch looked sane to me.  It didn't touch code I recently\nhave touched myself, so I didn't bother to ACK, but if it helps,\nAcked-by: Shawn O. Pearce <spearce@spearce.org>.\n\n-- \nShawn.\n"},{"id":"128893","messageId":"alpine.DEB.2.00.0912011843320.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"20091201160150.GB21299@spearce.org","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-01T16:51:54Z","receivedAt":"2009-12-01T16:51:54Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Tue, 1 Dec 2009, Shawn O. Pearce wrote:\n\n> Martin Storsj? <martin@martin.st> wrote:\n> > When using multi-pass authentication methods, the curl library may need\n> > to rewind the read buffers used for providing data to HTTP POST, if data\n> > has been output before a 401 error is received.\n> \n> In theory, since the cURL session stays active, we would have\n> received the 401 authentication error during the initial\n> \"GET $GIT_DIR/info/refs?service=git-$service\" request, and the subsequent\n> \"POST $GIT_DIR/git-$service\" requests would automatically include the\n> authentication data.\n> \n> That's theory.  Reality doesn't always agree with my theories.  :-)\n\nAs Tay said - his \"maintain curl sessions\" patch should make this \nredundant in most cases. But in case request pattern gets changed or if \nthe curl session for some other reason isn't able to authenticate on the \nfirst try, this is a quite non-intrusive way of ensuring that these \nrequests can be restarted.\n\n// Martin\n"},{"id":"128897","messageId":"alpine.DEB.2.00.0912011852030.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"20091201161428.GC21299@spearce.org","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-01T16:59:02Z","receivedAt":"2009-12-01T16:59:02Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Tue, 1 Dec 2009, Shawn O. Pearce wrote:\n\n> In the case of git-receive-pack requests, we might be uploading an\n> entire project to an empty repository on the remote side.  This could\n> be 8 GiB worth of data if the project was something huge like KDE.\n> We can't assume that we should malloc 8 GiB of memory to buffer\n> the payload.\n\nTrue, fair enough. This was mostly a proof of concept of how this could be \nimplemented, but with these comments for you, it's clear that this isn't a \nfeasible solution at all. There's no acute need for it either.\n\n> The *correct* way to support an arbitrary rewind is to modify the\n> outgoing channel from remote-curl to its protocol engine (client.in\n> within the rpc_service method) to somehow request the protocol engine\n> (aka git-send-pack or git-fetch-pack) to stop and regenerate the\n> current request.\n\nThat's a good idea!\n\n> Another approach would be to modify http-backend (and the protocol)\n> to support an \"auth ping\" request prior to spooling out the entire\n> payload if its more than an http.postBuffer size.  Basically we\n> do what the \"Expect: 100-continue\" protocol is supposed to do,\n> but in the application layer rather than the HTTP/1.1 layer, so\n> our CGI actually gets invoked.\n\nThat's also quite a good idea, especially if it would be done in a way so \nthat it's certain that the same curl session will be reused, instead of \ngetting a potentially new curl session when using get_active_slot().\n\n> This unfortunately still relies on the underlying libcurl to not\n> discard the authentication data after that initial \"auth ping\".\n> But to be honest, I think that is a reasonable expectation.  The\n> #@!*@!* library should be able to generate two requests back-to-back\n> to the same URL without needing to rewind the 2nd request.\n\nYeah, as long as the same curl session is preserved, this should be no \nproblem.\n\n// Martin\n"},{"id":"128905","messageId":"7vzl62zisy.fsf@alter.siamese.dyndns.org","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011232450.5582@cone.home.martin.st","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-01T17:49:01Z","receivedAt":"2009-12-01T17:49:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Storsjö <martin@martin.st> writes:\n\n> As long as the current rpc read buffer is the first one, we're able to\n> rewind without need for additional buffering.\n\n... and if the current buffer isn't the first one, what do we do?\n\n> +#ifndef NO_CURL_IOCTL\n> +curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)\n> +{\n> +\tstruct rpc_state *rpc = clientp;\n> +\n> +\tswitch (cmd) {\n> +\tcase CURLIOCMD_NOP:\n> +\t\treturn CURLIOE_OK;\n> +\n> +\tcase CURLIOCMD_RESTARTREAD:\n> +\t\tif (rpc->initial_buffer) {\n> +\t\t\trpc->pos = 0;\n> +\t\t\treturn CURLIOE_OK;\n> +\t\t}\n> +\t\tfprintf(stderr, \"Unable to rewind rpc post data - try increasing http.postBuffer\\n\");\n> +\t\treturn CURLIOE_FAILRESTART;\n> +\n> +\tdefault:\n> +\t\treturn CURLIOE_UNKNOWNCMD;\n> +\t}\n> +}\n> +#endif\n\nWhat will this result in?  A failed request, then the user increases\nhttp.postBuffer, and re-runs the entire command?  I am not suggesting the\ncode should do it differently (e.g.  retry with a larger buffer without\nhaving the user to help it).  At least not yet.  That is why my first\nquestion above was \"what do we do?\" and not \"what should we do?\".\n\nI am primarily interested in _documenting_ the expected user experience in\nthe failure case, so that people can notice the message, run \"git grep\" to\nfind the above line and then run \"git blame\" to find the commit to read\nits log message to understand what is going on.\n"},{"id":"128911","messageId":"alpine.DEB.2.00.0912011914270.30348@tvnag.unkk.fr","threadId":"18873","inReplyTo":"20091201161428.GC21299@spearce.org","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-12-01T18:18:49Z","receivedAt":"2009-12-01T18:18:49Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Tue, 1 Dec 2009, Shawn O. Pearce wrote:\n\n> The #@!*@!* library should be able to generate two requests back-to-back to \n> the same URL without needing to rewind the 2nd request.\n\nIf '#@!*@!*' is your pattern for matching libcurl or curl, then sure libcurl \ncertainly has no problem at all to send as many requests you like \nback-to-back.\n\nThe rewinding business is only really necessary for multipass authentication \nwhen Expect: 100-continue doesn't work (and thus libcurl has started to send \ndata that the server will discard and thus is needed to get sent again). And \nthat's not something you can blame \"the #@!*@!* library\" for, but rather your \nserver end and/or how HTTP is defined to work.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"128948","messageId":"be6fef0d0912011803u2ec9ab1bsa167cf59de4dd47c@mail.gmail.com","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011914270.30348@tvnag.unkk.fr","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-02T02:03:33Z","receivedAt":"2009-12-02T02:03:33Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Dec 2, 2009 at 2:18 AM, Daniel Stenberg <daniel@haxx.se> wrote:\n> If '#@!*@!*' is your pattern for matching libcurl or curl, then sure libcurl\n> certainly has no problem at all to send as many requests you like\n> back-to-back.\n\nI have a feeling Shawn's referring to the git http library on top of that. ;)\n\n> The rewinding business is only really necessary for multipass authentication\n> when Expect: 100-continue doesn't work (and thus libcurl has started to send\n> data that the server will discard and thus is needed to get sent again). And\n> that's not something you can blame \"the #@!*@!* library\" for, but rather\n> your server end and/or how HTTP is defined to work.\n\nAccording to Martin, Expect: 100-continue is not working due to libcurl.\n\nI quote him:\n\nDate: Tue, 1 Dec 2009 12:28:26 +0200 (EET)\nSubject: Re: [PATCH 0/2] http: allow multi-pass authentication\n\nOn Tue, Dec 1, 2009 at 6:28 PM, Martin Storsjö <martin@martin.st> wrote:\n> Normally, libcurl should add the Expect: 100-continue header\n> automatically, but for some reason\n> (http://article.gmane.org/gmane.comp.web.curl.library/25992) it doesn't,\n> so that's probably why we're manually adding that header in\n> remote-curl.c:371 at the moment. libcurl doesn't detect this at the moment\n> (http://article.gmane.org/gmane.comp.web.curl.library/25991) so it won't\n> wait for the 100 continue response before starting to send the body data.\n\nBut, again, don't read my blaming of libcurl for this 100 business as\na criticism of curl.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"128963","messageId":"be6fef0d0912011832k12eaa093o73b057ddf4ab866@mail.gmail.com","threadId":"18873","inReplyTo":"7vzl62zisy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-02T02:32:34Z","receivedAt":"2009-12-02T02:32:34Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Dec 2, 2009 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ... and if the current buffer isn't the first one, what do we do?\n> [snip]\n> What will this result in?  A failed request, then the user increases\n> http.postBuffer, and re-runs the entire command?  I am not suggesting the\n> code should do it differently (e.g.  retry with a larger buffer without\n> having the user to help it).  At least not yet.  That is why my first\n> question above was \"what do we do?\" and not \"what should we do?\".\n\nI guess that by \"we\" you're referring to the \"normal\" users of git?\n\n> I am primarily interested in _documenting_ the expected user experience in\n> the failure case, so that people can notice the message, run \"git grep\" to\n> find the above line and then run \"git blame\" to find the commit to read\n> its log message to understand what is going on.\n\nYes, the code will just fail. As you might suspect, the code won't\nattempt to mitigate the failure by doing anything, and would require\nintervention on the part of the user.\n\nWhat the user could do to make this work:\n\n1. Turn off multi-pass authentication and just go with Basic.\n\n2. Allow for persistent curl sessions. In theory, we get a 401 the\nfirst time when we send a GET for info/refs; subsequently, curl knows\nwhat authentication to use, so the POST request *should* take place\nwithout the need for rewinding. In theory.\n\n3. Increase http.postBuffer size in the config.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"128964","messageId":"be6fef0d0912011915u78945c77x29880da3f709a912@mail.gmail.com","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912011852030.5582@cone.home.martin.st","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-02T03:15:15Z","receivedAt":"2009-12-02T03:15:15Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Dec 2, 2009 at 12:59 AM, Martin Storsjö <martin@martin.st> wrote:\n> On Tue, 1 Dec 2009, Shawn O. Pearce wrote:\n>> The *correct* way to support an arbitrary rewind is to modify the\n>> outgoing channel from remote-curl to its protocol engine (client.in\n>> within the rpc_service method) to somehow request the protocol engine\n>> (aka git-send-pack or git-fetch-pack) to stop and regenerate the\n>> current request.\n>\n> That's a good idea!\n>\n>> Another approach would be to modify http-backend (and the protocol)\n>> to support an \"auth ping\" request prior to spooling out the entire\n>> payload if its more than an http.postBuffer size.  Basically we\n>> do what the \"Expect: 100-continue\" protocol is supposed to do,\n>> but in the application layer rather than the HTTP/1.1 layer, so\n>> our CGI actually gets invoked.\n>\n> That's also quite a good idea, especially if it would be done in a way so\n> that it's certain that the same curl session will be reused, instead of\n> getting a potentially new curl session when using get_active_slot().\n\nI think restarting the read by killing the protocol engine/client and\nstarting again would be the easier of the two.\n\nNot just that, it would be neater than storing everything that the\nprotocol engine has spewed out, like Martin's patch does.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"128982","messageId":"alpine.DEB.2.00.0912020931560.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"be6fef0d0912011832k12eaa093o73b057ddf4ab866@mail.gmail.com","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-02T07:45:42Z","receivedAt":"2009-12-02T07:45:42Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Wed, 2 Dec 2009, Tay Ray Chuan wrote:\n\n> > What will this result in?  A failed request, then the user increases\n> > http.postBuffer, and re-runs the entire command?  I am not suggesting the\n> > code should do it differently (e.g.  retry with a larger buffer without\n> > having the user to help it).  At least not yet.  That is why my first\n> > question above was \"what do we do?\" and not \"what should we do?\".\n> \n> I guess that by \"we\" you're referring to the \"normal\" users of git?\n> \n> > I am primarily interested in _documenting_ the expected user experience in\n> > the failure case, so that people can notice the message, run \"git grep\" to\n> > find the above line and then run \"git blame\" to find the commit to read\n> > its log message to understand what is going on.\n> \n> Yes, the code will just fail. As you might suspect, the code won't\n> attempt to mitigate the failure by doing anything, and would require\n> intervention on the part of the user.\n> \n> What the user could do to make this work:\n> \n> 1. Turn off multi-pass authentication and just go with Basic.\n> \n> 2. Allow for persistent curl sessions. In theory, we get a 401 the\n> first time when we send a GET for info/refs; subsequently, curl knows\n> what authentication to use, so the POST request *should* take place\n> without the need for rewinding. In theory.\n\nI'd actually put this as number 1 - if this error message pops up for some \nreason, the first thing would be to find out why reusing the previous curl \nsessions didn't work.\n\nOther options are:\n\n- Switch to a HTTP server that handles Expect: 100-continue properly\n- Try pushing the data in smaller chunks, e.g. if populating a new repo \nfrom scratch, don't push the whole history in one single run, or populate \nthrough some other mechanism and just do the incremental pushs over HTTP.\n\nAnd possibly: Update curl to a version post 7.19.7, which detects the \nExpect header set by git and tries to await a response from the server \nbefore proceeding. (The problem that would solve is if we start sending \nand manage to send the whole initial 1 MB buffer before the 401 reply from \nthe server is received. But it doesn't solve the case if the server \ndoesn't understand the Expect header at all.)\n\n> 3. Increase http.postBuffer size in the config.\n\nAs Shawn pointed out, if the whole request would have to be buffered, the \nneeded size may be prohibitively large, so I guess this isn't a good hint \nto include in the error message after all. But if the request is sensibly \nsized (e.g. on the order of tens of MBs), this may be a stopgap solution.\n\n\nSo, should we change the error message to something a bit more \ndescriptive, and add this discussion into the commit message?\n\n// Martin"},{"id":"128983","messageId":"alpine.DEB.2.00.0912021011430.19179@tvnag.unkk.fr","threadId":"18873","inReplyTo":"be6fef0d0912011803u2ec9ab1bsa167cf59de4dd47c@mail.gmail.com","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-12-02T09:19:00Z","receivedAt":"2009-12-02T09:19:00Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Wed, 2 Dec 2009, Tay Ray Chuan wrote:\n\n> According to Martin, Expect: 100-continue is not working due to libcurl.\n\nRight, that is/was a bug in how libcurl behaves when the application itself \nhas set the \"Expect: 100-continue\" header. Martin has provided a fix for that \nfor the next libcurl version though, but that won't make a lot of existing \nusers happy.\n\nThinking about this particular problem, what is the motivation for git to \nforcily add that header in the first place? I mean, libcurl does add the \nheader by itself when it thinks it is necessary and then it handles it \ncorrectly.\n\nI'm just suggesting (and speculating widely since I don't know git internals) \nthat a possible way to work around this particular bug may be to reconsider \nhow git adds the Expect header.\n\nIt's just an idea. Please ignore it if it is totally crazy.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"128986","messageId":"alpine.DEB.2.00.0912021130300.5582@cone.home.martin.st","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912021011430.19179@tvnag.unkk.fr","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2009-12-02T09:32:43Z","receivedAt":"2009-12-02T09:32:43Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Wed, 2 Dec 2009, Daniel Stenberg wrote:\n\n> On Wed, 2 Dec 2009, Tay Ray Chuan wrote:\n> \n> > According to Martin, Expect: 100-continue is not working due to libcurl.\n> \n> Right, that is/was a bug in how libcurl behaves when the application itself\n> has set the \"Expect: 100-continue\" header. Martin has provided a fix for that\n> for the next libcurl version though, but that won't make a lot of existing\n> users happy.\n> \n> Thinking about this particular problem, what is the motivation for git to\n> forcily add that header in the first place? I mean, libcurl does add the\n> header by itself when it thinks it is necessary and then it handles it\n> correctly.\n\nAs far as I saw, the reason for it being manually added is that curl \nactually didn't add it automatically in that case. That was the reason for \nthe second patch/rfc thread that I sent to curl-library (where postsize == \n0, as in unknown, didn't trigger the addition of any Expect header).\n\n// Martin\n"},{"id":"128989","messageId":"alpine.DEB.2.00.0912021053040.27454@tvnag.unkk.fr","threadId":"18873","inReplyTo":"alpine.DEB.2.00.0912021130300.5582@cone.home.martin.st","subject":"Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-12-02T10:04:17Z","receivedAt":"2009-12-02T10:04:17Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Wed, 2 Dec 2009, Martin Storsjö wrote:\n\n>> Thinking about this particular problem, what is the motivation for git to \n>> forcily add that header in the first place? I mean, libcurl does add the \n>> header by itself when it thinks it is necessary and then it handles it \n>> correctly.\n>\n> As far as I saw, the reason for it being manually added is that curl \n> actually didn't add it automatically in that case. That was the reason for \n> the second patch/rfc thread that I sent to curl-library (where postsize == \n> 0, as in unknown, didn't trigger the addition of any Expect header).\n\nAh right, thanks for the clarification. An unfortunate combination then... :-(\n\n-- \n\n  / daniel.haxx.se"}]}