remote: refactor proxy detection

Update the proxy detection for a remote.

1. Honor `http.<url>.proxy` syntax for a remote's direct URL and
   parent URLs.
2. Honor an empty configuration URL to override a proxy configuration.

Add tests to ensure that configuration specificity is honored.

7 files changed, 238 insertions, 221 deletions

diff --git a/src/remote.c b/src/remote.c
index e63f54a9b..7dddea93a 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -849,112 +849,91 @@ int git_remote_ls(const git_remote_head ***out, size_t *size, git_remote *remote
 	return remote->transport->ls(out, size, remote->transport);
 }
 
-int git_remote__get_http_proxy_bypass(git_net_url *url, git_buf *no_proxy_env, bool *bypass)
+static int lookup_config(char **out, git_config *cfg, const char *name)
 {
-	int error = 0;
-	char *p_start = no_proxy_env->ptr;
-	size_t p_length = 0;
-	char c;
-	git_buf hostport = GIT_BUF_INIT;
+	git_config_entry *ce = NULL;
+	int error;
 
-	error = git_buf_printf(&hostport, "%s:%s", url->host, url->port);
-	if (error < 0)
+	if ((error = git_config__lookup_entry(&ce, cfg, name, false)) < 0)
 		return error;
 
-	*bypass = false;
-
-	do {
-		c = *(p_start + p_length);
-		if ((c == ',') || (c == 0)) {
-			if ((p_length == 1) && (*p_start == '*')) {
-				// wildcard match (*)
-				goto found;
-			} else if ((p_length == strlen(url->host)) && !memcmp(p_start, url->host, p_length)) {
-				// exact host match
-				goto found;
-			} else if ((p_length == strlen(hostport.ptr)) && !memcmp(p_start, hostport.ptr, p_length)) {
-				// exact host:port match
-				goto found;
-			} else {
-				if ((p_length >= 2) && (*p_start == '*') && (*(p_start + 1) == '.')) {
-					// *.foo == .foo
-					p_start++;
-					p_length--;
-				}
-				if ((*p_start == '.') && (strlen(url->host) > p_length) && !memcmp(p_start, url->host + strlen(url->host) - p_length, p_length)) {
-					// host suffix match (.example.org)
-					goto found;
-				} else if ((*p_start == '.') && (strlen(hostport.ptr) > p_length) && !memcmp(p_start, hostport.ptr + strlen(hostport.ptr) - p_length, p_length)) {
-					// host:port suffix match (.example.org:443)
-					goto found;
-				}
-			}
-			p_start += p_length + 1;
-			p_length = 0;
-		} else {
-			p_length++;
-		}
-	} while(c != 0);
+	if (ce && ce->value) {
+		*out = git__strdup(ce->value);
+		GIT_ERROR_CHECK_ALLOC(*out);
+	} else {
+		error = GIT_ENOTFOUND;
+	}
+
+	git_config_entry_free(ce);
+	return error;
+}
 
-	goto end;
+static void url_config_trim(git_net_url *url)
+{
+	size_t len = strlen(url->path);
 
-found:
-	*bypass = true;
+	if (url->path[len - 1] == '/') {
+		len--;
+	} else {
+		while (len && url->path[len - 1] != '/')
+			len--;
+	}
 
-end:
-	git_buf_dispose(&hostport);
-	return 0;
+	url->path[len] = '\0';
 }
 
-int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, git_net_url *url, char **proxy_url)
+static int http_proxy_config(char **out, git_remote *remote, git_net_url *url)
 {
 	git_config *cfg;
-	git_config_entry *ce = NULL;
-	git_buf proxy_env = GIT_BUF_INIT;
-	git_buf no_proxy_env = GIT_BUF_INIT;
-	bool bypass = false;
+	git_buf buf = GIT_BUF_INIT;
+	git_net_url lookup_url = GIT_NET_URL_INIT;
 	int error;
 
-	GIT_ASSERT_ARG(remote);
+	if ((error = git_net_url_dup(&lookup_url, url)) < 0 ||
+	    (error = git_repository_config__weakptr(&cfg, remote->repo)) < 0)
+		goto done;
 
-	if (!proxy_url || !remote->repo)
-		return -1;
+	/* remote.<name>.proxy config setting */
+	if (remote->name && remote->name[0]) {
+		git_buf_clear(&buf);
 
-	*proxy_url = NULL;
+		if ((error = git_buf_printf(&buf, "remote.%s.proxy", remote->name)) < 0 ||
+		    (error = lookup_config(out, cfg, buf.ptr)) != GIT_ENOTFOUND)
+			goto done;
+	}
 
-	if ((error = git_repository_config__weakptr(&cfg, remote->repo)) < 0)
-		return error;
+	while (true) {
+		git_buf_clear(&buf);
 
-	/* Go through the possible sources for proxy configuration, from most specific
-	 * to least specific. */
+		if ((error = git_buf_puts(&buf, "http.")) < 0 ||
+		    (error = git_net_url_fmt(&buf, &lookup_url)) < 0 ||
+		    (error = git_buf_puts(&buf, ".proxy")) < 0 ||
+		    (error = lookup_config(out, cfg, buf.ptr)) != GIT_ENOTFOUND)
+			goto done;
 
-	/* remote.<name>.proxy config setting */
-	if (remote->name && remote->name[0]) {
-		git_buf buf = GIT_BUF_INIT;
+		if (! lookup_url.path[0])
+			break;
 
-		if ((error = git_buf_printf(&buf, "remote.%s.proxy", remote->name)) < 0)
-			return error;
+		url_config_trim(&lookup_url);
+	}
 
-		error = git_config__lookup_entry(&ce, cfg, git_buf_cstr(&buf), false);
-		git_buf_dispose(&buf);
+	git_buf_clear(&buf);
 
-		if (error < 0)
-			return error;
+	error = lookup_config(out, cfg, "http.proxy");
 
-		if (ce && ce->value) {
-			*proxy_url = git__strdup(ce->value);
-			goto found;
-		}
-	}
+done:
+	git_buf_dispose(&buf);
+	git_net_url_dispose(&lookup_url);
+	return error;
+}
 
-	/* http.proxy config setting */
-	if ((error = git_config__lookup_entry(&ce, cfg, "http.proxy", false)) < 0)
-		return error;
+static int http_proxy_env(char **out, git_remote *remote, git_net_url *url)
+{
+	git_buf proxy_env = GIT_BUF_INIT, no_proxy_env = GIT_BUF_INIT;
+	bool use_ssl = (strcmp(url->scheme, "https") == 0);
+	int error;
 
-	if (ce && ce->value) {
-		*proxy_url = git__strdup(ce->value);
-		goto found;
-	}
+	GIT_UNUSED(remote);
 
 	/* http_proxy / https_proxy environment variables */
 	error = git__getenv(&proxy_env, use_ssl ? "https_proxy" : "http_proxy");
@@ -963,46 +942,51 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, git_net_url *ur
 	if (error == GIT_ENOTFOUND)
 		error = git__getenv(&proxy_env, use_ssl ? "HTTPS_PROXY" : "HTTP_PROXY");
 
-	if (error < 0) {
-		if (error == GIT_ENOTFOUND) {
-			git_error_clear();
-			error = 0;
-		}
-
-		return error;
-	}
+	if (error)
+		goto done;
 
 	/* no_proxy/NO_PROXY environment variables */
 	error = git__getenv(&no_proxy_env, "no_proxy");
+
 	if (error == GIT_ENOTFOUND)
 		error = git__getenv(&no_proxy_env, "NO_PROXY");
 
-	if (error == GIT_ENOTFOUND) {
-		git_error_clear();
-		error = 0;
-	} else if (error < 0) {
-		goto cleanup;
-	} else {
-		error = git_remote__get_http_proxy_bypass(url, &no_proxy_env, &bypass);
-	}
-
-	if (bypass) {
-		git_buf_dispose(&proxy_env);
-		goto cleanup;
-	} else {
-		*proxy_url = git_buf_detach(&proxy_env);
-	}
+	if (error && error != GIT_ENOTFOUND)
+		goto done;
 
-found:
-	GIT_ERROR_CHECK_ALLOC(*proxy_url);
+	if (!git_net_url_matches_pattern_list(url, no_proxy_env.ptr))
+		*out = git_buf_detach(&proxy_env);
+	else
+		error = GIT_ENOTFOUND;
 
-cleanup:
+done:
+	git_buf_dispose(&proxy_env);
 	git_buf_dispose(&no_proxy_env);
-	git_config_entry_free(ce);
-
 	return error;
 }
 
+int git_remote__http_proxy(char **out, git_remote *remote, git_net_url *url)
+{
+	int error;
+
+	GIT_ASSERT_ARG(out);
+	GIT_ASSERT_ARG(remote);
+	GIT_ASSERT_ARG(remote->repo);
+
+	*out = NULL;
+
+	/*
+	 * Go through the possible sources for proxy configuration,
+	 * Examine the various git config options first, then
+	 * consult environment variables.
+	 */
+	if ((error = http_proxy_config(out, remote, url)) != GIT_ENOTFOUND ||
+	    (error = http_proxy_env(out, remote, url)) != GIT_ENOTFOUND)
+		return error;
+
+	return 0;
+}
+
 /* DWIM `refspecs` based on `refs` and append the output to `out` */
 static int dwim_refspecs(git_vector *out, git_vector *refspecs, git_vector *refs)
 {
diff --git a/src/remote.h b/src/remote.h
index ffcefdf7f..ce92db76a 100644
--- a/src/remote.h
+++ b/src/remote.h
@@ -9,13 +9,13 @@
 
 #include "common.h"
 
-#include "net.h"
 #include "git2/remote.h"
 #include "git2/transport.h"
 #include "git2/sys/transport.h"
 
 #include "refspec.h"
 #include "vector.h"
+#include "net.h"
 
 #define GIT_REMOTE_ORIGIN "origin"
 
@@ -47,8 +47,7 @@ typedef struct git_remote_connection_opts {
 int git_remote__connect(git_remote *remote, git_direction direction, const git_remote_callbacks *callbacks, const git_remote_connection_opts *conn);
 
 int git_remote__urlfordirection(git_buf *url_out, struct git_remote *remote, int direction, const git_remote_callbacks *callbacks);
-int git_remote__get_http_proxy_bypass(git_net_url *url, git_buf *no_proxy_env, bool *bypass);
-int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, git_net_url *url, char **proxy_url);
+int git_remote__http_proxy(char **out, git_remote *remote, git_net_url *url);
 
 git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname);
 git_refspec *git_remote__matching_dst_refspec(git_remote *remote, const char *refname);
diff --git a/src/transports/http.c b/src/transports/http.c
index 5468674e0..914335aba 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -290,7 +290,6 @@ static int lookup_proxy(
 {
 	const char *proxy;
 	git_remote *remote;
-	bool use_ssl;
 	char *config = NULL;
 	int error = 0;
 
@@ -304,9 +303,8 @@ static int lookup_proxy(
 
 	case GIT_PROXY_AUTO:
 		remote = transport->owner->owner;
-		use_ssl = !strcmp(transport->server.url.scheme, "https");
 
-		error = git_remote__get_http_proxy(remote, use_ssl, &transport->server.url, &config);
+		error = git_remote__http_proxy(&config, remote, &transport->server.url);
 
 		if (error || !config)
 			goto done;
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 8dc39d8bb..178773a41 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -373,7 +373,6 @@ static int winhttp_stream_connect(winhttp_stream *s)
 {
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
 	git_buf buf = GIT_BUF_INIT;
-	bool use_ssl;
 	char *proxy_url = NULL;
 	wchar_t ct[MAX_CONTENT_TYPE_LEN];
 	LPCWSTR types[] = { L"*/*", NULL };
@@ -430,8 +429,7 @@ static int winhttp_stream_connect(winhttp_stream *s)
 	proxy_opts = &t->owner->proxy;
 	if (proxy_opts->type == GIT_PROXY_AUTO) {
 		/* Set proxy if necessary */
-		use_ssl = strcmp(t->server.url.scheme, "https") == 0;
-		if (git_remote__get_http_proxy(t->owner->owner, use_ssl, &t->server.url, &proxy_url) < 0)
+		if (git_remote__http_proxy(&proxy_url, t->owner->owner, &t->server.url) < 0)
 			goto on_error;
 	}
 	else if (proxy_opts->type == GIT_PROXY_SPECIFIED) {
@@ -744,7 +742,7 @@ static void CALLBACK winhttp_status(
 				git_error_set(GIT_ERROR_HTTP, "unknown security error %lu", status);
 
 			break;
-		
+
 		case WINHTTP_CALLBACK_STATUS_SENDING_REQUEST:
 			((winhttp_stream *) ctx)->status_sending_request_reached = 1;
 
diff --git a/tests/online/clone.c b/tests/online/clone.c
index d9b1837df..7d43c6a09 100644
--- a/tests/online/clone.c
+++ b/tests/online/clone.c
@@ -898,67 +898,6 @@ void test_online_clone__proxy_credentials_in_url_https(void)
 	git_buf_dispose(&url);
 }
 
-struct no_proxy_test_entry {
-	char no_proxy[128];
-	bool bypass;
-};
-
-static struct no_proxy_test_entry no_proxy_test_entries[] = {
-	{"*", true},
-	{"github.com", true},
-	{"github.com:443", true},
-	{"github.com:80", false},
-	{".github.com", false},
-	{"*.github.com", false},
-	{".com", true},
-	{"*.com", true},
-	{".com:443", true},
-	{"*.com:443", true},
-	{".com:80", false},
-	{"*.com:80", false},
-	{"", false}
-};
-
-void test_online_clone__no_proxy_in_environment(void)
-{
-	int error = 0;
-	unsigned int i;
-	git_buf proxy_url = GIT_BUF_INIT;
-
-	_orig_http_proxy = cl_getenv("HTTP_PROXY");
-	_orig_https_proxy = cl_getenv("HTTPS_PROXY");
-	_orig_no_proxy = cl_getenv("NO_PROXY");
-	_orig_proxies_need_reset = 1;
-
-	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_AUTO;
-	g_options.fetch_opts.proxy_opts.certificate_check = proxy_cert_cb;
-
-	cl_git_pass(git_buf_printf(&proxy_url, "http://does-not-exists.example.org:1234/"));
-
-	cl_setenv("HTTP_PROXY", proxy_url.ptr);
-	cl_setenv("HTTPS_PROXY", proxy_url.ptr);
-
-
-	for (i = 0; i < ARRAY_SIZE(no_proxy_test_entries); ++i) {
-		cl_setenv("NO_PROXY", no_proxy_test_entries[i].no_proxy);
-		error = git_clone(&g_repo, "https://github.com/libgit2/TestGitRepository", "./foo", &g_options);
-
-		if (no_proxy_test_entries[i].bypass) {
-			cl_assert_(error == 0, no_proxy_test_entries[i].no_proxy);
-		} else {
-			cl_assert_(error == -1, no_proxy_test_entries[i].no_proxy);
-		}
-
-		if (g_repo) {
-			git_repository_free(g_repo);
-			g_repo = NULL;
-		}
-		cl_fixture_cleanup("./foo");
-	}
-
-	git_buf_dispose(&proxy_url);
-}
-
 void test_online_clone__proxy_auto_not_detected(void)
 {
 	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_AUTO;
diff --git a/tests/remote/httpproxy.c b/tests/remote/httpproxy.c
new file mode 100644
index 000000000..097db4cd5
--- /dev/null
+++ b/tests/remote/httpproxy.c
@@ -0,0 +1,139 @@
+#include "clar_libgit2.h"
+#include "remote.h"
+#include "net.h"
+
+static git_repository *repo;
+static git_net_url url = GIT_NET_URL_INIT;
+
+static int orig_proxies_need_reset = 0;
+static char *orig_http_proxy = NULL;
+static char *orig_https_proxy = NULL;
+static char *orig_no_proxy = NULL;
+
+void test_remote_httpproxy__initialize(void)
+{
+	git_remote *remote;
+
+	repo = cl_git_sandbox_init("testrepo");
+	cl_git_pass(git_remote_create(&remote, repo, "lg2", "https://github.com/libgit2/libgit2"));
+	cl_git_pass(git_net_url_parse(&url, "https://github.com/libgit2/libgit2"));
+
+	git_remote_free(remote);
+
+	orig_proxies_need_reset = 0;
+}
+
+void test_remote_httpproxy__cleanup(void)
+{
+	if (orig_proxies_need_reset) {
+		cl_setenv("HTTP_PROXY", orig_http_proxy);
+		cl_setenv("HTTPS_PROXY", orig_https_proxy);
+		cl_setenv("NO_PROXY", orig_no_proxy);
+
+		git__free(orig_http_proxy);
+		git__free(orig_https_proxy);
+		git__free(orig_no_proxy);
+	}
+
+	git_net_url_dispose(&url);
+	cl_git_sandbox_cleanup();
+}
+
+void assert_proxy_is(const char *expected)
+{
+	git_remote *remote;
+	char *proxy;
+
+	cl_git_pass(git_remote_lookup(&remote, repo, "lg2"));
+	cl_git_pass(git_remote__http_proxy(&proxy, remote, &url));
+
+	if (expected)
+		cl_assert_equal_s(proxy, expected);
+	else
+		cl_assert_equal_p(proxy, expected);
+
+	git_remote_free(remote);
+	git__free(proxy);
+}
+
+void assert_config_match(const char *config, const char *expected)
+{
+	git_remote *remote;
+	char *proxy;
+
+	if (config)
+		cl_repo_set_string(repo, config, expected);
+
+	cl_git_pass(git_remote_lookup(&remote, repo, "lg2"));
+	cl_git_pass(git_remote__http_proxy(&proxy, remote, &url));
+
+	if (expected)
+		cl_assert_equal_s(proxy, expected);
+	else
+		cl_assert_equal_p(proxy, expected);
+
+	git_remote_free(remote);
+	git__free(proxy);
+}
+
+void test_remote_httpproxy__config_overrides(void)
+{
+	/*
+	 * http.proxy should be honored, then http.<url>.proxy should
+	 * be honored in increasing specificity of the url.  finally,
+	 * remote.<name>.proxy is the most specific.
+	 */
+	assert_config_match(NULL, NULL);
+	assert_config_match("http.proxy", "http://localhost:1/");
+	assert_config_match("http.https://github.com.proxy", "http://localhost:2/");
+	assert_config_match("http.https://github.com/.proxy", "http://localhost:3/");
+	assert_config_match("http.https://github.com/libgit2.proxy", "http://localhost:4/");
+	assert_config_match("http.https://github.com/libgit2/.proxy", "http://localhost:5/");
+	assert_config_match("http.https://github.com/libgit2/libgit2.proxy", "http://localhost:6/");
+	assert_config_match("remote.lg2.proxy", "http://localhost:7/");
+}
+
+void test_remote_httpproxy__config_empty_overrides(void)
+{
+	/*
+	 * with greater specificity, an empty config entry overrides
+	 * a set one
+	 */
+	assert_config_match("http.proxy", "http://localhost:1/");
+	assert_config_match("http.https://github.com.proxy", "");
+	assert_config_match("http.https://github.com/libgit2/libgit2.proxy", "http://localhost:2/");
+	assert_config_match("remote.lg2.proxy", "");
+}
+
+void test_remote_httpproxy__env(void)
+{
+	orig_http_proxy = cl_getenv("HTTP_PROXY");
+	orig_https_proxy = cl_getenv("HTTPS_PROXY");
+	orig_no_proxy = cl_getenv("NO_PROXY");
+	orig_proxies_need_reset = 1;
+
+	/* HTTP proxy is ignored for HTTPS */
+	cl_setenv("HTTP_PROXY", "http://localhost:9/");
+	assert_proxy_is(NULL);
+
+	/* HTTPS proxy is honored for HTTPS */
+	cl_setenv("HTTPS_PROXY", "http://localhost:10/");
+	assert_proxy_is("http://localhost:10/");
+
+	/* NO_PROXY is honored */
+	cl_setenv("NO_PROXY", "github.com:443");
+	assert_proxy_is(NULL);
+
+	cl_setenv("NO_PROXY", "github.com:80");
+	assert_proxy_is("http://localhost:10/");
+
+	cl_setenv("NO_PROXY", "github.com");
+	assert_proxy_is(NULL);
+
+	cl_setenv("NO_PROXY", "github.dev,github.com,github.foo");
+	assert_proxy_is(NULL);
+
+	/* configuration overrides environment variables */
+	cl_setenv("NO_PROXY", "github.none");
+	assert_config_match("http.https://github.com.proxy", "http://localhost:11/");
+}
diff --git a/tests/remote/no_proxy.c b/tests/remote/no_proxy.c
deleted file mode 100644
index 4f758415c..000000000
--- a/tests/remote/no_proxy.c
+++ /dev/null
@@ -1,40 +0,0 @@
-#include "clar_libgit2.h"
-#include "remote.h"
-
-/* Suite data */
-struct no_proxy_test_entry {
-	char url[128];
-	char no_proxy[128];
-	bool bypass;
-};
-
-static struct no_proxy_test_entry no_proxy_test_entries[] = {
-	{"https://example.com/", "", false},
-	{"https://example.com/", "example.org", false},
-	{"https://example.com/", "*", true},
-	{"https://example.com/", "example.com,example.org", true},
-	{"https://example.com/", ".example.com,example.org", false},
-	{"https://foo.example.com/", ".example.com,example.org", true},
-	{"https://example.com/", "foo.example.com,example.org", false},
-
-};
-
-void test_remote_no_proxy__entries(void)
-{
-	unsigned int i;
-	git_net_url url = GIT_NET_URL_INIT;
-	git_buf no_proxy = GIT_BUF_INIT;
-	bool bypass = false;
-
-	for (i = 0; i < ARRAY_SIZE(no_proxy_test_entries); ++i) {
-		cl_git_pass(git_net_url_parse(&url, no_proxy_test_entries[i].url));
-		cl_git_pass(git_buf_sets(&no_proxy, no_proxy_test_entries[i].no_proxy));
-		cl_git_pass(git_remote__get_http_proxy_bypass(&url, &no_proxy, &bypass));
-
-		cl_assert_(bypass == no_proxy_test_entries[i].bypass, no_proxy_test_entries[i].no_proxy);
-
-		git_net_url_dispose(&url);
-		git_buf_dispose(&no_proxy);
-	}
-
-}