{"thread":{"id":"35211","subject":"[PATCH v3] remote-curl: fix large pushes with GSSAPI","startedAt":"2013-10-26T22:34:42Z","lastAt":"2013-10-29T02:17:47Z","messageCount":2,"participants":["brian m. carlson","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"229578","messageId":"1382826882-48704-1-git-send-email-sandals@crustytoothpaste.net","threadId":"35211","inReplyTo":null,"subject":"[PATCH v3] remote-curl: fix large pushes with GSSAPI","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-26T22:34:42Z","receivedAt":"2013-10-26T22:34:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Due to an interaction between the way libcurl handles GSSAPI authentication over\nHTTP and the way git uses libcurl, large pushes (those over http.postBuffer\nbytes) would fail due to an authentication failure requiring a rewind of the\ncurl buffer.  Such a rewind was not possible because the data did not fit into\nthe entire buffer.\n\nEnable the use of the Expect: 100-continue header for large requests where the\nserver offers GSSAPI authentication to avoid this issue, since the request would\notherwise fail.  This allows git to get the authentication data right before\nsending the pack contents.  Existing cases where pushes would succeed, including\nsmall requests using GSSAPI, still disable the use of 100 Continue, as it causes\nproblems for some remote HTTP implementations (servers and proxies).\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n remote-curl.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex c9b891a..35698e7 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -449,6 +449,7 @@ static int post_rpc(struct rpc_state *rpc)\n \tchar *gzip_body = NULL;\n \tsize_t gzip_size = 0;\n \tint err, large_request = 0;\n+\tint needs_100_continue = 0;\n \n \t/* Try to load the entire request, if we can fit it into the\n \t * allocated buffer space we can use HTTP/1.0 and avoid the\n@@ -472,6 +473,8 @@ static int post_rpc(struct rpc_state *rpc)\n \t}\n \n \tif (large_request) {\n+\t\tlong authtype = 0;\n+\n \t\tdo {\n \t\t\terr = probe_rpc(rpc);\n \t\t\tif (err == HTTP_REAUTH)\n@@ -479,11 +482,17 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t} while (err == HTTP_REAUTH);\n \t\tif (err != HTTP_OK)\n \t\t\treturn -1;\n+\n+\t\tslot = get_active_slot();\n+\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n+\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n+\t\t\tneeds_100_continue = 1;\n \t}\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+\theaders = curl_slist_append(headers, needs_100_continue ?\n+\t\t\"Expect: 100-continue\" : \"Expect:\");\n \n retry:\n \tslot = get_active_slot();\n-- \n1.8.4.1.635.g55556a5\n"},{"id":"229696","messageId":"20131029021747.GF11861@sigill.intra.peff.net","threadId":"35211","inReplyTo":"1382826882-48704-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v3] remote-curl: fix large pushes with GSSAPI","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-29T02:17:47Z","receivedAt":"2013-10-29T02:17:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 26, 2013 at 10:34:42PM +0000, brian m. carlson wrote:\n\n> Enable the use of the Expect: 100-continue header for large requests where the\n> server offers GSSAPI authentication to avoid this issue, since the request would\n> otherwise fail.  This allows git to get the authentication data right before\n> sending the pack contents.  Existing cases where pushes would succeed, including\n> small requests using GSSAPI, still disable the use of 100 Continue, as it causes\n> problems for some remote HTTP implementations (servers and proxies).\n\nThis iteration looks very reasonable to me.\n\nOne minor nit:\n\n> +\t\tslot = get_active_slot();\n> +\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n> +\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n> +\t\t\tneeds_100_continue = 1;\n\nAccording to curl_easy_getinfo(3), CURLINFO_HTTPAUTH_AVAIL was\nintroduced in 7.10.8 (and it looks like CURLAUTH_GSSNEGOTIATE came\nearlier in 7.10.6). We should probably surround it with\n\n  #if LIBCURL_VERSION_NUM >= 0x070a08\n\n-Peff\n"}]}