git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Kyle J. McKay" <mackyle@gmail.com>
Subject: [PATCH 9/9] remote-curl: rewrite base url from info/refs redirects
Date: Sat, 28 Sep 2013 04:35:35 -0400	[thread overview]
Message-ID: <20130928083535.GD2782@sigill.intra.peff.net> (raw)
In-Reply-To: <20130928082956.GA22610@sigill.intra.peff.net>

For efficiency and security reasons, an earlier commit in
this series taught http_get_* to re-write the base url based
on redirections we saw while making a specific request.

This commit wires that option into the info/refs request,
meaning that a redirect from

    http://example.com/foo.git/info/refs

to

    https://example.com/bar.git/info/refs

will behave as if "https://example.com/bar.git" had been
provided to git in the first place.

The tests bear some explanation. We introduce two new
hierearchies into the httpd test config:

  1. Requests to /smart-redir-limited will work only for the
     initial info/refs request, but not any subsequent
     requests. As a result, we can confirm whether the
     client is re-rooting its requests after the initial
     contact, since otherwise it will fail (it will ask for
     "repo.git/git-upload-pack", which is not redirected).

  2. Requests to smart-redir-auth will redirect, and require
     auth after the redirection. Since we are using the
     redirected base for further requests, we also update
     the credential struct, in order not to mislead the user
     (or credential helpers) about which credential is
     needed. We can therefore check the GIT_ASKPASS prompts
     to make sure we are prompting for the new location.
     Because we have neither multiple servers nor https
     support in our test setup, we can only redirect between
     paths, meaning we need to turn on
     credential.useHttpPath to see the difference.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c           |  4 ++++
 t/lib-httpd.sh          |  3 ++-
 t/lib-httpd/apache.conf |  2 ++
 t/t5551-http-fetch.sh   | 11 +++++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/remote-curl.c b/remote-curl.c
index 5add2cb..c9b891a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -207,6 +207,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
 	struct strbuf type = STRBUF_INIT;
 	struct strbuf buffer = STRBUF_INIT;
 	struct strbuf refs_url = STRBUF_INIT;
+	struct strbuf effective_url = STRBUF_INIT;
 	struct discovery *last = last_discovery;
 	int http_ret, maybe_smart = 0;
 	struct http_get_options options;
@@ -228,6 +229,8 @@ static struct discovery* discover_refs(const char *service, int for_push)
 
 	memset(&options, 0, sizeof(options));
 	options.content_type = &type;
+	options.effective_url = &effective_url;
+	options.base_url = &url;
 	options.no_cache = 1;
 	options.keep_error = 1;
 
@@ -287,6 +290,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
 	strbuf_release(&refs_url);
 	strbuf_release(&exp);
 	strbuf_release(&type);
+	strbuf_release(&effective_url);
 	strbuf_release(&buffer);
 	last_discovery = last;
 	return last;
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 54dbbfe..ad8f1ef 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -204,7 +204,8 @@ expect_askpass() {
 }
 
 expect_askpass() {
-	dest=$HTTPD_DEST
+	dest=$HTTPD_DEST${3+/$3}
+
 	{
 		case "$1" in
 		none)
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 397c480..3a03e82 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -110,6 +110,8 @@ RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
 RewriteEngine on
 RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
 RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
+RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301]
+RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301]
 
 <IfDefine SSL>
 LoadModule ssl_module modules/mod_ssl.so
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 8196af1..fb16833 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -113,6 +113,10 @@ test_expect_success 'follow redirects (302)' '
 	git clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t
 '
 
+test_expect_success 'redirects re-root further requests' '
+	git clone $HTTPD_URL/smart-redir-limited/repo.git repo-redir-limited
+'
+
 test_expect_success 'clone from password-protected repository' '
 	echo two >expect &&
 	set_askpass user@host &&
@@ -146,6 +150,13 @@ test_expect_success 'no-op half-auth fetch does not require a password' '
 	expect_askpass none
 '
 
+test_expect_success 'redirects send auth to new location' '
+	set_askpass user@host &&
+	git -c credential.useHttpPath=true \
+	  clone $HTTPD_URL/smart-redir-auth/repo.git repo-redir-auth &&
+	expect_askpass both user@host auth/smart/repo.git
+'
+
 test_expect_success 'disable dumb http on server' '
 	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
 		config http.getanyfile false
-- 
1.8.4.rc3.19.g9da5bf6

      parent reply	other threads:[~2013-09-28  8:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-28  8:29 [PATCH 0/9] following http redirects Jeff King
2013-09-28  8:31 ` [PATCH 1/9] http_get_file: style fixes Jeff King
2013-09-28  8:31 ` [PATCH 2/9] http_request: factor out curlinfo_strbuf Jeff King
2013-09-28  8:31 ` [PATCH 3/9] http: refactor options to http_get_* Jeff King
2013-09-28  8:31 ` [PATCH 4/9] http: hoist credential request out of handle_curl_result Jeff King
2013-09-28  8:32 ` [PATCH 5/9] http: provide effective url to callers Jeff King
2013-09-28  8:34 ` [PATCH 6/9] http: update base URLs when we see redirects Jeff King
2013-09-29 18:54   ` brian m. carlson
2013-09-29 19:26   ` Eric Sunshine
2013-09-29 19:32     ` Jeff King
2013-09-29 19:50       ` Eric Sunshine
2013-10-18 18:25   ` Junio C Hamano
2013-10-18 18:42     ` Jeff King
2013-10-18 19:20       ` Junio C Hamano
2013-09-28  8:35 ` [PATCH 7/9] remote-curl: make refs_url a strbuf Jeff King
2013-09-28  8:35 ` [PATCH 8/9] remote-curl: store url as " Jeff King
2013-09-28  8:35 ` 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

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

  git send-email \
    --in-reply-to=20130928083535.GD2782@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mackyle@gmail.com \
    /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.
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 NNTP newsgroup(s).