Merge branch 'jk/http-auth-redirects' into maint
authorJunio C Hamano <gitster@pobox.com>
Fri, 8 Nov 2013 19:37:25 +0000 (11:37 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 8 Nov 2013 19:37:26 +0000 (11:37 -0800)
We did not handle cases where http transport gets redirected during
the authorization request (e.g. from http:// to https://).

* 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

http-push.c
http.c
http.h
remote-curl.c
t/lib-httpd.sh
t/lib-httpd/apache.conf
t/t5551-http-fetch.sh
index cde6416d37751fe0724e7c021e858530a74cf466..b1e74d2086b8e0dee261358622016f6613249fcf 100644 (file)
@@ -1543,7 +1543,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;
@@ -1567,7 +1567,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 2d086aedfac9a8ea384c53f3b1963149b880ed31..3447945ee4f4e0e4b273d979ea1805b4c0f63b4c 100644 (file)
--- a/http.c
+++ b/http.c
@@ -45,7 +45,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
-static struct credential http_auth = CREDENTIAL_INIT;
+struct credential http_auth = CREDENTIAL_INIT;
 static int http_proactive_auth;
 static const char *user_agent;
 
@@ -806,7 +806,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 {
@@ -820,12 +819,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;
@@ -858,9 +870,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);
@@ -878,13 +890,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);
@@ -892,12 +904,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;
 
@@ -907,7 +978,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);
@@ -916,15 +987,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);
 }
 
 /*
@@ -933,7 +1006,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;
@@ -941,16 +1015,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);
@@ -959,12 +1033,15 @@ static int http_get_file(const char *url, const char *filename, int options)
 
 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);
@@ -995,7 +1072,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;
@@ -1048,6 +1125,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,
 
 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;
@@ -1057,7 +1135,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 d77c1b54f24d142461b9e1a0d11cf6de041bc59f..12ca5c892d61a063c856bcdef9154a103c0aa4bc 100644 (file)
--- 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);
 
index 5b3ce9eed299e312c132b590ea4926cfb019a271..ef1684b9df3ee5cff0d63ab5f7ed51e7f351a686 100644 (file)
@@ -8,9 +8,11 @@
 #include "pkt-line.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;
@@ -111,7 +113,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) +
@@ -130,7 +133,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;
@@ -184,40 +187,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));
@@ -258,9 +268,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;
@@ -444,6 +455,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;
@@ -543,8 +556,10 @@ 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)
+       if (err == HTTP_REAUTH && !large_request) {
+               credential_fill(&http_auth);
                goto retry;
+       }
        if (err != HTTP_OK)
                err = -1;
 
@@ -579,7 +594,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);
@@ -631,7 +646,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;
@@ -676,7 +691,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++) {
@@ -774,7 +789,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;
@@ -804,7 +819,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
        else if (options.verbosity > 1)
                argv_array_push(&args, "--verbose");
        argv_array_push(&args, options.progress ? "--progress" : "--no-progress");
-       argv_array_push(&args, url);
+       argv_array_push(&args, url.buf);
        for (i = 0; i < nr_spec; i++)
                argv_array_push(&args, specs[i]);
 
@@ -885,14 +900,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) {
index 895b9258b07ca65b78ac376023e203f482d8ac16..7059cc6c21518ec43df28770ffc18d5e0ac1a660 100644 (file)
@@ -187,7 +187,8 @@ set_askpass() {
 }
 
 expect_askpass() {
-       dest=$HTTPD_DEST
+       dest=$HTTPD_DEST${3+/$3}
+
        {
                case "$1" in
                none)
index dd17e3a09d728a08c8038c5ce8187ea63112cdf3..4a261f13f533daaa3d687d2bb88481b560432378 100644 (file)
@@ -102,6 +102,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
index 55a866af803452e164e2c11c421b1e63f62d2e1e..1b71bb515646e47181da33f83f97f04b6e0e8617 100755 (executable)
@@ -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