Skip to content

Commit

Permalink
Merge branch 'jk/http-auth-redirects' into maint
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Junio C Hamano committed Nov 8, 2013
2 parents 486b65a + 70900ed commit e5becd0
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 65 deletions.
4 changes: 2 additions & 2 deletions http-push.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
137 changes: 108 additions & 29 deletions http.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -878,26 +890,85 @@ 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);

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;

Expand All @@ -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);
Expand All @@ -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);
}

/*
Expand All @@ -933,24 +1006,25 @@ 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;
FILE *result;

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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down
30 changes: 25 additions & 5 deletions http.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -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
Expand All @@ -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);

Expand Down
Loading

0 comments on commit e5becd0

Please sign in to comment.