From: Junio C Hamano Date: Tue, 17 Jan 2017 23:19:03 +0000 (-0800) Subject: Merge branch 'dt/smart-http-detect-server-going-away' into maint X-Git-Tag: v2.11.1~36 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/48d23c12e7329213e0dfd73bbffc6ad05efcaf9d?ds=inline;hp=-c Merge branch 'dt/smart-http-detect-server-going-away' into maint When the http server gives an incomplete response to a smart-http rpc call, it could lead to client waiting for a full response that will never come. Teach the client side to notice this condition and abort the transfer. An improvement counterproposal has failed. cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net> * dt/smart-http-detect-server-going-away: upload-pack: optionally allow fetching any sha1 remote-curl: don't hang when a server dies before any output --- 48d23c12e7329213e0dfd73bbffc6ad05efcaf9d diff --combined Documentation/config.txt index d51182a060,b7f9991cc7..febf95d6c6 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@@ -1891,16 -1891,6 +1891,16 @@@ http.userAgent: of common USER_AGENT strings (but not including those like git/1.7.1). Can be overridden by the `GIT_HTTP_USER_AGENT` environment variable. +http.followRedirects:: + Whether git should follow HTTP redirects. If set to `true`, git + will transparently follow any redirect issued by a server it + encounters. If set to `false`, git will treat all redirects as + errors. If set to `initial`, git will follow redirects only for + the initial request to a remote, but not for subsequent + follow-up HTTP requests. Since git uses the redirected URL as + the base for the follow-up requests, this is generally + sufficient. The default is `initial`. + http..*:: Any of the http.* options above can be applied selectively to some URLs. For a config key to match a URL, each element of the config key is @@@ -2971,6 -2961,11 +2971,11 @@@ uploadpack.allowReachableSHA1InWant: calculating object reachability is computationally expensive. Defaults to `false`. + uploadpack.allowAnySHA1InWant:: + Allow `upload-pack` to accept a fetch request that asks for any + object at all. + Defaults to `false`. + uploadpack.keepAlive:: When `upload-pack` has started `pack-objects`, there may be a quiet period while `pack-objects` prepares the pack. Normally diff --combined remote-curl.c index 28d9d10638,ee4423659f..34a97e7328 --- a/remote-curl.c +++ b/remote-curl.c @@@ -274,7 -274,7 +274,7 @@@ static struct discovery *discover_refs( struct strbuf effective_url = STRBUF_INIT; struct discovery *last = last_discovery; int http_ret, maybe_smart = 0; - struct http_get_options options; + struct http_get_options http_options; if (last && !strcmp(service, last->service)) return last; @@@ -291,16 -291,15 +291,16 @@@ strbuf_addf(&refs_url, "service=%s", service); } - memset(&options, 0, sizeof(options)); - options.content_type = &type; - options.charset = &charset; - options.effective_url = &effective_url; - options.base_url = &url; - options.no_cache = 1; - options.keep_error = 1; + memset(&http_options, 0, sizeof(http_options)); + http_options.content_type = &type; + http_options.charset = &charset; + http_options.effective_url = &effective_url; + http_options.base_url = &url; + http_options.initial_request = 1; + http_options.no_cache = 1; + http_options.keep_error = 1; - http_ret = http_get_strbuf(refs_url.buf, &buffer, &options); + http_ret = http_get_strbuf(refs_url.buf, &buffer, &http_options); switch (http_ret) { case HTTP_OK: break; @@@ -315,9 -314,6 +315,9 @@@ die("unable to access '%s': %s", url.buf, curl_errorstr); } + if (options.verbosity && !starts_with(refs_url.buf, url.buf)) + warning(_("redirecting to %s"), url.buf); + last= xcalloc(1, sizeof(*last_discovery)); last->service = service; last->buf_alloc = strbuf_detach(&buffer, &last->len); @@@ -404,6 -400,7 +404,7 @@@ struct rpc_state size_t pos; int in; int out; + int any_written; struct strbuf result; unsigned gzip_request : 1; unsigned initial_buffer : 1; @@@ -460,6 -457,8 +461,8 @@@ static size_t rpc_in(char *ptr, size_t { size_t size = eltsize * nmemb; struct rpc_state *rpc = buffer_; + if (size) + rpc->any_written = 1; write_or_die(rpc->in, ptr, size); return size; } @@@ -663,6 -662,8 +666,8 @@@ retry curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in); curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc); + + rpc->any_written = 0; err = run_slot(slot, NULL); if (err == HTTP_REAUTH && !large_request) { credential_fill(&http_auth); @@@ -671,6 -672,9 +676,9 @@@ if (err != HTTP_OK) err = -1; + if (!rpc->any_written) + err = -1; + curl_slist_free_all(headers); free(gzip_body); return err; diff --combined t/t5551-http-fetch-smart.sh index 6e5b9e42fb,8d3db405c0..a51b7e20d3 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@@ -119,10 -119,6 +119,10 @@@ test_expect_success 'redirects re-root git clone $HTTPD_URL/smart-redir-limited/repo.git repo-redir-limited ' +test_expect_success 're-rooting dies on insane schemes' ' + test_must_fail git clone $HTTPD_URL/insane-redir/repo.git insane +' + test_expect_success 'clone from password-protected repository' ' echo two >expect && set_askpass user@host pass@host && @@@ -280,6 -276,58 +280,58 @@@ test_expect_success 'large fetch-pack r test_line_count = 2 posts ' + test_expect_success 'test allowreachablesha1inwant' ' + test_when_finished "rm -rf test_reachable.git" && + server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + master_sha=$(git -C "$server" rev-parse refs/heads/master) && + git -C "$server" config uploadpack.allowreachablesha1inwant 1 && + + git init --bare test_reachable.git && + git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" && + git -C test_reachable.git fetch origin "$master_sha" + ' + + test_expect_success 'test allowreachablesha1inwant with unreachable' ' + test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" && + + #create unreachable sha + echo content >file2 && + git add file2 && + git commit -m two && + git push public HEAD:refs/heads/doomed && + git push public :refs/heads/doomed && + + server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + master_sha=$(git -C "$server" rev-parse refs/heads/master) && + git -C "$server" config uploadpack.allowreachablesha1inwant 1 && + + git init --bare test_reachable.git && + git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" && + test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" + ' + + test_expect_success 'test allowanysha1inwant with unreachable' ' + test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" && + + #create unreachable sha + echo content >file2 && + git add file2 && + git commit -m two && + git push public HEAD:refs/heads/doomed && + git push public :refs/heads/doomed && + + server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + master_sha=$(git -C "$server" rev-parse refs/heads/master) && + git -C "$server" config uploadpack.allowreachablesha1inwant 1 && + + git init --bare test_reachable.git && + git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" && + test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" && + + git -C "$server" config uploadpack.allowanysha1inwant 1 && + git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" + ' + test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' ' ( cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&