Merge pull request #6597 from mathworks/proxy_invalid_url_crash

proxy: Return an error for invalid proxy URLs instead of crashing.

src/libgit2/transports/http.c

@@ -335,9 +335,15 @@ static int lookup_proxy(
 	}
 
 	if (!proxy ||
-	    (error = git_net_url_parse(&transport->proxy.url, proxy)) < 0)
+	    (error = git_net_url_parse_http(&transport->proxy.url, proxy)) < 0)
 		goto done;
 
+	if (!git_net_url_valid(&transport->proxy.url)) {
+		git_error_set(GIT_ERROR_HTTP, "invalid URL: '%s'", proxy);
+		error = -1;
+		goto done;
+	}
+
 	*out_use = true;
 
 done:

src/libgit2/transports/httpclient.c

@@ -837,6 +837,11 @@ GIT_INLINE(int) server_setup_from_url(
 	git_http_server *server,
 	git_net_url *url)
 {
+	GIT_ASSERT_ARG(url);
+	GIT_ASSERT_ARG(url->scheme);
+	GIT_ASSERT_ARG(url->host);
+	GIT_ASSERT_ARG(url->port);
+
 	if (!server->url.scheme || strcmp(server->url.scheme, url->scheme) ||
 	    !server->url.host || strcmp(server->url.host, url->host) ||
 	    !server->url.port || strcmp(server->url.port, url->port)) {

src/libgit2/transports/winhttp.c

@@ -443,10 +443,10 @@ static int winhttp_stream_connect(winhttp_stream *s)
 
 		git_net_url_dispose(&t->proxy.url);
 
-		if ((error = git_net_url_parse(&t->proxy.url, proxy_url)) < 0)
+		if ((error = git_net_url_parse_http(&t->proxy.url, proxy_url)) < 0)
 			goto on_error;
 
-		if (strcmp(t->proxy.url.scheme, "http") != 0 && strcmp(t->proxy.url.scheme, "https") != 0) {
+		if (!git_net_url_valid(&t->proxy.url)) {
 			git_error_set(GIT_ERROR_HTTP, "invalid URL: '%s'", proxy_url);
 			error = -1;
 			goto on_error;

src/util/net.h

@@ -57,6 +57,14 @@ extern int git_net_url_parse_scp(git_net_url *url, const char *str);
  */
 extern int git_net_url_parse_standard_or_scp(git_net_url *url, const char *str);
 
+/**
+ * Parses a string containing an HTTP endpoint that may not be a
+ * well-formed URL. For example, "localhost" or "localhost:port".
+ */
+extern int git_net_url_parse_http(
+	git_net_url *url,
+	const char *str);
+
 /** Appends a path and/or query string to the given URL */
 extern int git_net_url_joinpath(
 	git_net_url *out,

tests/libgit2/online/clone.c

@@ -43,7 +43,6 @@ static char *_github_ssh_privkey = NULL;
 static char *_github_ssh_passphrase = NULL;
 static char *_github_ssh_remotehostkey = NULL;
 
-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;
@@ -99,10 +98,12 @@ void test_online_clone__initialize(void)
 	_github_ssh_passphrase = cl_getenv("GITTEST_GITHUB_SSH_PASSPHRASE");
 	_github_ssh_remotehostkey = cl_getenv("GITTEST_GITHUB_SSH_REMOTE_HOSTKEY");
 
+	_orig_http_proxy = cl_getenv("HTTP_PROXY");
+	_orig_https_proxy = cl_getenv("HTTPS_PROXY");
+	_orig_no_proxy = cl_getenv("NO_PROXY");
+
 	if (_remote_expectcontinue)
 		git_libgit2_opts(GIT_OPT_ENABLE_HTTP_EXPECT_CONTINUE, 1);
-
-	_orig_proxies_need_reset = 0;
 }
 
 void test_online_clone__cleanup(void)
@@ -140,15 +141,13 @@ void test_online_clone__cleanup(void)
 	git__free(_github_ssh_passphrase);
 	git__free(_github_ssh_remotehostkey);
 
-	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);
+	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__free(_orig_http_proxy);
+	git__free(_orig_https_proxy);
+	git__free(_orig_no_proxy);
 
 	git_libgit2_opts(GIT_OPT_SET_SSL_CERT_LOCATIONS, NULL, NULL);
 	git_libgit2_opts(GIT_OPT_SET_SERVER_TIMEOUT, 0);
@@ -968,6 +967,92 @@ static int proxy_cert_cb(git_cert *cert, int valid, const char *host, void *payl
 	return valid ? 0 : GIT_ECERTIFICATE;
 }
 
+void test_online_clone__proxy_http_host_port_in_opts(void)
+{
+	if (!_remote_proxy_host || !_remote_proxy_user || !_remote_proxy_pass)
+		cl_skip();
+
+	if (_remote_proxy_scheme && strcmp(_remote_proxy_scheme, "http") != 0)
+		cl_skip();
+
+	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_SPECIFIED;
+	g_options.fetch_opts.proxy_opts.url = _remote_proxy_host;
+	g_options.fetch_opts.proxy_opts.credentials = proxy_cred_cb;
+
+	called_proxy_creds = 0;
+	cl_git_pass(git_clone(&g_repo, "https://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+	cl_assert(called_proxy_creds == 1);
+}
+
+void test_online_clone__proxy_http_host_port_in_env(void)
+{
+	if (!_remote_proxy_host || !_remote_proxy_user || !_remote_proxy_pass)
+		cl_skip();
+
+	if (_remote_proxy_scheme && strcmp(_remote_proxy_scheme, "http") != 0)
+		cl_skip();
+
+	cl_setenv("HTTP_PROXY", _remote_proxy_host);
+	cl_setenv("HTTPS_PROXY", _remote_proxy_host);
+	cl_setenv("NO_PROXY", NULL);
+
+	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_AUTO;
+	g_options.fetch_opts.proxy_opts.credentials = proxy_cred_cb;
+
+	called_proxy_creds = 0;
+	cl_git_pass(git_clone(&g_repo, "https://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+	cl_assert(called_proxy_creds == 1);
+}
+
+static int repository_create_with_proxy(
+	git_repository **out,
+	const char *path,
+	int bare,
+	void *payload)
+{
+	git_repository *repo;
+	git_config *config;
+	char *value = (char *)payload;
+
+	cl_git_pass(git_repository_init(&repo, path, bare));
+	cl_git_pass(git_repository_config(&config, repo));
+
+	cl_git_pass(git_config_set_string(config, "http.proxy", value));
+
+	git_config_free(config);
+
+	*out = repo;
+	return 0;
+}
+
+void test_online_clone__proxy_http_host_port_in_config(void)
+{
+	if (!_remote_proxy_host || !_remote_proxy_user || !_remote_proxy_pass)
+		cl_skip();
+
+	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_AUTO;
+	g_options.fetch_opts.proxy_opts.credentials = proxy_cred_cb;
+	g_options.repository_cb = repository_create_with_proxy;
+	g_options.repository_cb_payload = _remote_proxy_host;
+
+	called_proxy_creds = 0;
+	cl_git_pass(git_clone(&g_repo, "https://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+	cl_assert(called_proxy_creds == 1);
+}
+
+void test_online_clone__proxy_invalid_url(void)
+{
+	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_SPECIFIED;
+	g_options.fetch_opts.proxy_opts.credentials = proxy_cred_cb;
+	g_options.fetch_opts.proxy_opts.certificate_check = proxy_cert_cb;
+
+	g_options.fetch_opts.proxy_opts.url = "noschemeorport";
+	cl_git_fail(git_clone(&g_repo, "http://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+
+	g_options.fetch_opts.proxy_opts.url = "noscheme:8080";
+	cl_git_fail(git_clone(&g_repo, "http://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+}
+
 void test_online_clone__proxy_credentials_request(void)
 {
 	git_str url = GIT_STR_INIT;
@@ -990,7 +1075,7 @@ void test_online_clone__proxy_credentials_request(void)
 	git_str_dispose(&url);
 }
 
-void test_online_clone__proxy_credentials_in_url(void)
+void test_online_clone__proxy_credentials_in_well_formed_url(void)
 {
 	git_str url = GIT_STR_INIT;
 
@@ -1011,17 +1096,35 @@ void test_online_clone__proxy_credentials_in_url(void)
 	git_str_dispose(&url);
 }
 
-void test_online_clone__proxy_credentials_in_environment(void)
+void test_online_clone__proxy_credentials_in_host_port_format(void)
 {
 	git_str url = GIT_STR_INIT;
 
 	if (!_remote_proxy_host || !_remote_proxy_user || !_remote_proxy_pass)
 		cl_skip();
 
-	_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;
+	if (_remote_proxy_scheme && strcmp(_remote_proxy_scheme, "http") != 0)
+		cl_skip();
+
+	cl_git_pass(git_str_printf(&url, "%s:%s@%s",
+		_remote_proxy_user, _remote_proxy_pass, _remote_proxy_host));
+
+	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_SPECIFIED;
+	g_options.fetch_opts.proxy_opts.url = url.ptr;
+	g_options.fetch_opts.proxy_opts.certificate_check = proxy_cert_cb;
+	called_proxy_creds = 0;
+	cl_git_pass(git_clone(&g_repo, "http://github.com/libgit2/TestGitRepository", "./foo", &g_options));
+	cl_assert(called_proxy_creds == 0);
+
+	git_str_dispose(&url);
+}
+
+void test_online_clone__proxy_credentials_in_environment(void)
+{
+	git_str url = GIT_STR_INIT;
+
+	if (!_remote_proxy_host || !_remote_proxy_user || !_remote_proxy_pass)
+		cl_skip();
 
 	g_options.fetch_opts.proxy_opts.type = GIT_PROXY_AUTO;
 	g_options.fetch_opts.proxy_opts.certificate_check = proxy_cert_cb;

tests/util/url/parse.c

@@ -669,6 +669,20 @@ void test_url_parse__ipv6_invalid_addresses(void)
 
 /* Oddities */
 
+void test_url_parse__empty_scheme(void)
+{
+	cl_git_pass(git_net_url_parse(&conndata, "://example.com/resource"));
+	cl_assert_equal_s(conndata.scheme, NULL);
+	cl_assert_equal_s(conndata.host, NULL);
+	cl_assert_equal_s(conndata.port, NULL);
+	cl_assert_equal_s(conndata.path, "//example.com/resource");
+	cl_assert_equal_p(conndata.username, NULL);
+	cl_assert_equal_p(conndata.password, NULL);
+	cl_assert_equal_p(conndata.query, NULL);
+	cl_assert_equal_p(conndata.fragment, NULL);
+	cl_assert_equal_i(git_net_url_is_default_port(&conndata), 0);
+}
+
 void test_url_parse__invalid_scheme_is_relative(void)
 {
 	cl_git_pass(git_net_url_parse(&conndata, "foo!bar://host:42/path/to/project?query_string=yes"));