From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Kyle J. McKay" <mackyle@gmail.com>
Subject: [PATCH 6/9] http: update base URLs when we see redirects
Date: Sat, 28 Sep 2013 04:34:05 -0400 [thread overview]
Message-ID: <20130928083405.GA2782@sigill.intra.peff.net> (raw)
In-Reply-To: <20130928082956.GA22610@sigill.intra.peff.net>
If a caller asks the http_get_* functions to go to a
particular URL and we end up elsewhere due to a redirect,
the effective_url field can tell us where we went.
It would be nice to remember this redirect and short-cut
further requests for two reasons:
1. It's more efficient. Otherwise we spend an extra http
round-trip to the server for each subsequent request,
just to get redirected.
2. If we end up with an http 401 and are going to ask for
credentials, it is to feed them to the redirect target.
If the redirect is an http->https upgrade, this means
our credentials may be provided on the http leg, just
to end up redirected to https. And if the redirect
crosses server boundaries, then curl will drop the
credentials entirely as it follows the redirect.
However, it, it is not enough to simply record the effective
URL we saw and use that for subsequent requests. We were
originally fed a "base" url like:
http://example.com/foo.git
and we want to figure out what the new base is, even though
the URLs we see may be:
original: http://example.com/foo.git/info/refs
effective: http://example.com/bar.git/info/refs
Subsequent requests will not be for "info/refs", but for
other paths relative to the base. We must ask the caller to
pass in the original base, and we must pass the redirected
base back to the caller (so that it can generte more URLs
from it). Furthermore, we need to feed the new base to the
credential code, so that requests to credential helpers (or
to the user) match the URL we will be requesting.
This patch teaches http_request_reauth to do this munging.
Since it is the caller who cares about making more URLs, it
seems at first glance that callers could simply check
effective_url themselves and handle it. However, since we
need to update the credential struct before the second
re-auth request, we have to do it inside http_request_reauth.
Signed-off-by: Jeff King <peff@peff.net>
---
http.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
http.h | 8 ++++++++
2 files changed, 68 insertions(+)
diff --git a/http.c b/http.c
index 65a0048..8775b5c 100644
--- a/http.c
+++ b/http.c
@@ -921,11 +921,71 @@ static int http_request_reauth(const char *url,
return ret;
}
+/*
+ * Update the "base" url to a more appropriate value, as deduced by
+ * redirects seen when requesting a URL starting with "url".
+ *
+ * The "asked" parameter is a URL that we asked curl to access, and must begin
+ * with "base".
+ *
+ * The "got" parameter is the URL that curl reported to us as where we ended
+ * up.
+ *
+ * Returns 1 if we updated the base url, 0 otherwise.
+ *
+ * Our basic strategy is to compare "base" and "asked" to find the bits
+ * specific to our request. We then strip those bits off of "got" to yield the
+ * new base. So for example, if our base is "http://example.com/foo.git",
+ * and we ask for "http://example.com/foo.git/info/refs", we might end up
+ * with "https://other.example.com/foo.git/info/refs". We would want the
+ * new URL to become "https://other.example.com/foo.git".
+ *
+ * Note that this assumes a sane redirect scheme. It's entirely possible
+ * in the example above to end up at a URL that does not even end in
+ * "info/refs". In such a case we simply punt, as there is not much we can
+ * do (and such a scheme is unlikely to represent a real git repository,
+ * which means we are likely about to abort anyway).
+ */
+static int update_url_from_redirect(struct strbuf *base,
+ const char *asked,
+ const struct strbuf *got)
+{
+ const char *tail;
+ size_t tail_len;
+
+ if (!strcmp(asked, got->buf))
+ return 0;
+
+ if (strncmp(asked, base->buf, base->len))
+ die("BUG: update_url_from_redirect: %s is not a superset of %s",
+ asked, base->buf);
+
+ tail = asked + base->len;
+ tail_len = strlen(tail);
+
+ if (got->len < tail_len ||
+ strcmp(tail, got->buf + got->len - tail_len))
+ return 0; /* insane redirect scheme */
+
+ strbuf_reset(base);
+ strbuf_add(base, got->buf, got->len - tail_len);
+ return 1;
+}
+
static int http_request_reauth(const char *url,
void *result, int target,
struct http_get_options *options)
{
int ret = http_request(url, result, target, options);
+
+ if (options && options->effective_url && options->base_url) {
+ if (update_url_from_redirect(options->base_url,
+ url, options->effective_url)) {
+ credential_from_url(&http_auth, options->base_url->buf);
+ url = options->effective_url->buf;
+ }
+ }
+
if (ret != HTTP_REAUTH)
return ret;
diff --git a/http.h b/http.h
index 974ede7..12ca5c8 100644
--- a/http.h
+++ b/http.h
@@ -139,6 +139,14 @@ struct http_get_options {
* redirects we followed.
*/
struct strbuf *effective_url;
+
+ /*
+ * If both base_url and effective_url are non-NULL, the base URL will
+ * be munged to reflect any redirections going from the requested url
+ * to effective_url. See the definition of update_url_from_redirect
+ * for details.
+ */
+ struct strbuf *base_url;
};
/* Return values for http_get_*() */
--
1.8.4.rc3.19.g9da5bf6
next prev parent reply other threads:[~2013-09-28 8:34 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 ` Jeff King [this message]
2013-09-29 18:54 ` [PATCH 6/9] http: update base URLs when we see redirects 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 ` [PATCH 9/9] remote-curl: rewrite base url from info/refs redirects Jeff King
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=20130928083405.GA2782@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).