Skip to content

Commit

Permalink
remote-curl: retry failed requests for auth even with gzip
Browse files Browse the repository at this point in the history
Commit b81401c taught the post_rpc function to retry the
http request after prompting for credentials. However, it
did not handle two cases:

  1. If we have a large request, we do not retry. That's OK,
     since we would have sent a probe (with retry) already.

  2. If we are gzipping the request, we do not retry. That
     was considered OK, because the intended use was for
     push (e.g., listing refs is OK, but actually pushing
     objects is not), and we never gzip on push.

This patch teaches post_rpc to retry even a gzipped request.
This has two advantages:

  1. It is possible to configure a "half-auth" state for
     fetching, where the set of refs and their sha1s are
     advertised, but one cannot actually fetch objects.

     This is not a recommended configuration, as it leaks
     some information about what is in the repository (e.g.,
     an attacker can try brute-forcing possible content in
     your repository and checking whether it matches your
     branch sha1). However, it can be slightly more
     convenient, since a no-op fetch will not require a
     password at all.

  2. It future-proofs us should we decide to ever gzip more
     requests.

Signed-off-by: Jeff King <peff@peff.net>
  • Loading branch information
Jeff King committed Oct 31, 2012
1 parent df126e1 commit 2e736fd
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
11 changes: 10 additions & 1 deletion remote-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,15 @@ static int post_rpc(struct rpc_state *rpc)
fflush(stderr);
}

} else if (gzip_body) {
/*
* If we are looping to retry authentication, then the previous
* run will have set up the headers and gzip buffer already,
* and we just need to send it.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, gzip_size);

} else if (use_gzip && 1024 < rpc->len) {
/* The client backend isn't giving us compressed data so
* we can try to deflate it ourselves, this may save on.
Expand Down Expand Up @@ -530,7 +539,7 @@ static int post_rpc(struct rpc_state *rpc)
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);

err = run_slot(slot);
if (err == HTTP_REAUTH && !large_request && !use_gzip)
if (err == HTTP_REAUTH && !large_request)
goto retry;
if (err != HTTP_OK)
err = -1;
Expand Down
7 changes: 7 additions & 0 deletions t/lib-httpd/apache.conf
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ SSLEngine On
Require valid-user
</LocationMatch>

<LocationMatch "^/auth-fetch/.*/git-upload-pack$">
AuthType Basic
AuthName "git-auth"
AuthUserFile passwd
Require valid-user
</LocationMatch>

<IfDefine DAV>
LoadModule dav_module modules/mod_dav.so
LoadModule dav_fs_module modules/mod_dav_fs.so
Expand Down
15 changes: 15 additions & 0 deletions t/t5551-http-fetch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,21 @@ test_expect_success 'clone from auth-only-for-push repository' '
test_cmp expect actual
'

test_expect_success 'clone from auth-only-for-objects repository' '
echo two >expect &&
set_askpass user@host &&
git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth &&
expect_askpass both user@host &&
git --git-dir=half-auth log -1 --format=%s >actual &&
test_cmp expect actual
'

test_expect_success 'no-op half-auth fetch does not require a password' '
set_askpass wrong &&
git --git-dir=half-auth fetch &&
expect_askpass none
'

test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE

test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
Expand Down

0 comments on commit 2e736fd

Please sign in to comment.