summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar Junio C Hamano <gitster@pobox.com>2013-10-30 12:09:53 -0700
committerLibravatar Junio C Hamano <gitster@pobox.com>2013-10-30 12:09:53 -0700
commit177f0a400955f2345c73a5c8b0459d617429ffd9 (patch)
treebefe6d8d3e767c1dc6bc06b3a8557d44a5faaefe
parentAlmost -rc0 for 1.8.5 (diff)
parenthttp.c: Spell the null pointer as NULL (diff)
downloadtgif-177f0a400955f2345c73a5c8b0459d617429ffd9.tar.xz
Merge branch 'jk/http-auth-redirects'
Handle the case where http transport gets redirected during the authorization request better. * jk/http-auth-redirects: http.c: Spell the null pointer as NULL remote-curl: rewrite base url from info/refs redirects remote-curl: store url as a strbuf remote-curl: make refs_url a strbuf http: update base URLs when we see redirects http: provide effective url to callers http: hoist credential request out of handle_curl_result http: refactor options to http_get_* http_request: factor out curlinfo_strbuf http_get_file: style fixes
-rw-r--r--http-push.c4
-rw-r--r--http.c137
-rw-r--r--http.h30
-rw-r--r--remote-curl.c69
-rw-r--r--t/lib-httpd.sh3
-rw-r--r--t/lib-httpd/apache.conf2
-rwxr-xr-xt/t5551-http-fetch.sh11
7 files changed, 191 insertions, 65 deletions
diff --git a/http-push.c b/http-push.c
index 69200baf76..34cb70f90e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1542,7 +1542,7 @@ static int remote_exists(const char *path)
sprintf(url, "%s%s", repo->url, path);
- switch (http_get_strbuf(url, NULL, NULL, 0)) {
+ switch (http_get_strbuf(url, NULL, NULL)) {
case HTTP_OK:
ret = 1;
break;
@@ -1566,7 +1566,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
url = xmalloc(strlen(repo->url) + strlen(path) + 1);
sprintf(url, "%s%s", repo->url, path);
- if (http_get_strbuf(url, NULL, &buffer, 0) != HTTP_OK)
+ if (http_get_strbuf(url, &buffer, NULL) != HTTP_OK)
die("Couldn't get %s for remote symref\n%s", url,
curl_errorstr);
free(url);
diff --git a/http.c b/http.c
index 0ddb164a6f..bcf54aa35f 100644
--- a/http.c
+++ b/http.c
@@ -47,7 +47,7 @@ static int curl_ftp_no_epsv;
static const char *curl_http_proxy;
static const char *curl_cookie_file;
static int curl_save_cookies;
-static struct credential http_auth = CREDENTIAL_INIT;
+struct credential http_auth = CREDENTIAL_INIT;
static int http_proactive_auth;
static const char *user_agent;
@@ -861,7 +861,6 @@ int handle_curl_result(struct slot_results *results)
credential_reject(&http_auth);
return HTTP_NOAUTH;
} else {
- credential_fill(&http_auth);
return HTTP_REAUTH;
}
} else {
@@ -875,12 +874,25 @@ int handle_curl_result(struct slot_results *results)
}
}
+static CURLcode curlinfo_strbuf(CURL *curl, CURLINFO info, struct strbuf *buf)
+{
+ char *ptr;
+ CURLcode ret;
+
+ strbuf_reset(buf);
+ ret = curl_easy_getinfo(curl, info, &ptr);
+ if (!ret && ptr)
+ strbuf_addstr(buf, ptr);
+ return ret;
+}
+
/* http_request() targets */
#define HTTP_REQUEST_STRBUF 0
#define HTTP_REQUEST_FILE 1
-static int http_request(const char *url, struct strbuf *type,
- void *result, int target, int options)
+static int http_request(const char *url,
+ void *result, int target,
+ const struct http_get_options *options)
{
struct active_request_slot *slot;
struct slot_results results;
@@ -913,9 +925,9 @@ static int http_request(const char *url, struct strbuf *type,
}
strbuf_addstr(&buf, "Pragma:");
- if (options & HTTP_NO_CACHE)
+ if (options && options->no_cache)
strbuf_addstr(&buf, " no-cache");
- if (options & HTTP_KEEP_ERROR)
+ if (options && options->keep_error)
curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
headers = curl_slist_append(headers, buf.buf);
@@ -933,13 +945,13 @@ static int http_request(const char *url, struct strbuf *type,
ret = HTTP_START_FAILED;
}
- if (type) {
- char *t;
- strbuf_reset(type);
- curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t);
- if (t)
- strbuf_addstr(type, t);
- }
+ if (options && options->content_type)
+ curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE,
+ options->content_type);
+
+ if (options && options->effective_url)
+ curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL,
+ options->effective_url);
curl_slist_free_all(headers);
strbuf_release(&buf);
@@ -947,12 +959,71 @@ static int http_request(const char *url, struct strbuf *type,
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 (prefixcmp(asked, base->buf))
+ 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,
- struct strbuf *type,
void *result, int target,
- int options)
+ struct http_get_options *options)
{
- int ret = http_request(url, type, result, target, 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;
@@ -962,7 +1033,7 @@ static int http_request_reauth(const char *url,
* making our next request. We only know how to do this for
* the strbuf case, but that is enough to satisfy current callers.
*/
- if (options & HTTP_KEEP_ERROR) {
+ if (options && options->keep_error) {
switch (target) {
case HTTP_REQUEST_STRBUF:
strbuf_reset(result);
@@ -971,15 +1042,17 @@ static int http_request_reauth(const char *url,
die("BUG: HTTP_KEEP_ERROR is only supported with strbufs");
}
}
- return http_request(url, type, result, target, options);
+
+ credential_fill(&http_auth);
+
+ return http_request(url, result, target, options);
}
int http_get_strbuf(const char *url,
- struct strbuf *type,
- struct strbuf *result, int options)
+ struct strbuf *result,
+ struct http_get_options *options)
{
- return http_request_reauth(url, type, result,
- HTTP_REQUEST_STRBUF, options);
+ return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
}
/*
@@ -988,7 +1061,8 @@ int http_get_strbuf(const char *url,
* If a previous interrupted download is detected (i.e. a previous temporary
* file is still around) the download is resumed.
*/
-static int http_get_file(const char *url, const char *filename, int options)
+static int http_get_file(const char *url, const char *filename,
+ struct http_get_options *options)
{
int ret;
struct strbuf tmpfile = STRBUF_INIT;
@@ -996,16 +1070,16 @@ static int http_get_file(const char *url, const char *filename, int options)
strbuf_addf(&tmpfile, "%s.temp", filename);
result = fopen(tmpfile.buf, "a");
- if (! result) {
+ if (!result) {
error("Unable to open local file %s", tmpfile.buf);
ret = HTTP_ERROR;
goto cleanup;
}
- ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, options);
+ ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
fclose(result);
- if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
+ if (ret == HTTP_OK && move_temp_to_file(tmpfile.buf, filename))
ret = HTTP_ERROR;
cleanup:
strbuf_release(&tmpfile);
@@ -1014,12 +1088,15 @@ cleanup:
int http_fetch_ref(const char *base, struct ref *ref)
{
+ struct http_get_options options = {0};
char *url;
struct strbuf buffer = STRBUF_INIT;
int ret = -1;
+ options.no_cache = 1;
+
url = quote_ref_url(base, ref->name);
- if (http_get_strbuf(url, NULL, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
+ if (http_get_strbuf(url, &buffer, &options) == HTTP_OK) {
strbuf_rtrim(&buffer);
if (buffer.len == 40)
ret = get_sha1_hex(buffer.buf, ref->old_sha1);
@@ -1050,7 +1127,7 @@ static char *fetch_pack_index(unsigned char *sha1, const char *base_url)
strbuf_addf(&buf, "%s.temp", sha1_pack_index_name(sha1));
tmp = strbuf_detach(&buf, NULL);
- if (http_get_file(url, tmp, 0) != HTTP_OK) {
+ if (http_get_file(url, tmp, NULL) != HTTP_OK) {
error("Unable to get pack index %s", url);
free(tmp);
tmp = NULL;
@@ -1103,6 +1180,7 @@ add_pack:
int http_get_info_packs(const char *base_url, struct packed_git **packs_head)
{
+ struct http_get_options options = {0};
int ret = 0, i = 0;
char *url, *data;
struct strbuf buf = STRBUF_INIT;
@@ -1112,7 +1190,8 @@ int http_get_info_packs(const char *base_url, struct packed_git **packs_head)
strbuf_addstr(&buf, "objects/info/packs");
url = strbuf_detach(&buf, NULL);
- ret = http_get_strbuf(url, NULL, &buf, HTTP_NO_CACHE);
+ options.no_cache = 1;
+ ret = http_get_strbuf(url, &buf, &options);
if (ret != HTTP_OK)
goto cleanup;
diff --git a/http.h b/http.h
index d77c1b54f2..12ca5c892d 100644
--- a/http.h
+++ b/http.h
@@ -102,6 +102,7 @@ extern void http_cleanup(void);
extern int active_requests;
extern int http_is_verbose;
extern size_t http_post_buffer;
+extern struct credential http_auth;
extern char curl_errorstr[CURL_ERROR_SIZE];
@@ -125,11 +126,30 @@ extern void append_remote_object_url(struct strbuf *buf, const char *url,
extern char *get_remote_object_url(const char *url, const char *hex,
int only_two_digit_prefix);
-/* Options for http_request_*() */
-#define HTTP_NO_CACHE 1
-#define HTTP_KEEP_ERROR 2
+/* Options for http_get_*() */
+struct http_get_options {
+ unsigned no_cache:1,
+ keep_error:1;
+
+ /* If non-NULL, returns the content-type of the response. */
+ struct strbuf *content_type;
+
+ /*
+ * If non-NULL, returns the URL we ended up at, including any
+ * 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_request_*() */
+/* Return values for http_get_*() */
#define HTTP_OK 0
#define HTTP_MISSING_TARGET 1
#define HTTP_ERROR 2
@@ -142,7 +162,7 @@ extern char *get_remote_object_url(const char *url, const char *hex,
*
* If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
*/
-int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options);
+int http_get_strbuf(const char *url, struct strbuf *result, struct http_get_options *options);
extern int http_fetch_ref(const char *base, struct ref *ref);
diff --git a/remote-curl.c b/remote-curl.c
index b5ebe01800..c9b891adbf 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -9,9 +9,11 @@
#include "string-list.h"
#include "sideband.h"
#include "argv-array.h"
+#include "credential.h"
static struct remote *remote;
-static const char *url; /* always ends with a trailing slash */
+/* always ends with a trailing slash */
+static struct strbuf url = STRBUF_INIT;
struct options {
int verbosity;
@@ -130,7 +132,8 @@ static struct ref *parse_info_refs(struct discovery *heads)
mid = &data[i];
if (data[i] == '\n') {
if (mid - start != 40)
- die("%sinfo/refs not valid: is this a git repository?", url);
+ die("%sinfo/refs not valid: is this a git repository?",
+ url.buf);
data[i] = 0;
ref_name = mid + 1;
ref = xmalloc(sizeof(struct ref) +
@@ -149,7 +152,7 @@ static struct ref *parse_info_refs(struct discovery *heads)
}
ref = alloc_ref("HEAD");
- if (!http_fetch_ref(url, ref) &&
+ if (!http_fetch_ref(url.buf, ref) &&
!resolve_remote_symref(ref, refs)) {
ref->next = refs;
refs = ref;
@@ -203,40 +206,47 @@ static struct discovery* discover_refs(const char *service, int for_push)
struct strbuf exp = STRBUF_INIT;
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;
- char *refs_url;
int http_ret, maybe_smart = 0;
+ struct http_get_options options;
if (last && !strcmp(service, last->service))
return last;
free_discovery(last);
- strbuf_addf(&buffer, "%sinfo/refs", url);
- if ((!prefixcmp(url, "http://") || !prefixcmp(url, "https://")) &&
+ strbuf_addf(&refs_url, "%sinfo/refs", url.buf);
+ if ((!prefixcmp(url.buf, "http://") || !prefixcmp(url.buf, "https://")) &&
git_env_bool("GIT_SMART_HTTP", 1)) {
maybe_smart = 1;
- if (!strchr(url, '?'))
- strbuf_addch(&buffer, '?');
+ if (!strchr(url.buf, '?'))
+ strbuf_addch(&refs_url, '?');
else
- strbuf_addch(&buffer, '&');
- strbuf_addf(&buffer, "service=%s", service);
+ strbuf_addch(&refs_url, '&');
+ strbuf_addf(&refs_url, "service=%s", service);
}
- refs_url = strbuf_detach(&buffer, NULL);
- http_ret = http_get_strbuf(refs_url, &type, &buffer,
- HTTP_NO_CACHE | HTTP_KEEP_ERROR);
+ 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;
+
+ http_ret = http_get_strbuf(refs_url.buf, &buffer, &options);
switch (http_ret) {
case HTTP_OK:
break;
case HTTP_MISSING_TARGET:
show_http_message(&type, &buffer);
- die("repository '%s' not found", url);
+ die("repository '%s' not found", url.buf);
case HTTP_NOAUTH:
show_http_message(&type, &buffer);
- die("Authentication failed for '%s'", url);
+ die("Authentication failed for '%s'", url.buf);
default:
show_http_message(&type, &buffer);
- die("unable to access '%s': %s", url, curl_errorstr);
+ die("unable to access '%s': %s", url.buf, curl_errorstr);
}
last= xcalloc(1, sizeof(*last_discovery));
@@ -277,9 +287,10 @@ static struct discovery* discover_refs(const char *service, int for_push)
else
last->refs = parse_info_refs(last);
- free(refs_url);
+ strbuf_release(&refs_url);
strbuf_release(&exp);
strbuf_release(&type);
+ strbuf_release(&effective_url);
strbuf_release(&buffer);
last_discovery = last;
return last;
@@ -463,6 +474,8 @@ static int post_rpc(struct rpc_state *rpc)
if (large_request) {
do {
err = probe_rpc(rpc);
+ if (err == HTTP_REAUTH)
+ credential_fill(&http_auth);
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
@@ -562,8 +575,10 @@ retry:
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
err = run_slot(slot);
- if (err == HTTP_REAUTH && !large_request)
+ if (err == HTTP_REAUTH && !large_request) {
+ credential_fill(&http_auth);
goto retry;
+ }
if (err != HTTP_OK)
err = -1;
@@ -598,7 +613,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
rpc->out = client.out;
strbuf_init(&rpc->result, 0);
- strbuf_addf(&buf, "%s%s", url, svc);
+ strbuf_addf(&buf, "%s%s", url.buf, svc);
rpc->service_url = strbuf_detach(&buf, NULL);
strbuf_addf(&buf, "Content-Type: application/x-%s-request", svc);
@@ -650,7 +665,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
for (i = 0; i < nr_heads; i++)
targets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
- walker = get_http_walker(url);
+ walker = get_http_walker(url.buf);
walker->get_all = 1;
walker->get_tree = 1;
walker->get_history = 1;
@@ -697,7 +712,7 @@ static int fetch_git(struct discovery *heads,
depth_arg = strbuf_detach(&buf, NULL);
argv[argc++] = depth_arg;
}
- argv[argc++] = url;
+ argv[argc++] = url.buf;
argv[argc++] = NULL;
for (i = 0; i < nr_heads; i++) {
@@ -795,7 +810,7 @@ static int push_dav(int nr_spec, char **specs)
argv[argc++] = "--dry-run";
if (options.verbosity > 1)
argv[argc++] = "--verbose";
- argv[argc++] = url;
+ argv[argc++] = url.buf;
for (i = 0; i < nr_spec; i++)
argv[argc++] = specs[i];
argv[argc++] = NULL;
@@ -828,7 +843,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
argv_array_push(&args, options.progress ? "--progress" : "--no-progress");
for_each_string_list_item(cas_option, &cas_options)
argv_array_push(&args, cas_option->string);
- argv_array_push(&args, url);
+ argv_array_push(&args, url.buf);
for (i = 0; i < nr_spec; i++)
argv_array_push(&args, specs[i]);
@@ -909,14 +924,12 @@ int main(int argc, const char **argv)
remote = remote_get(argv[1]);
if (argc > 2) {
- end_url_with_slash(&buf, argv[2]);
+ end_url_with_slash(&url, argv[2]);
} else {
- end_url_with_slash(&buf, remote->url[0]);
+ end_url_with_slash(&url, remote->url[0]);
}
- url = strbuf_detach(&buf, NULL);
-
- http_init(remote, url, 0);
+ http_init(remote, url.buf, 0);
do {
if (strbuf_getline(&buf, stdin, '\n') == EOF) {
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 54dbbfe5ce..ad8f1ef71e 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -204,7 +204,8 @@ set_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 397c480401..3a03e8263d 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -110,6 +110,8 @@ ScriptAlias /broken_smart/ broken-smart-http.sh/
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 8196af19f6..fb16833f76 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