Merge branch 'dt/smart-http-detect-server-going-away'
authorJunio C Hamano <gitster@pobox.com>
Tue, 10 Jan 2017 23:24:25 +0000 (15:24 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 10 Jan 2017 23:24:25 +0000 (15:24 -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 4a991f35828dc8ed6037cf931bc1f5b140bef22d,b7f9991cc782f259fd363595a5631995f5aafd1c..1abf078c4e1c79ee11b05c2a26d83ba28c809a66
@@@ -1409,9 -1409,7 +1409,9 @@@ gc.pruneExpire:
        Override the grace period with this config variable.  The value
        "now" may be used to disable this grace period and always prune
        unreachable objects immediately, or "never" may be used to
 -      suppress pruning.
 +      suppress pruning.  This feature helps prevent corruption when
 +      'git gc' runs concurrently with another process writing to the
 +      repository; see the "NOTES" section of linkgit:git-gc[1].
  
  gc.worktreePruneExpire::
        When 'git gc' is run, it calls
@@@ -1893,16 -1891,6 +1893,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
@@@ -2320,52 -2308,6 +2320,52 @@@ pretty.<name>:
        Note that an alias with the same name as a built-in format
        will be silently ignored.
  
 +protocol.allow::
 +      If set, provide a user defined default policy for all protocols which
 +      don't explicitly have a policy (`protocol.<name>.allow`).  By default,
 +      if unset, known-safe protocols (http, https, git, ssh, file) have a
 +      default policy of `always`, known-dangerous protocols (ext) have a
 +      default policy of `never`, and all other protocols have a default
 +      policy of `user`.  Supported policies:
 ++
 +--
 +
 +* `always` - protocol is always able to be used.
 +
 +* `never` - protocol is never able to be used.
 +
 +* `user` - protocol is only able to be used when `GIT_PROTOCOL_FROM_USER` is
 +  either unset or has a value of 1.  This policy should be used when you want a
 +  protocol to be directly usable by the user but don't want it used by commands which
 +  execute clone/fetch/push commands without user input, e.g. recursive
 +  submodule initialization.
 +
 +--
 +
 +protocol.<name>.allow::
 +      Set a policy to be used by protocol `<name>` with clone/fetch/push
 +      commands. See `protocol.allow` above for the available policies.
 ++
 +The protocol names currently used by git are:
 ++
 +--
 +  - `file`: any local file-based path (including `file://` URLs,
 +    or local paths)
 +
 +  - `git`: the anonymous git protocol over a direct TCP
 +    connection (or proxy, if configured)
 +
 +  - `ssh`: git over ssh (including `host:path` syntax,
 +    `ssh://`, etc).
 +
 +  - `http`: git over http, both "smart http" and "dumb http".
 +    Note that this does _not_ include `https`; if you want to configure
 +    both, you must do so individually.
 +
 +  - any external helpers are named by their protocol (e.g., use
 +    `hg` to allow the `git-remote-hg` helper)
 +--
 +
  pull.ff::
        By default, Git does not create an extra merge commit when merging
        a commit that is a descendant of the current commit. Instead, the
@@@ -2988,11 -2930,6 +2988,11 @@@ is omitted from the advertisements but 
  `refs/namespaces/bar/refs/heads/master` are still advertised as so-called
  "have" lines. In order to match refs before stripping, add a `^` in front of
  the ref name. If you combine `!` and `^`, `!` must be specified first.
 ++
 +Even if you hide refs, a client may still be able to steal the target
 +objects via the techniques described in the "SECURITY" section of the
 +linkgit:gitnamespaces[7] man page; it's best to keep private data in a
 +separate repository.
  
  transfer.unpackLimit::
        When `fetch.unpackLimit` or `receive.unpackLimit` are
  uploadarchive.allowUnreachable::
        If true, allow clients to use `git archive --remote` to request
        any tree, whether reachable from the ref tips or not. See the
 -      discussion in the `SECURITY` section of
 +      discussion in the "SECURITY" section of
        linkgit:git-upload-archive[1] for more details. Defaults to
        `false`.
  
@@@ -3016,20 -2953,19 +3016,25 @@@ uploadpack.allowTipSHA1InWant:
        When `uploadpack.hideRefs` is in effect, allow `upload-pack`
        to accept a fetch request that asks for an object at the tip
        of a hidden ref (by default, such a request is rejected).
 -      see also `uploadpack.hideRefs`.
 +      See also `uploadpack.hideRefs`.  Even if this is false, a client
 +      may be able to steal objects via the techniques described in the
 +      "SECURITY" section of the linkgit:gitnamespaces[7] man page; it's
 +      best to keep private data in a separate repository.
  
  uploadpack.allowReachableSHA1InWant::
        Allow `upload-pack` to accept a fetch request that asks for an
        object that is reachable from any ref tip. However, note that
        calculating object reachability is computationally expensive.
 -      Defaults to `false`.
 +      Defaults to `false`.  Even if this is false, a client may be able
 +      to steal objects via the techniques described in the "SECURITY"
 +      section of the linkgit:gitnamespaces[7] man page; it's best to
 +      keep private data in a separate repository.
  
+ 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" &&