{"thread":{"id":"35087","subject":"[PATCH 0/2] HTTP GSS-Negotiate improvements","startedAt":"2013-10-08T20:48:05Z","lastAt":"2013-10-12T02:02:15Z","messageCount":11,"participants":["brian m. carlson","Jeff King","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"228628","messageId":"1381265287-39331-1-git-send-email-sandals@crustytoothpaste.net","threadId":"35087","inReplyTo":null,"subject":"[PATCH 0/2] HTTP GSS-Negotiate improvements","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-08T20:48:05Z","receivedAt":"2013-10-08T20:48:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This patch set adds an option, http.continue, to enable requests for 100\nContinue responses when pushing over HTTP. This is needed for large pushes\nusing GSS-Negotiate authentication.\n\nI decided to be optimistic and default the option to on, which means that 100\nContinue responses will be requested. My rationale for this is that modern\nversions of Squid, Apache, and nginx seem to do the right thing, as well as most\nnon-proxy HTTP 1.1 implementations, and that we should optimize for functional,\nup-to-date software. HTTP 1.1 is 16 years old, after all.\n\nbrian m. carlson (2):\n http: add option to enable 100 Continue responses\n Update documentation for http.continue option\n\n Documentation/config.txt | 9 +++++++++\n http.c | 6 ++++++\n http.h | 1 +\n remote-curl.c | 7 ++++++-\n 4 files changed, 22 insertions(+), 1 deletion(-)\n\n-- \n1.8.4.rc3\n"},{"id":"228629","messageId":"1381265287-39331-2-git-send-email-sandals@crustytoothpaste.net","threadId":"35087","inReplyTo":"1381265287-39331-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-08T20:48:06Z","receivedAt":"2013-10-08T20:48:06Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When using GSS-Negotiate authentication with libcurl, the authentication\nprovided will change every time, and so the probe that git uses to determine if\nauthentication is needed is not sufficient to guarantee that data can be sent.\nIf the data fits entirely in http.postBuffer bytes, the data can be rewound and\nresent if authentication fails; otherwise, a 100 Continue must be requested in\nthis case.\n\nBy default, curl will send an Expect: 100-continue if a certain amount of data\nis to be uploaded, but when using chunked data this is never triggered. Add an\noption http.continue, which defaults to enabled, to control whether this header\nis sent. The addition of an option is necessary because some proxies break\nbadly if sent this header.\n\nSigned-off-by: brian m. carlson \n---\n http.c | 6 ++++++\n http.h | 1 +\n remote-curl.c | 7 ++++++-\n 3 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/http.c b/http.c\nindex f3e1439..941c941 100644\n--- a/http.c\n+++ b/http.c\n@@ -11,6 +11,7 @@\n int active_requests;\n int http_is_verbose;\n size_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n+int http_use_100_continue = 1;\n \n #if LIBCURL_VERSION_NUM >= 0x070a06\n #define LIBCURL_CAN_HANDLE_AUTH_ANY\n@@ -213,6 +214,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.continue\", var)) {\n+\t\thttp_use_100_continue = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.useragent\", var))\n \t\treturn git_config_string(&user_agent, var, value);\n \ndiff --git a/http.h b/http.h\nindex d77c1b5..e72786e 100644\n--- a/http.h\n+++ b/http.h\n@@ -102,6 +102,7 @@ extern void http_cleanup(void);\n extern int active_requests;\n extern int http_is_verbose;\n extern size_t http_post_buffer;\n+extern int http_use_100_continue;\n \n extern char curl_errorstr[CURL_ERROR_SIZE];\n \ndiff --git a/remote-curl.c b/remote-curl.c\nindex b5ebe01..3b5e160 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc)\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n-\theaders = curl_slist_append(headers, \"Expect:\");\n+\n+\t/* Force it either on or off, since curl will try to decide based on how\n+\t * much data is to be uploaded and we want consistency.\n+\t */\n+\theaders = curl_slist_append(headers, http_use_100_continue ?\n+\t\t\"Expect: 100-continue\" : \"Expect:\");\n \n retry:\n \tslot = get_active_slot();\n-- \n1.8.4.rc3\n"},{"id":"228630","messageId":"1381265287-39331-3-git-send-email-sandals@crustytoothpaste.net","threadId":"35087","inReplyTo":"1381265287-39331-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 2/2] Update documentation for http.continue option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-08T20:48:07Z","receivedAt":"2013-10-08T20:48:07Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Explain the reason for and behavior of the new http.continue option, and that it\nis enabled by default. Furthermore, explain that it is required for large\nGSS-Negotiate requests but incompatible with some proxies.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/config.txt | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex fca7749..5fff2b2 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1516,6 +1516,15 @@ http.postBuffer::\n \tmassive pack file locally. Default is 1 MiB, which is\n \tsufficient for most requests.\n \n+http.continue::\n+\tEnsure that authentication succeeds before sending the pack data when\n+\tPOSTing data using the smart HTTP transport. This is done by\n+\trequesting a 100 Continue response. For requests larger than\n+\t'http.postBuffer', this is required when using GSS-Negotiate\n+\t(Kerberos) authentication over HTTP. However, some proxies do not\n+\thandle the protocol exchange gracefully; for them, this option must be\n+\tdisabled. Defaults to enabled.\n+\n http.lowSpeedLimit, http.lowSpeedTime::\n \tIf the HTTP transfer speed is less than 'http.lowSpeedLimit'\n \tfor longer than 'http.lowSpeedTime' seconds, the transfer is aborted.\n-- \n1.8.4.rc3\n"},{"id":"228671","messageId":"20131009193054.GA3767@sigill.intra.peff.net","threadId":"35087","inReplyTo":"1381265287-39331-2-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-09T19:30:54Z","receivedAt":"2013-10-09T19:30:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 08, 2013 at 08:48:06PM +0000, brian m. carlson wrote:\n\n> When using GSS-Negotiate authentication with libcurl, the authentication\n> provided will change every time, and so the probe that git uses to determine if\n> authentication is needed is not sufficient to guarantee that data can be sent.\n> If the data fits entirely in http.postBuffer bytes, the data can be rewound and\n> resent if authentication fails; otherwise, a 100 Continue must be requested in\n> this case.\n> \n> By default, curl will send an Expect: 100-continue if a certain amount of data\n> is to be uploaded, but when using chunked data this is never triggered. Add an\n> option http.continue, which defaults to enabled, to control whether this header\n> is sent. The addition of an option is necessary because some proxies break\n> badly if sent this header.\n\n[+cc spearce]\n\nI'd be more comfortable defaulting this to \"on\" if I understood more\nabout the original problem that led to 959dfcf and 206b099. It sounds\nlike enabling this all the time will cause annoying stalls in the\nprotocol, unless the number of non-crappy proxy implementations has\ngotten smaller over the past few years.\n\n> diff --git a/remote-curl.c b/remote-curl.c\n> index b5ebe01..3b5e160 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc)\n> \n> \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n> \theaders = curl_slist_append(headers, rpc->hdr_accept);\n> -\theaders = curl_slist_append(headers, \"Expect:\");\n> +\n> +\t/* Force it either on or off, since curl will try to decide based on how\n> +\t * much data is to be uploaded and we want consistency.\n> +\t */\n> +\theaders = curl_slist_append(headers, http_use_100_continue ?\n> +\t\t\"Expect: 100-continue\" : \"Expect:\");\n\nIs there any point in sending the Expect: header in cases where curl\nwould not send it, though? It seems like we should assume curl does the\nright thing most of the time, and have our option only be to override\ncurl in the negative direction.\n\n-Peff\n"},{"id":"228675","messageId":"CAJo=hJvyorMjFYZnVwz4iZr88ewor6LuqOE-mpt4LsPyoddBqg@mail.gmail.com","threadId":"35087","inReplyTo":"20131009193054.GA3767@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2013-10-09T21:19:36Z","receivedAt":"2013-10-09T21:19:36Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Oct 9, 2013 at 12:30 PM, Jeff King wrote:\n> On Tue, Oct 08, 2013 at 08:48:06PM +0000, brian m. carlson wrote:\n>\n>> When using GSS-Negotiate authentication with libcurl, the authentication\n>> provided will change every time, and so the probe that git uses to determine if\n>> authentication is needed is not sufficient to guarantee that data can be sent.\n>> If the data fits entirely in http.postBuffer bytes, the data can be rewound and\n>> resent if authentication fails; otherwise, a 100 Continue must be requested in\n>> this case.\n>>\n>> By default, curl will send an Expect: 100-continue if a certain amount of data\n>> is to be uploaded, but when using chunked data this is never triggered. Add an\n>> option http.continue, which defaults to enabled, to control whether this header\n>> is sent. The addition of an option is necessary because some proxies break\n>> badly if sent this header.\n>\n> [+cc spearce]\n>\n> I'd be more comfortable defaulting this to \"on\" if I understood more\n> about the original problem that led to 959dfcf and 206b099. It sounds\n> like enabling this all the time will cause annoying stalls in the\n> protocol, unless the number of non-crappy proxy implementations has\n> gotten smaller over the past few years.\n\nIt actually hasn't, not significantly.\n\n206b099 was written because the Google web servers for\nandroid.googlesource.com and code.google.com do not support\n100-continue semantics. This caused the client to stall a full 1\nsecond before each POST exchange. If ancestor negotiation required\nO(128) have lines to be advertised I think this was 2 or 4 POSTs,\nresulting in 2-4 second stalls above the other latency of the network\nand the server.\n\nThe advice I received from the Google web server developers was to not\nrely on 100-continue, because a large number of corporate proxy\nservers do not support it correctly. HTTP 100-continue is just pretty\nbroken in the wild.\n\nIf \"Expect: 100-continue\" is required for GSS-Negotiate to work then\nGit should only enable the option if the server is demanding\nGSS-Negotiate for authentication. Per 206b099 the default should still\nbe off for anonymous and HTTP basic, digest, and SSL certificate\nauthentication.\n\n>> diff --git a/remote-curl.c b/remote-curl.c\n>> index b5ebe01..3b5e160 100644\n>> --- a/remote-curl.c\n>> +++ b/remote-curl.c\n>> @@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc)\n>>\n>> headers = curl_slist_append(headers, rpc->hdr_content_type);\n>> headers = curl_slist_append(headers, rpc->hdr_accept);\n>> - headers = curl_slist_append(headers, \"Expect:\");\n>> +\n>> + /* Force it either on or off, since curl will try to decide based on how\n>> + * much data is to be uploaded and we want consistency.\n>> + */\n>> + headers = curl_slist_append(headers, http_use_100_continue ?\n>> + \"Expect: 100-continue\" : \"Expect:\");\n>\n> Is there any point in sending the Expect: header in cases where curl\n> would not send it, though? It seems like we should assume curl does the\n> right thing most of the time, and have our option only be to override\n> curl in the negative direction.\n\nAdding a header of \"Expect:\" causes curl to disable the header and\nnever use it. Always supplying the header with no value prevents\nlibcurl from using 100-continue on its own, which is what I fixed in\n959dfcf.\n"},{"id":"228676","messageId":"20131009213742.GA8362@sigill.intra.peff.net","threadId":"35087","inReplyTo":"CAJo=hJvyorMjFYZnVwz4iZr88ewor6LuqOE-mpt4LsPyoddBqg@mail.gmail.com","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-09T21:37:42Z","receivedAt":"2013-10-09T21:37:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote:\n\n> > I'd be more comfortable defaulting this to \"on\" if I understood more\n> > about the original problem that led to 959dfcf and 206b099. It sounds\n> > like enabling this all the time will cause annoying stalls in the\n> > protocol, unless the number of non-crappy proxy implementations has\n> > gotten smaller over the past few years.\n> \n> It actually hasn't, not significantly.\n\nThanks for update; my pessimistic side assumed this was the case, but it\nis always good to check. :)\n\n> 206b099 was written because the Google web servers for\n> android.googlesource.com and code.google.com do not support\n> 100-continue semantics. This caused the client to stall a full 1\n> second before each POST exchange. If ancestor negotiation required\n> O(128) have lines to be advertised I think this was 2 or 4 POSTs,\n> resulting in 2-4 second stalls above the other latency of the network\n> and the server.\n\nYuck.\n\n> If \"Expect: 100-continue\" is required for GSS-Negotiate to work then\n> Git should only enable the option if the server is demanding\n> GSS-Negotiate for authentication. Per 206b099 the default should still\n> be off for anonymous and HTTP basic, digest, and SSL certificate\n> authentication.\n\nPart of the problem is that curl is the one handling the negotiation.\nWhen we get a 401, I think we can ask curl_easy_getinfo to tell us which\nauth methods are available (via CURLINFO_HTTPAUTH_AVAIL). But I don't\nknow how we decide that GSS is what's going to be used. I guess if it is\nthe only option, and there is no basic-auth offered?\n\nAnd then in that case turn on \"Expect\" (or more accurately, stop\ndisabling it).\n\nI don't have a GSS-enabled server to test on. Brian, can you try the\npatch at the end of this message on your non-working server and see what\nit outputs?\n\n> >> + headers = curl_slist_append(headers, http_use_100_continue ?\n> >> + \"Expect: 100-continue\" : \"Expect:\");\n> >\n> > Is there any point in sending the Expect: header in cases where curl\n> > would not send it, though? It seems like we should assume curl does the\n> > right thing most of the time, and have our option only be to override\n> > curl in the negative direction.\n> \n> Adding a header of \"Expect:\" causes curl to disable the header and\n> never use it. Always supplying the header with no value prevents\n> libcurl from using 100-continue on its own, which is what I fixed in\n> 959dfcf.\n\nRight. What I meant is that we do not want to unconditionally send the\n\"Expect: 100-continue\" in the other case. IOW, we would just want:\n\n if (!http_use_100_continue)\n headers = curl_slist_append(headers, \"Expect:\");\n\n-Peff\n\n-- >8 --\ndiff --git a/http.c b/http.c\nindex f3e1439..e7257d7 100644\n--- a/http.c\n+++ b/http.c\n@@ -889,6 +889,12 @@ static int http_request(const char *url, struct strbuf *type,\n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n \t\tret = handle_curl_result(&results);\n+\t\tif (ret == HTTP_REAUTH) {\n+\t\t\tlong auth_avail;\n+\t\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,\n+\t\t\t\t\t &auth_avail);\n+\t\t\tfprintf(stderr, \"offered auth: %ld\\n\", auth_avail);\n+\t\t}\n \t} else {\n \t\tsnprintf(curl_errorstr, sizeof(curl_errorstr),\n \t\t\t \"failed to start HTTP request\");\n"},{"id":"228680","messageId":"20131010013547.GA62549@vauxhall.crustytoothpaste.net","threadId":"35087","inReplyTo":"20131009213742.GA8362@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-10T01:35:47Z","receivedAt":"2013-10-10T01:35:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 09, 2013 at 05:37:42PM -0400, Jeff King wrote:\n> On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote:\n> > 206b099 was written because the Google web servers for\n> > android.googlesource.com and code.google.com do not support\n> > 100-continue semantics. This caused the client to stall a full 1\n> > second before each POST exchange. If ancestor negotiation required\n> > O(128) have lines to be advertised I think this was 2 or 4 POSTs,\n> > resulting in 2-4 second stalls above the other latency of the network\n> > and the server.\n> \n> Yuck.\n\nShame on Google. Of all people, they should be able to implement HTTP\n1.1 properly.\n\n> Part of the problem is that curl is the one handling the negotiation.\n> When we get a 401, I think we can ask curl_easy_getinfo to tell us which\n> auth methods are available (via CURLINFO_HTTPAUTH_AVAIL). But I don't\n> know how we decide that GSS is what's going to be used. I guess if it is\n> the only option, and there is no basic-auth offered?\n> \n> And then in that case turn on \"Expect\" (or more accurately, stop\n> disabling it).\n> \n> I don't have a GSS-enabled server to test on. Brian, can you try the\n> patch at the end of this message on your non-working server and see what\n> it outputs?\n\nIt doesn't trigger. My server only requires authentication for the\nactual push operation, and it looks like your code doesn't trigger in\nthat case.\n\n> > >> + headers = curl_slist_append(headers, http_use_100_continue ?\n> > >> + \"Expect: 100-continue\" : \"Expect:\");\n> > >\n> > > Is there any point in sending the Expect: header in cases where curl\n> > > would not send it, though? It seems like we should assume curl does the\n> > > right thing most of the time, and have our option only be to override\n> > > curl in the negative direction.\n\nI think curl expects that data in the POSTFIELDS option in order to\ntrigger. I wasn't able to get it to trigger on its own.\n\n> > Adding a header of \"Expect:\" causes curl to disable the header and\n> > never use it. Always supplying the header with no value prevents\n> > libcurl from using 100-continue on its own, which is what I fixed in\n> > 959dfcf.\n> \n> Right. What I meant is that we do not want to unconditionally send the\n> \"Expect: 100-continue\" in the other case. IOW, we would just want:\n> \n> if (!http_use_100_continue)\n> headers = curl_slist_append(headers, \"Expect:\");\n\nI tried that. It doesn't work, since it never sends the Expect:\n100-continue. I made libcurl dump all the headers it sends, and it\ndoesn't send them in that case.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"228681","messageId":"20131010014319.GA15106@sigill.intra.peff.net","threadId":"35087","inReplyTo":"20131010013547.GA62549@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-10T01:43:19Z","receivedAt":"2013-10-10T01:43:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 10, 2013 at 01:35:47AM +0000, brian m. carlson wrote:\n\n> > I don't have a GSS-enabled server to test on. Brian, can you try the\n> > patch at the end of this message on your non-working server and see what\n> > it outputs?\n> \n> It doesn't trigger. My server only requires authentication for the\n> actual push operation, and it looks like your code doesn't trigger in\n> that case.\n\nCan you describe the sequence of requests, then?\n\nDo you not have to auth for the info/refs request, and then have to auth\nlater for the git-receive-pack request? Does the auth trigger for the\nprobe_rpc call?\n\nCan you show us GIT_CURL_VERBOSE=1 output for a session that fails (do\nnote that it will dump your auth headers, so you may want to redact them\nbefore sharing)?\n\n> > > > Is there any point in sending the Expect: header in cases where curl\n> > > > would not send it, though? It seems like we should assume curl does the\n> > > > right thing most of the time, and have our option only be to override\n> > > > curl in the negative direction.\n> \n> I think curl expects that data in the POSTFIELDS option in order to\n> trigger. I wasn't able to get it to trigger on its own.\n\nWas your POST perhaps not big enough? My impression is that it only\nsends it if the POST is too large to rewind (which seems like a good\nthing in general), and I thought earlier in the thread that command-line\ncurl was sending one. I'm not sure what would be different for us here,\nonce our manual \"Expect\" suppression is turned off.\n\n-Peff\n"},{"id":"228690","messageId":"CAJo=hJtBapzmF7BEawwRGJ0NKH1W0J5P4c4iGK6G_==gZahhgg@mail.gmail.com","threadId":"35087","inReplyTo":"20131010013547.GA62549@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2013-10-10T08:14:28Z","receivedAt":"2013-10-10T08:14:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Oct 9, 2013 at 6:35 PM, brian m. carlson\n wrote:\n> On Wed, Oct 09, 2013 at 05:37:42PM -0400, Jeff King wrote:\n>> On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote:\n>> > 206b099 was written because the Google web servers for\n>> > android.googlesource.com and code.google.com do not support\n>> > 100-continue semantics. This caused the client to stall a full 1\n>> > second before each POST exchange. If ancestor negotiation required\n>> > O(128) have lines to be advertised I think this was 2 or 4 POSTs,\n>> > resulting in 2-4 second stalls above the other latency of the network\n>> > and the server.\n>>\n>> Yuck.\n>\n> Shame on Google. Of all people, they should be able to implement HTTP\n> 1.1 properly.\n\nHeh. =)\n\nIf a large enough percentage of users are stuck behind a proxy that\ndoesn't support 100-continue, it is hard to rely on that part of HTTP\n1.1. You need to build the work-around for them anyway, so you might\nas well just make everyone use the work-around and assume 100-continue\ndoes not exist.\n\n100-continue is frequently used when there is a large POST body, but\nthose suck for users on slow or unstable connections. Typically the\nPOST cannot be resumed where the connection was broken. To be friendly\nto users on less reliable connections than your gigabit office\nethernet, you need to design the client side with some sort of\nchunking and gracefully retrying. So Git is really doing it all wrong.\n:-)\n\nProperly using 100-continue adds a full RTT to any request using it.\nIf the RTT time for an end-user to server is already 100-160ms on the\npublic Internet, using 100-continue just added an extra 160ms of\nlatency to whatever the operation was. That is hardly useful to\nanyone. During that RTT the server has resources tied up associated\nwith that client connection. For your 10-person workgroup server this\nis probably no big deal; at scale it can be a daunting additional\nresource load.\n\nEtc.\n\n\nEven if you want to live in the fairy land where all servers support\n100-continue, I'm not sure clients should pay that 100-160ms latency\npenalty during ancestor negotiation. Do 5 rounds of negotiation and\nits suddenly an extra half second for `git fetch`, and that is a\nfairly well connected client. Let me know how it works from India to a\nserver on the west coast of the US, latency might be more like 200ms,\nand 5 rounds is now 1 full second of additional lag.\n"},{"id":"228752","messageId":"20131011223151.GC79408@vauxhall.crustytoothpaste.net","threadId":"35087","inReplyTo":"CAJo=hJtBapzmF7BEawwRGJ0NKH1W0J5P4c4iGK6G_==gZahhgg@mail.gmail.com","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-11T22:31:52Z","receivedAt":"2013-10-11T22:31:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Oct 10, 2013 at 01:14:28AM -0700, Shawn Pearce wrote:\n> If a large enough percentage of users are stuck behind a proxy that\n> doesn't support 100-continue, it is hard to rely on that part of HTTP\n> 1.1. You need to build the work-around for them anyway, so you might\n> as well just make everyone use the work-around and assume 100-continue\n> does not exist.\n\nWell, the issue is that 100-continue is needed for functionality in some\ncases, unless we want to restart the git-upload-pack command again or\nforce people to use outrageous sizes for http.postBuffer. My preference\nis generally to optimize for sane, standards-compliant behavior first,\nand let the people with broken infrastructure turn on options to work\naround that breakage. I realize that git as a project is a little more\ntolerant of people's myriad forms of breakage than I am personally.\n\nRegardless, I have a reroll that leaves it disabled by default that I'll\nsend in a few minutes.\n\n> 100-continue is frequently used when there is a large POST body, but\n> those suck for users on slow or unstable connections. Typically the\n> POST cannot be resumed where the connection was broken. To be friendly\n> to users on less reliable connections than your gigabit office\n> ethernet, you need to design the client side with some sort of\n> chunking and gracefully retrying. So Git is really doing it all wrong.\n> :-)\n\nYeah, there's been requests for resumable pull/push before. The proper\nway to do it would probably to send lots of little mini-packs that each\ndepend on the previous pack sent; if the connection gets reset, then at\nleast some of the data has been transferred, and negotiation would\nrestart the next time. The number of SHA-1s sent during negotiation\nwould have to increase though, because you couldn't be guaranteed that\nan entire ref would be able to be transferred each time. Large blobs\nwould still be a problem, though, and efficiency would plummet.\n\n> Even if you want to live in the fairy land where all servers support\n> 100-continue, I'm not sure clients should pay that 100-160ms latency\n> penalty during ancestor negotiation. Do 5 rounds of negotiation and\n> its suddenly an extra half second for `git fetch`, and that is a\n> fairly well connected client. Let me know how it works from India to a\n> server on the west coast of the US, latency might be more like 200ms,\n> and 5 rounds is now 1 full second of additional lag.\n\nThere shouldn't be that many rounds of negotiation. HTTP retrieves the\nlist of refs over one connection, and then performs the POST over\nanother two. Regardless, you should be using SSL over that connection,\nand the number of round trips required for SSL negotiation in that case\ncompletely dwarfs the overhead for the 100 continue, especially since\nyou'll do it thrice (even though the session is usually reused). The\nefficient way to do push is SSH, where you can avoid making multiple\nconnections and reuse the same encrypted connection at every stage.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"228767","messageId":"CAJo=hJu30M-vk5oavfi8TAS8B551MRmABx7EqN5GGxq_Hd9g=A@mail.gmail.com","threadId":"35087","inReplyTo":"20131011223151.GC79408@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/2] http: add option to enable 100 Continue responses","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2013-10-12T02:02:15Z","receivedAt":"2013-10-12T02:02:15Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Fri, Oct 11, 2013 at 3:31 PM, brian m. carlson\n wrote:\n> On Thu, Oct 10, 2013 at 01:14:28AM -0700, Shawn Pearce wrote:\n>> Even if you want to live in the fairy land where all servers support\n>> 100-continue, I'm not sure clients should pay that 100-160ms latency\n>> penalty during ancestor negotiation. Do 5 rounds of negotiation and\n>> its suddenly an extra half second for `git fetch`, and that is a\n>> fairly well connected client. Let me know how it works from India to a\n>> server on the west coast of the US, latency might be more like 200ms,\n>> and 5 rounds is now 1 full second of additional lag.\n>\n> There shouldn't be that many rounds of negotiation. HTTP retrieves the\n> list of refs over one connection, and then performs the POST over\n> another two.\n\nWhy two connections? This should be a single HTTP connection with HTTP\nKeep-Alive semantics allowing the same TCP stream and the same SSL\nstream to be used for all requests. Which is nearly equivalent to SSH.\nWhere SSH wins is the multi_ack protocol allowing the server to talk\nwhile the client is talking.\n\n> Regardless, you should be using SSL over that connection,\n> and the number of round trips required for SSL negotiation in that case\n> completely dwarfs the overhead for the 100 continue, especially since\n> you'll do it thrice (even though the session is usually reused). The\n> efficient way to do push is SSH, where you can avoid making multiple\n> connections and reuse the same encrypted connection at every stage.\n\nSSH setup is also not free. Like SSL its going to require a round trip\nor two on top of what Git needs.\n"}]}