git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: 乙酸鋰 <ch3cooli@gmail.com>
Cc: git@vger.kernel.org
Subject: [PATCH 2/2] remote-curl: retry failed requests for auth even with gzip
Date: Wed, 31 Oct 2012 08:01:12 -0400	[thread overview]
Message-ID: <20121031120112.GB21011@sigill.intra.peff.net> (raw)
In-Reply-To: <20121031115346.GC30879@sigill.intra.peff.net>

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>
---
I doubt we would ever want to gzip push requests. The bulk of the data
is objects, which are already compressed. In theory we could gzip the
non-pack parts, but that would first mean splitting them into a separate
request. The tiny bit of savings are almost certainly not worth the
complexity.

But the future-proofing might still be valuable if we ever modify the
protocol to have a new phase.

 remote-curl.c           | 11 ++++++++++-
 t/lib-httpd/apache.conf |  7 +++++++
 t/t5551-http-fetch.sh   | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/remote-curl.c b/remote-curl.c
index 10cd47d..fac2bef 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -474,6 +474,15 @@ retry:
 			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.
@@ -530,7 +539,7 @@ retry:
 	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;
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index ec8618d..15a3c71 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -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
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 7380f2a..5f174da 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -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' '
-- 
1.8.0.207.gdf2154c

      parent reply	other threads:[~2012-10-31 12:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-30 14:35 git smart-http do not authent to allow git ls-remote to be called anonymously 乙酸鋰
2012-10-01  0:53 ` Shawn Pearce
2012-10-01  1:09 ` Jeff King
     [not found]   ` <CAHtLG6QFu1rOfUeWREwVG540WvXtM1SnH6aHEJ9dKLzwNxbkVg@mail.gmail.com>
     [not found]     ` <CAHtLG6T=hFsSy=ScRK2cYBoUcmAG_tsg12FiFMTvzpHGmPTzfg@mail.gmail.com>
2012-10-14  6:30       ` Jeff King
     [not found]         ` <CAHtLG6QR4CtC3RkVE3FQXhrZPJem6SZbrJFyn9K_4yHzhzYt1Q@mail.gmail.com>
     [not found]           ` <20121026134907.GK1455@sigill.intra.peff.net>
     [not found]             ` <CAHtLG6SsMgwquaD_Rd0YvR9Ues-u1ktEdOXC2MAfEM9Naac=5g@mail.gmail.com>
2012-10-31 11:53               ` Jeff King
2012-10-31 11:55                 ` [PATCH 1/2] remote-curl: hoist gzip buffer size to top of post_rpc Jeff King
2012-10-31 12:01                 ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121031120112.GB21011@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=ch3cooli@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).