diff options
author | Edward Thomson <ethomson@edwardthomson.com> | 2019-12-01 14:00:49 +1100 |
---|---|---|
committer | Edward Thomson <ethomson@edwardthomson.com> | 2020-01-24 09:54:29 -0600 |
commit | 471daeea559ada7ac215806d55f2f686e7389608 (patch) | |
tree | c283027795a31511782327a4307a6a639e325762 | |
parent | 297c61e41f23caacb39d8c00957fd5ec050c9cbf (diff) | |
download | libgit2-471daeea559ada7ac215806d55f2f686e7389608.tar.gz |
net: refactor gitno redirect handling
Move the redirect handling into `git_net_url` for consistency.
-rw-r--r-- | src/net.c | 106 | ||||
-rw-r--r-- | src/net.h | 6 | ||||
-rw-r--r-- | src/netops.c | 98 | ||||
-rw-r--r-- | src/netops.h | 11 | ||||
-rw-r--r-- | src/transports/http.c | 13 | ||||
-rw-r--r-- | src/transports/winhttp.c | 2 | ||||
-rw-r--r-- | tests/network/redirect.c | 20 |
7 files changed, 124 insertions, 132 deletions
@@ -153,6 +153,112 @@ done: return error; } +/* + * Some servers strip the query parameters from the Location header + * when sending a redirect. Others leave it in place. + * Check for both, starting with the stripped case first, + * since it appears to be more common. + */ +static void remove_service_suffix( + git_net_url *url, + const char *service_suffix) +{ + const char *service_query = strchr(service_suffix, '?'); + size_t full_suffix_len = strlen(service_suffix); + size_t suffix_len = service_query ? + (size_t)(service_query - service_suffix) : full_suffix_len; + size_t path_len = strlen(url->path); + ssize_t truncate = -1; + + /* + * Check for a redirect without query parameters, + * like "/newloc/info/refs"' + */ + if (suffix_len && path_len >= suffix_len) { + size_t suffix_offset = path_len - suffix_len; + + if (git__strncmp(url->path + suffix_offset, service_suffix, suffix_len) == 0 && + (!service_query || git__strcmp(url->query, service_query + 1) == 0)) { + truncate = suffix_offset; + } + } + + /* + * If we haven't already found where to truncate to remove the + * suffix, check for a redirect with query parameters, like + * "/newloc/info/refs?service=git-upload-pack" + */ + if (truncate < 0 && git__suffixcmp(url->path, service_suffix) == 0) + truncate = path_len - full_suffix_len; + + /* Ensure we leave a minimum of '/' as the path */ + if (truncate == 0) + truncate++; + + if (truncate > 0) { + url->path[truncate] = '\0'; + + git__free(url->query); + url->query = NULL; + } +} + +int git_net_url_apply_redirect( + git_net_url *url, + const char *redirect_location, + const char *service_suffix) +{ + git_net_url tmp = GIT_NET_URL_INIT; + int error = 0; + + assert(url && redirect_location); + + if (redirect_location[0] == '/') { + git__free(url->path); + + if ((url->path = git__strdup(redirect_location)) == NULL) { + error = -1; + goto done; + } + } else { + git_net_url *original = url; + + if ((error = git_net_url_parse(&tmp, redirect_location)) < 0) + goto done; + + /* Validate that this is a legal redirection */ + + if (original->scheme && + strcmp(original->scheme, tmp.scheme) != 0 && + strcmp(tmp.scheme, "https") != 0) { + git_error_set(GIT_ERROR_NET, "cannot redirect from '%s' to '%s'", + original->scheme, tmp.scheme); + + error = -1; + goto done; + } + + if (original->host && + git__strcasecmp(original->host, tmp.host) != 0) { + git_error_set(GIT_ERROR_NET, "cannot redirect from '%s' to '%s'", + original->host, tmp.host); + + error = -1; + goto done; + } + + git_net_url_swap(url, &tmp); + } + + /* Remove the service suffix if it was given to us */ + if (service_suffix) + remove_service_suffix(url, service_suffix); + +done: + git_net_url_dispose(&tmp); + return error; +} + bool git_net_url_valid(git_net_url *url) { return (url->host && url->port && url->path); @@ -30,6 +30,12 @@ extern bool git_net_url_valid(git_net_url *url); /** Returns nonzero if the URL is on the default port. */ extern int git_net_url_is_default_port(git_net_url *url); +/* Applies a redirect to the URL with a git-aware service suffix. */ +extern int git_net_url_apply_redirect( + git_net_url *url, + const char *redirect_location, + const char *service_suffix); + /** Swaps the contents of one URL for another. */ extern void git_net_url_swap(git_net_url *a, git_net_url *b); diff --git a/src/netops.c b/src/netops.c index c885d5e89..04ae824cc 100644 --- a/src/netops.c +++ b/src/netops.c @@ -121,101 +121,3 @@ int gitno__match_host(const char *pattern, const char *host) return -1; } - -int gitno_connection_data_handle_redirect( - git_net_url *url, - const char *redirect_str, - const char *service_suffix) -{ - git_net_url tmp = GIT_NET_URL_INIT; - int error = 0; - - assert(url && redirect_str); - - if (redirect_str[0] == '/') { - git__free(url->path); - - if ((url->path = git__strdup(redirect_str)) == NULL) { - error = -1; - goto done; - } - } else { - git_net_url *original = url; - - if ((error = git_net_url_parse(&tmp, redirect_str)) < 0) - goto done; - - /* Validate that this is a legal redirection */ - - if (original->scheme && - strcmp(original->scheme, tmp.scheme) != 0 && - strcmp(tmp.scheme, "https") != 0) { - git_error_set(GIT_ERROR_NET, "cannot redirect from '%s' to '%s'", - original->scheme, tmp.scheme); - - error = -1; - goto done; - } - - if (original->host && - git__strcasecmp(original->host, tmp.host) != 0) { - git_error_set(GIT_ERROR_NET, "cannot redirect from '%s' to '%s'", - original->host, tmp.host); - - error = -1; - goto done; - } - - git_net_url_swap(url, &tmp); - } - - /* Remove the service suffix if it was given to us */ - if (service_suffix) { - /* - * Some servers strip the query parameters from the Location header - * when sending a redirect. Others leave it in place. - * Check for both, starting with the stripped case first, - * since it appears to be more common. - */ - const char *service_query = strchr(service_suffix, '?'); - size_t full_suffix_len = strlen(service_suffix); - size_t suffix_len = service_query ? - (size_t)(service_query - service_suffix) : full_suffix_len; - size_t path_len = strlen(url->path); - ssize_t truncate = -1; - - /* Check for a redirect without query parameters, like "/newloc/info/refs" */ - if (suffix_len && path_len >= suffix_len) { - size_t suffix_offset = path_len - suffix_len; - - if (git__strncmp(url->path + suffix_offset, service_suffix, suffix_len) == 0 && - (!service_query || git__strcmp(url->query, service_query + 1) == 0)) { - truncate = suffix_offset; - } - } - - /* - * If we haven't already found where to truncate to remove the suffix, - * check for a redirect with query parameters, - * like "/newloc/info/refs?service=git-upload-pack" - */ - if (truncate == -1 && git__suffixcmp(url->path, service_suffix) == 0) { - truncate = path_len - full_suffix_len; - } - - if (truncate >= 0) { - /* Ensure we leave a minimum of '/' as the path */ - if (truncate == 0) - truncate++; - url->path[truncate] = '\0'; - - git__free(url->query); - url->query = NULL; - } - } - -done: - git_net_url_dispose(&tmp); - return error; -} - diff --git a/src/netops.h b/src/netops.h index 4c4bf78b0..52f1cccb6 100644 --- a/src/netops.h +++ b/src/netops.h @@ -65,15 +65,4 @@ int gitno_recv(gitno_buffer *buf); void gitno_consume(gitno_buffer *buf, const char *ptr); void gitno_consume_n(gitno_buffer *buf, size_t cons); -/* - * This replaces all the pointers in `data` with freshly-allocated strings, - * that the caller is responsible for freeing. - * `gitno_connection_data_free_ptrs` is good for this. - */ - -int gitno_connection_data_handle_redirect( - git_net_url *data, - const char *url, - const char *service_suffix); - #endif diff --git a/src/transports/http.c b/src/transports/http.c index 045b72157..cd209efff 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -609,17 +609,9 @@ static int on_headers_complete(http_parser *parser) parser->status_code == 308) && t->location) { - if (gitno_connection_data_handle_redirect(&t->server.url, t->location, s->service_url) < 0) + if (git_net_url_apply_redirect(&t->server.url, t->location, s->service_url) < 0) return t->parse_error = PARSE_ERROR_GENERIC; - /* Set the redirect URL on the stream. This is a transfer of - * ownership of the memory. */ - if (s->redirect_url) - git__free(s->redirect_url); - - s->redirect_url = t->location; - t->location = NULL; - t->connected = 0; t->parse_error = PARSE_ERROR_REPLAY; return 0; @@ -1422,9 +1414,6 @@ static void http_stream_free(git_smart_subtransport_stream *stream) if (s->chunk_buffer) git__free(s->chunk_buffer); - if (s->redirect_url) - git__free(s->redirect_url); - git__free(s); } diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index 57c6efcf6..3360ec947 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -1128,7 +1128,7 @@ replay: if (!git__prefixcmp_icase(location8, prefix_https)) { /* Upgrade to secure connection; disconnect and start over */ - if (gitno_connection_data_handle_redirect(&t->server.url, location8, s->service_url) < 0) { + if (git_net_url_apply_redirect(&t->server.url, location8, s->service_url) < 0) { git__free(location8); return -1; } diff --git a/tests/network/redirect.c b/tests/network/redirect.c index ce0a080dd..7ce1310db 100644 --- a/tests/network/redirect.c +++ b/tests/network/redirect.c @@ -18,7 +18,7 @@ void test_network_redirect__redirect_http(void) { cl_git_pass(git_net_url_parse(&conndata, "http://example.com/foo/bar/baz")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "http://example.com/foo/bar/baz", "bar/baz")); cl_assert_equal_s(conndata.scheme, "http"); cl_assert_equal_s(conndata.host, "example.com"); @@ -32,7 +32,7 @@ void test_network_redirect__redirect_ssl(void) { cl_git_pass(git_net_url_parse(&conndata, "https://example.com/foo/bar/baz")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "https://example.com/foo/bar/baz", "bar/baz")); cl_assert_equal_s(conndata.scheme, "https"); cl_assert_equal_s(conndata.host, "example.com"); @@ -46,7 +46,7 @@ void test_network_redirect__redirect_leaves_root_path(void) { cl_git_pass(git_net_url_parse(&conndata, "https://example.com/foo/bar/baz")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "https://example.com/foo/bar/baz", "/foo/bar/baz")); cl_assert_equal_s(conndata.scheme, "https"); cl_assert_equal_s(conndata.host, "example.com"); @@ -60,7 +60,7 @@ void test_network_redirect__redirect_encoded_username_password(void) { cl_git_pass(git_net_url_parse(&conndata, "https://user%2fname:pass%40word%zyx%v@example.com/foo/bar/baz")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "https://user%2fname:pass%40word%zyx%v@example.com/foo/bar/baz", "bar/baz")); cl_assert_equal_s(conndata.scheme, "https"); cl_assert_equal_s(conndata.host, "example.com"); @@ -73,7 +73,7 @@ void test_network_redirect__redirect_encoded_username_password(void) void test_network_redirect__redirect_cross_host_denied(void) { cl_git_pass(git_net_url_parse(&conndata, "https://bar.com/bar/baz")); - cl_git_fail_with(gitno_connection_data_handle_redirect(&conndata, + cl_git_fail_with(git_net_url_apply_redirect(&conndata, "https://foo.com/bar/baz", NULL), -1); } @@ -81,7 +81,7 @@ void test_network_redirect__redirect_cross_host_denied(void) void test_network_redirect__redirect_http_downgrade_denied(void) { cl_git_pass(git_net_url_parse(&conndata, "https://foo.com/bar/baz")); - cl_git_fail_with(gitno_connection_data_handle_redirect(&conndata, + cl_git_fail_with(git_net_url_apply_redirect(&conndata, "http://foo.com/bar/baz", NULL), -1); } @@ -89,7 +89,7 @@ void test_network_redirect__redirect_http_downgrade_denied(void) void test_network_redirect__redirect_relative(void) { cl_git_pass(git_net_url_parse(&conndata, "http://foo.com/bar/baz/biff")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "/zap/baz/biff?bam", NULL)); cl_assert_equal_s(conndata.scheme, "http"); cl_assert_equal_s(conndata.host, "foo.com"); @@ -102,7 +102,7 @@ void test_network_redirect__redirect_relative(void) void test_network_redirect__redirect_relative_ssl(void) { cl_git_pass(git_net_url_parse(&conndata, "https://foo.com/bar/baz/biff")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "/zap/baz/biff?bam", NULL)); cl_assert_equal_s(conndata.scheme, "https"); cl_assert_equal_s(conndata.host, "foo.com"); @@ -115,7 +115,7 @@ void test_network_redirect__redirect_relative_ssl(void) void test_network_redirect__service_query_no_query_params_in_location(void) { cl_git_pass(git_net_url_parse(&conndata, "https://foo.com/bar/info/refs?service=git-upload-pack")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "/baz/info/refs", "/info/refs?service=git-upload-pack")); cl_assert_equal_s(conndata.path, "/baz"); } @@ -123,7 +123,7 @@ void test_network_redirect__service_query_no_query_params_in_location(void) void test_network_redirect__service_query_with_query_params_in_location(void) { cl_git_pass(git_net_url_parse(&conndata, "https://foo.com/bar/info/refs?service=git-upload-pack")); - cl_git_pass(gitno_connection_data_handle_redirect(&conndata, + cl_git_pass(git_net_url_apply_redirect(&conndata, "/baz/info/refs?service=git-upload-pack", "/info/refs?service=git-upload-pack")); cl_assert_equal_s(conndata.path, "/baz"); } |