Merge pull request #1879 from libgit2/redir-refactor

Redir refactor

2 files changed, 37 insertions, 232 deletions

diff --git a/src/transports/http.c b/src/transports/http.c
index ab2f9a47f..ace0d97d0 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -12,8 +12,6 @@
 #include "netops.h"
 #include "smart.h"
 
-static const char *prefix_http = "http://";
-static const char *prefix_https = "https://";
 static const char *upload_pack_service = "upload-pack";
 static const char *upload_pack_ls_service_url = "/info/refs?service=git-upload-pack";
 static const char *upload_pack_service_url = "/git-upload-pack";
@@ -59,16 +57,11 @@ typedef struct {
 	git_smart_subtransport parent;
 	transport_smart *owner;
 	gitno_socket socket;
-	char *path;
-	char *host;
-	char *port;
-	char *user_from_url;
-	char *pass_from_url;
+	gitno_connection_data connection_data;
 	git_cred *cred;
 	git_cred *url_cred;
 	http_authmechanism_t auth_mechanism;
-	unsigned connected : 1,
-		use_ssl : 1;
+	bool connected;
 
 	/* Parser structures */
 	http_parser parser;
@@ -125,12 +118,12 @@ static int gen_request(
 	size_t content_length)
 {
 	http_subtransport *t = OWNING_SUBTRANSPORT(s);
-	const char *path = t->path ? t->path : "/";
+	const char *path = t->connection_data.path ? t->connection_data.path : "/";
 
 	git_buf_printf(buf, "%s %s%s HTTP/1.1\r\n", s->verb, path, s->service_url);
 
 	git_buf_puts(buf, "User-Agent: git/1.0 (libgit2 " LIBGIT2_VERSION ")\r\n");
-	git_buf_printf(buf, "Host: %s\r\n", t->host);
+	git_buf_printf(buf, "Host: %s\r\n", t->connection_data.host);
 
 	if (s->chunked || content_length > 0) {
 		git_buf_printf(buf, "Accept: application/x-git-%s-result\r\n", s->service);
@@ -150,9 +143,9 @@ static int gen_request(
 		return -1;
 
 	/* Use url-parsed basic auth if username and password are both provided */
-	if (!t->cred && t->user_from_url && t->pass_from_url) {
-		if (!t->url_cred &&
-			 git_cred_userpass_plaintext_new(&t->url_cred, t->user_from_url, t->pass_from_url) < 0)
+	if (!t->cred && t->connection_data.user && t->connection_data.pass) {
+		if (!t->url_cred && git_cred_userpass_plaintext_new(&t->url_cred,
+					t->connection_data.user, t->connection_data.pass) < 0)
 			return -1;
 		if (apply_basic_credential(buf, t->url_cred) < 0) return -1;
 	}
@@ -249,98 +242,6 @@ static int on_header_value(http_parser *parser, const char *str, size_t len)
 	return 0;
 }
 
-static void free_connection_data(http_subtransport *t)
-{
-	if (t->host) {
-		git__free(t->host);
-		t->host = NULL;
-	}
-
-	if (t->port) {
-		git__free(t->port);
-		t->port = NULL;
-	}
-
-	if (t->user_from_url) {
-		git__free(t->user_from_url);
-		t->user_from_url = NULL;
-	}
-
-	if (t->pass_from_url) {
-		git__free(t->pass_from_url);
-		t->pass_from_url = NULL;
-	}
-
-	if (t->path) {
-		git__free(t->path);
-		t->path = NULL;
-	}
-}
-
-static int set_connection_data_from_url(
-	http_subtransport *t, const char *url, const char *service_suffix)
-{
-	int error = 0;
-	const char *default_port = NULL;
-	char *original_host = NULL;
-
-	if (!git__prefixcmp(url, prefix_http)) {
-		url = url + strlen(prefix_http);
-		default_port = "80";
-
-		if (t->use_ssl) {
-			giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed");
-			return -1;
-		}
-	}
-
-	if (!git__prefixcmp(url, prefix_https)) {
-		url += strlen(prefix_https);
-		default_port = "443";
-		t->use_ssl = 1;
-	}
-
-	if (!default_port) {
-		giterr_set(GITERR_NET, "Unrecognized URL prefix");
-		return -1;
-	}
-
-	/* preserve original host name for checking */
-	original_host = t->host;
-	t->host = NULL;
-
-	free_connection_data(t);
-
-	error = gitno_extract_url_parts(
-		&t->host, &t->port, &t->user_from_url, &t->pass_from_url,
-		url, default_port);
-
-	if (!error) {
-		const char *path = strchr(url, '/');
-		size_t pathlen = strlen(path);
-		size_t suffixlen = service_suffix ? strlen(service_suffix) : 0;
-
-		if (suffixlen &&
-			!memcmp(path + pathlen - suffixlen, service_suffix, suffixlen))
-			t->path = git__strndup(path, pathlen - suffixlen);
-		else
-			t->path = git__strdup(path);
-
-		/* Allow '/'-led urls, or a change of protocol */
-		if (original_host != NULL) {
-			if (strcmp(original_host, t->host) && t->location[0] != '/') {
-				giterr_set(GITERR_NET, "Cross host redirect not allowed");
-				error = -1;
-			}
-
-			git__free(original_host);
-		}
-	}
-
-	return error;
-}
-
-
 static int on_headers_complete(http_parser *parser)
 {
 	parser_context *ctx = (parser_context *) parser->data;
@@ -369,7 +270,7 @@ static int on_headers_complete(http_parser *parser)
 
 			if (t->owner->cred_acquire_cb(&t->cred,
 					t->owner->url,
-					t->user_from_url,
+					t->connection_data.user,
 					allowed_types,
 					t->owner->cred_acquire_payload) < 0)
 				return PARSE_ERROR_GENERIC;
@@ -384,17 +285,17 @@ static int on_headers_complete(http_parser *parser)
 	/* Check for a redirect.
 	 * Right now we only permit a redirect to the same hostname. */
 	if ((parser->status_code == 301 ||
-		 parser->status_code == 302 ||
-		 (parser->status_code == 303 && get_verb == s->verb) ||
-		 parser->status_code == 307) &&
-		t->location) {
+	     parser->status_code == 302 ||
+	     (parser->status_code == 303 && get_verb == s->verb) ||
+	     parser->status_code == 307) &&
+	    t->location) {
 
 		if (s->redirect_count >= 7) {
 			giterr_set(GITERR_NET, "Too many redirects");
 			return t->parse_error = PARSE_ERROR_GENERIC;
 		}
 
-		if (set_connection_data_from_url(t, t->location, s->service_url) < 0)
+		if (gitno_connection_data_from_url(&t->connection_data, 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
@@ -552,7 +453,7 @@ static int http_connect(http_subtransport *t)
 	if (t->socket.socket)
 		gitno_close(&t->socket);
 
-	if (t->use_ssl) {
+	if (t->connection_data.use_ssl) {
 		int tflags;
 
 		if (t->owner->parent.read_flags(&t->owner->parent, &tflags) < 0)
@@ -564,7 +465,7 @@ static int http_connect(http_subtransport *t)
 			flags |= GITNO_CONNECT_SSL_NO_CHECK_CERT;
 	}
 
-	if (gitno_connect(&t->socket, t->host, t->port, flags) < 0)
+	if (gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags) < 0)
 		return -1;
 
 	t->connected = 1;
@@ -911,10 +812,9 @@ static int http_action(
 	if (!stream)
 		return -1;
 
-	if (!t->host || !t->port || !t->path) {
-		if ((ret = set_connection_data_from_url(t, url, NULL)) < 0)
-			return ret;
-	}
+	if ((!t->connection_data.host || !t->connection_data.port || !t->connection_data.path) &&
+		 (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL)) < 0)
+		return ret;
 
 	if (http_connect(t) < 0)
 		return -1;
@@ -958,7 +858,7 @@ static int http_close(git_smart_subtransport *subtransport)
 		t->url_cred = NULL;
 	}
 
-	free_connection_data(t);
+	gitno_connection_data_free_ptrs(&t->connection_data);
 
 	return 0;
 }
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 377f2ef97..067d6fcc3 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -73,17 +73,12 @@ typedef struct {
 typedef struct {
 	git_smart_subtransport parent;
 	transport_smart *owner;
-	char *path;
-	char *host;
-	char *port;
-	char *user_from_url;
-	char *pass_from_url;
+	gitno_connection_data connection_data;
 	git_cred *cred;
 	git_cred *url_cred;
 	int auth_mechanism;
 	HINTERNET session;
 	HINTERNET connection;
-	unsigned use_ssl : 1;
 } winhttp_subtransport;
 
 static int apply_basic_credential(HINTERNET request, git_cred *cred)
@@ -155,7 +150,7 @@ static int winhttp_stream_connect(winhttp_stream *s)
 	unsigned long disable_redirects = WINHTTP_DISABLE_REDIRECTS;
 
 	/* Prepare URL */
-	git_buf_printf(&buf, "%s%s", t->path, s->service_url);
+	git_buf_printf(&buf, "%s%s", t->connection_data.path, s->service_url);
 
 	if (git_buf_oom(&buf))
 		return -1;
@@ -188,7 +183,7 @@ static int winhttp_stream_connect(winhttp_stream *s)
 			NULL,
 			WINHTTP_NO_REFERER,
 			types,
-			t->use_ssl ? WINHTTP_FLAG_SECURE : 0);
+			t->connection_data.use_ssl ? WINHTTP_FLAG_SECURE : 0);
 
 	if (!s->request) {
 		giterr_set(GITERR_OS, "Failed to open request");
@@ -196,7 +191,7 @@ static int winhttp_stream_connect(winhttp_stream *s)
 	}
 
 	/* Set proxy if necessary */
-	if (git_remote__get_http_proxy(t->owner->owner, !!t->use_ssl, &proxy_url) < 0)
+	if (git_remote__get_http_proxy(t->owner->owner, !!t->connection_data.use_ssl, &proxy_url) < 0)
 		goto on_error;
 
 	if (proxy_url) {
@@ -285,7 +280,7 @@ static int winhttp_stream_connect(winhttp_stream *s)
 	}
 
 	/* If requested, disable certificate validation */
-	if (t->use_ssl) {
+	if (t->connection_data.use_ssl) {
 		int flags;
 
 		if (t->owner->parent.read_flags(&t->owner->parent, &flags) < 0)
@@ -308,9 +303,9 @@ static int winhttp_stream_connect(winhttp_stream *s)
 
 	/* If no other credentials have been applied and the URL has username and
 	 * password, use those */
-	if (!t->cred && t->user_from_url && t->pass_from_url) {
+	if (!t->cred && t->connection_data.user && t->connection_data.pass) {
 		if (!t->url_cred &&
-			 git_cred_userpass_plaintext_new(&t->url_cred, t->user_from_url, t->pass_from_url) < 0)
+			git_cred_userpass_plaintext_new(&t->url_cred, t->connection_data.user, t->connection_data.pass) < 0)
 			goto on_error;
 		if (apply_basic_credential(s->request, t->url_cred) < 0)
 			goto on_error;
@@ -392,98 +387,6 @@ static int write_chunk(HINTERNET request, const char *buffer, size_t len)
 	return 0;
 }
 
-static void free_connection_data(winhttp_subtransport *t)
-{
-	if (t->host) {
-		git__free(t->host);
-		t->host = NULL;
-	}
-
-	if (t->port) {
-		git__free(t->port);
-		t->port = NULL;
-	}
-
-	if (t->user_from_url) {
-		git__free(t->user_from_url);
-		t->user_from_url = NULL;
-	}
-
-	if (t->pass_from_url) {
-		git__free(t->pass_from_url);
-		t->pass_from_url = NULL;
-	}
-
-	if (t->path) {
-		git__free(t->path);
-		t->path = NULL;
-	}
-}
-
-static int set_connection_data_from_url(
-	winhttp_subtransport *t, const char *url, const char *service_suffix)
-{
-	int error = 0;
-	const char *default_port = NULL;
-	char *original_host = NULL;
-	const char *original_url = url;
-
-	if (!git__prefixcmp(url, prefix_http)) {
-		url += strlen(prefix_http);
-		default_port = "80";
-
-		if (t->use_ssl) {
-			giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed");
-			return -1;
-		}
-	}
-
-	if (!git__prefixcmp(url, prefix_https)) {
-		url += strlen(prefix_https);
-		default_port = "443";
-		t->use_ssl = 1;
-	}
-
-	if (!default_port) {
-		giterr_set(GITERR_NET, "Unrecognized URL prefix");
-		return -1;
-	}
-
-	/* preserve original host name for checking */
-	original_host = t->host;
-	t->host = NULL;
-
-	free_connection_data(t);
-
-	error = gitno_extract_url_parts(
-		&t->host, &t->port, &t->user_from_url, &t->pass_from_url,
-		url, default_port);
-
-	if (!error) {
-		const char *path = strchr(url, '/');
-		size_t pathlen = strlen(path);
-		size_t suffixlen = service_suffix ? strlen(service_suffix) : 0;
-
-		if (suffixlen &&
-			!memcmp(path + pathlen - suffixlen, service_suffix, suffixlen))
-			t->path = git__strndup(path, pathlen - suffixlen);
-		else
-			t->path = git__strdup(path);
-
-		/* Allow '/'-led urls, or a change of protocol */
-		if (original_host != NULL) {
-			if (strcmp(original_host, t->host) && original_url[0] != '/') {
-				giterr_set(GITERR_NET, "Cross host redirect not allowed");
-				error = -1;
-			}
-
-			git__free(original_host);
-		}
-	}
-
-	return error;
-}
-
 static int winhttp_connect(
 	winhttp_subtransport *t,
 	const char *url)
@@ -494,11 +397,11 @@ static int winhttp_connect(
 	const char *default_port = "80";
 
 	/* Prepare port */
-	if (git__strtol32(&port, t->port, NULL, 10) < 0)
+	if (git__strtol32(&port, t->connection_data.port, NULL, 10) < 0)
 		return -1;
 
 	/* Prepare host */
-	git_win32_path_from_c(host, t->host);
+	git_win32_path_from_c(host, t->connection_data.host);
 
 	/* Establish session */
 	t->session = WinHttpOpen(
@@ -699,7 +602,8 @@ replay:
 
 			if (!git__prefixcmp_icase(location8, prefix_https)) {
 				/* Upgrade to secure connection; disconnect and start over */
-				set_connection_data_from_url(t, location8, s->service_url);
+				if (gitno_connection_data_from_url(&t->connection_data, location8, s->service_url) < 0)
+					return -1;
 				winhttp_connect(t, location8);
 			}
 
@@ -718,7 +622,8 @@ replay:
 			if (allowed_types &&
 				(!t->cred || 0 == (t->cred->credtype & allowed_types))) {
 
-				if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, t->user_from_url, allowed_types, t->owner->cred_acquire_payload) < 0)
+				if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, t->connection_data.user, allowed_types, 
+								t->owner->cred_acquire_payload) < 0)
 					return -1;
 
 				assert(t->cred);
@@ -1101,10 +1006,10 @@ static int winhttp_action(
 	winhttp_stream *s;
 	int ret = -1;
 
-	if (!t->connection &&
-		(set_connection_data_from_url(t, url, NULL) < 0 ||
-		 winhttp_connect(t, url) < 0))
-		return -1;
+	if (!t->connection)
+		if (gitno_connection_data_from_url(&t->connection_data, url, NULL) < 0 ||
+			 winhttp_connect(t, url) < 0)
+			return -1;
 
 	if (winhttp_stream_alloc(t, &s) < 0)
 		return -1;
@@ -1145,7 +1050,7 @@ static int winhttp_close(git_smart_subtransport *subtransport)
 	winhttp_subtransport *t = (winhttp_subtransport *)subtransport;
 	int ret = 0;
 
-	free_connection_data(t);
+	gitno_connection_data_free_ptrs(&t->connection_data);
 
 	if (t->cred) {
 		t->cred->free(t->cred);