Merge branch 'dt/smart-http-detect-server-going-away' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 17 Jan 2017 23:19:03 +0000 (15:19 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 17 Jan 2017 23:19:03 +0000 (15:19 -0800)
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

1  2 
Documentation/config.txt
remote-curl.c
t/t5551-http-fetch-smart.sh
diff --combined Documentation/config.txt
index d51182a0606aa30168d640d2d821a3ec1bc2458d,b7f9991cc782f259fd363595a5631995f5aafd1c..febf95d6c663d723a66a30d936bd49b1a02b467a
@@@ -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.<url>.*::
        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 28d9d1063880b9b7e6ec18ea18c18f77fb99ef32,ee4423659f5be499c8ef70c77ecfad14758a7b12..34a97e7328d440d48badd53bb0d3167a58558ee9
@@@ -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;
                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;
                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);
        if (err != HTTP_OK)
                err = -1;
  
+       if (!rpc->any_written)
+               err = -1;
        curl_slist_free_all(headers);
        free(gzip_body);
        return err;
index 6e5b9e42fb6d3f8c25c1a08388c4eb5ec3284500,8d3db405c0f5e1ee020d5fc8bc219860fdf02c3d..a51b7e20d32158d4b6609ba954e2ed59edaa6342
@@@ -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" &&