remote-curl: don't hang when a server dies before any output

In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

David Turner committed Nov 18, 2016 at 15:30 UTC 296b847c0d6de63353e236cfbf94163d24155529
2 files changed +38
remote-curl.c
+8
@@ -400,6 +400,7 @@ struct rpc_state {
400 size_t pos;
401 int in;
402 int out;
403 + int any_written;
404 struct strbuf result;
405 unsigned gzip_request : 1;
406 unsigned initial_buffer : 1;
@@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
457 {
458 size_t size = eltsize * nmemb;
459 struct rpc_state *rpc = buffer_;
460 + if (size)
461 + rpc->any_written = 1;
462 write_or_die(rpc->in, ptr, size);
463 return size;
464 }
@@ -659,6 +662,8 @@ retry:
662 curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
663 curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
664
665 +
666 + rpc->any_written = 0;
667 err = run_slot(slot, NULL);
668 if (err == HTTP_REAUTH && !large_request) {
669 credential_fill(&http_auth);
@@ -667,6 +672,9 @@ retry:
672 if (err != HTTP_OK)
673 err = -1;
674
675 + if (!rpc->any_written)
676 + err = -1;
677 +
678 curl_slist_free_all(headers);
679 free(gzip_body);
680 return err;
t/t5551-http-fetch-smart.sh
+30
@@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '
276 test_line_count = 2 posts
277 '
278
279 +test_expect_success 'test allowreachablesha1inwant' '
280 + test_when_finished "rm -rf test_reachable.git" &&
281 + server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
282 + master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
283 + git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
284 +
285 + git init --bare test_reachable.git &&
286 + git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
287 + git -C test_reachable.git fetch origin "$master_sha"
288 +'
289 +
290 +test_expect_success 'test allowreachablesha1inwant with unreachable' '
291 + test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
292 +
293 + #create unreachable sha
294 + echo content >file2 &&
295 + git add file2 &&
296 + git commit -m two &&
297 + git push public HEAD:refs/heads/doomed &&
298 + git push public :refs/heads/doomed &&
299 +
300 + server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
301 + master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
302 + git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
303 +
304 + git init --bare test_reachable.git &&
305 + git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
306 + test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
307 +'
308 +
309 test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
310 (
311 cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&