Re: [PATCH v3] remote-curl: fix large pushes with GSSAPI

[Jeff King]

On Sat, Oct 26, 2013 at 10:34:42PM +, brian m. carlson wrote:

 Enable the use of the Expect: 100-continue header for large requests where the
 server offers GSSAPI authentication to avoid this issue, since the request 
 would
 otherwise fail.  This allows git to get the authentication data right before
 sending the pack contents.  Existing cases where pushes would succeed, 
 including
 small requests using GSSAPI, still disable the use of 100 Continue, as it 
 causes
 problems for some remote HTTP implementations (servers and proxies).

This iteration looks very reasonable to me.

One minor nit:

 + slot = get_active_slot();
 + curl_easy_getinfo(slot-curl, CURLINFO_HTTPAUTH_AVAIL, 
 authtype);
 + if (authtype  CURLAUTH_GSSNEGOTIATE)
 + needs_100_continue = 1;

According to curl_easy_getinfo(3), CURLINFO_HTTPAUTH_AVAIL was
introduced in 7.10.8 (and it looks like CURLAUTH_GSSNEGOTIATE came
earlier in 7.10.6). We should probably surround it with

  #if LIBCURL_VERSION_NUM = 0x070a08

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] remote-curl: fix large pushes with GSSAPI

[brian m. carlson]

Due to an interaction between the way libcurl handles GSSAPI authentication over
HTTP and the way git uses libcurl, large pushes (those over http.postBuffer
bytes) would fail due to an authentication failure requiring a rewind of the
curl buffer.  Such a rewind was not possible because the data did not fit into
the entire buffer.

Enable the use of the Expect: 100-continue header for large requests where the
server offers GSSAPI authentication to avoid this issue, since the request would
otherwise fail.  This allows git to get the authentication data right before
sending the pack contents.  Existing cases where pushes would succeed, including
small requests using GSSAPI, still disable the use of 100 Continue, as it causes
problems for some remote HTTP implementations (servers and proxies).

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 remote-curl.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/remote-curl.c b/remote-curl.c
index c9b891a..35698e7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -449,6 +449,7 @@ static int post_rpc(struct rpc_state *rpc)
char *gzip_body = NULL;
size_t gzip_size = 0;
int err, large_request = 0;
+   int needs_100_continue = 0;
 
/* Try to load the entire request, if we can fit it into the
 * allocated buffer space we can use HTTP/1.0 and avoid the
@@ -472,6 +473,8 @@ static int post_rpc(struct rpc_state *rpc)
}
 
if (large_request) {
+   long authtype = 0;
+
do {
err = probe_rpc(rpc);
if (err == HTTP_REAUTH)
@@ -479,11 +482,17 @@ static int post_rpc(struct rpc_state *rpc)
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
+
+   slot = get_active_slot();
+   curl_easy_getinfo(slot-curl, CURLINFO_HTTPAUTH_AVAIL, 
authtype);
+   if (authtype  CURLAUTH_GSSNEGOTIATE)
+   needs_100_continue = 1;
}
 
headers = curl_slist_append(headers, rpc-hdr_content_type);
headers = curl_slist_append(headers, rpc-hdr_accept);
-   headers = curl_slist_append(headers, Expect:);
+   headers = curl_slist_append(headers, needs_100_continue ?
+   Expect: 100-continue : Expect:);
 
 retry:
slot = get_active_slot();
-- 
1.8.4.1.635.g6a5

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html