Commit 7142964f38863276c087169cc74a2b2c1cb07854
2019-12-13T10:56:19
netops: handle intact query parameters in service_suffix removal Some servers leave the query parameters intact in the Location header when responding with a redirect. The service_suffix removal check as written assumed that the server removed them. Handle both cases. Along with PR #5325, this fixes #5321. There are two new tests. The first already passed; the second previously failed.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83
diff --git a/src/netops.c b/src/netops.c
index f19bae9..c885d5e 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -171,25 +171,46 @@ int gitno_connection_data_handle_redirect(
/* 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) : strlen(service_suffix);
+ (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)) {
- /* Ensure we leave a minimum of '/' as the path */
- if (suffix_offset == 0)
- suffix_offset++;
+ truncate = suffix_offset;
+ }
+ }
- url->path[suffix_offset] = '\0';
+ /*
+ * 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;
+ }
- git__free(url->query);
- url->query = NULL;
- }
+ 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;
}
}
diff --git a/tests/network/redirect.c b/tests/network/redirect.c
index 3fc0b18..ce0a080 100644
--- a/tests/network/redirect.c
+++ b/tests/network/redirect.c
@@ -111,3 +111,19 @@ void test_network_redirect__redirect_relative_ssl(void)
cl_assert_equal_p(conndata.username, NULL);
cl_assert_equal_p(conndata.password, NULL);
}
+
+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,
+ "/baz/info/refs", "/info/refs?service=git-upload-pack"));
+ cl_assert_equal_s(conndata.path, "/baz");
+}
+
+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,
+ "/baz/info/refs?service=git-upload-pack", "/info/refs?service=git-upload-pack"));
+ cl_assert_equal_s(conndata.path, "/baz");
+}