Commit 515daeaf4d5918ae93b02a5478adee563b4dac5e

Edward Thomson 2022-01-04T06:16:30

remote: introduce `follow_redirects` connect option Give callers the ability to select how to handle redirects - either supporting redirects during the initial connection (so that, for example, `git.example.com/repo` can redirect to `github.com/example/repo`) or all/no redirects. This is for compatibility with git.

diff --git a/ci/test.sh b/ci/test.sh
index bcbda90..e1a0e61 100755
--- a/ci/test.sh
+++ b/ci/test.sh
@@ -101,7 +101,7 @@ if [ -z "$SKIP_PROXY_TESTS" ]; then
 	java -jar poxyproxy.jar --address 127.0.0.1 --port 8090 --credentials foo:bar --auth-type ntlm --quiet &
 fi
 
-if [ -z "$SKIP_NTLM_TESTS" ]; then
+if [ -z "$SKIP_NTLM_TESTS" -o -z "$SKIP_ONLINE_TESTS" ]; then
 	curl --location --silent --show-error https://github.com/ethomson/poxygit/releases/download/v0.5.1/poxygit-0.5.1.jar >poxygit.jar
 
 	echo ""
@@ -188,7 +188,11 @@ if [ -z "$SKIP_ONLINE_TESTS" ]; then
 	echo "## Running (online) tests"
 	echo "##############################################################################"
 
+	export GITTEST_REMOTE_REDIRECT_INITIAL="http://localhost:9000/initial-redirect/libgit2/TestGitRepository"
+	export GITTEST_REMOTE_REDIRECT_SUBSEQUENT="http://localhost:9000/subsequent-redirect/libgit2/TestGitRepository"
 	run_test online
+	unset GITTEST_REMOTE_REDIRECT_INITIAL
+	unset GITTEST_REMOTE_REDIRECT_SUBSEQUENT
 
 	# Run the online tests that immutably change global state separately
 	# to avoid polluting the test environment.
diff --git a/include/git2/remote.h b/include/git2/remote.h
index f1aa35f..162b076 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -42,6 +42,30 @@ GIT_EXTERN(int) git_remote_create(
 		const char *url);
 
 /**
+ * Remote redirection settings; whether redirects to another host
+ * are permitted.  By default, git will follow a redirect on the
+ * initial request (`/info/refs`), but not subsequent requests.
+ */
+typedef enum {
+	/**
+	 * Do not follow any off-site redirects at any stage of
+	 * the fetch or push.
+	 */
+	GIT_REMOTE_REDIRECT_NONE = (1 << 0),
+
+	/**
+	 * Allow off-site redirects only upon the initial request.
+	 * This is the default.
+	 */
+	GIT_REMOTE_REDIRECT_INITIAL = (1 << 1),
+
+	/**
+	 * Allow redirects at any stage in the fetch or push.
+	 */
+	GIT_REMOTE_REDIRECT_ALL = (1 << 2)
+} git_remote_redirect_t;
+
+/**
  * Remote creation options flags
  */
 typedef enum {
@@ -718,6 +742,13 @@ typedef struct {
 	git_proxy_options proxy_opts;
 
 	/**
+	 * Whether to allow off-site redirects.  If this is not
+	 * specified, the `http.followRedirects` configuration setting
+	 * will be consulted.
+	 */
+	git_remote_redirect_t follow_redirects;
+
+	/**
 	 * Extra headers for this fetch operation
 	 */
 	git_strarray custom_headers;
@@ -769,6 +800,13 @@ typedef struct {
 	git_proxy_options proxy_opts;
 
 	/**
+	 * Whether to allow off-site redirects.  If this is not
+	 * specified, the `http.followRedirects` configuration setting
+	 * will be consulted.
+	 */
+	git_remote_redirect_t follow_redirects;
+
+	/**
 	 * Extra headers for this push operation
 	 */
 	git_strarray custom_headers;
@@ -807,6 +845,13 @@ typedef struct {
 	/** HTTP Proxy settings */
 	git_proxy_options proxy_opts;
 
+	/**
+	 * Whether to allow off-site redirects.  If this is not
+	 * specified, the `http.followRedirects` configuration setting
+	 * will be consulted.
+	 */
+	git_remote_redirect_t follow_redirects;
+
 	/** Extra HTTP headers to use in this connection */
 	git_strarray custom_headers;
 } git_remote_connect_options;
diff --git a/src/remote.c b/src/remote.c
index 208d8d0..2e818cd 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1171,6 +1171,7 @@ static int ls_to_vector(git_vector *out, git_remote *remote)
 		(out)->callbacks = (in)->callbacks; \
 		(out)->proxy_opts = (in)->proxy_opts; \
 		(out)->custom_headers = (in)->custom_headers; \
+		(out)->follow_redirects = (in)->follow_redirects; \
 	}
 
 GIT_INLINE(int) connect_opts_from_fetch_opts(
diff --git a/src/transports/http.c b/src/transports/http.c
index 6a17fa9..7db5582 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -38,7 +38,8 @@ typedef struct {
 	const char *url;
 	const char *request_type;
 	const char *response_type;
-	unsigned chunked : 1;
+	unsigned int initial : 1,
+	             chunked : 1;
 } http_service;
 
 typedef struct {
@@ -70,24 +71,28 @@ static const http_service upload_pack_ls_service = {
 	GIT_HTTP_METHOD_GET, "/info/refs?service=git-upload-pack",
 	NULL,
 	"application/x-git-upload-pack-advertisement",
+	1,
 	0
 };
 static const http_service upload_pack_service = {
 	GIT_HTTP_METHOD_POST, "/git-upload-pack",
 	"application/x-git-upload-pack-request",
 	"application/x-git-upload-pack-result",
+	0,
 	0
 };
 static const http_service receive_pack_ls_service = {
 	GIT_HTTP_METHOD_GET, "/info/refs?service=git-receive-pack",
 	NULL,
 	"application/x-git-receive-pack-advertisement",
+	1,
 	0
 };
 static const http_service receive_pack_service = {
 	GIT_HTTP_METHOD_POST, "/git-receive-pack",
 	"application/x-git-receive-pack-request",
 	"application/x-git-receive-pack-result",
+	0,
 	1
 };
 
@@ -215,6 +220,19 @@ GIT_INLINE(int) handle_proxy_auth(
 		connect_opts->proxy_opts.payload);
 }
 
+static bool allow_redirect(http_stream *stream)
+{
+	http_subtransport *transport = OWNING_SUBTRANSPORT(stream);
+
+	switch (transport->owner->connect_opts.follow_redirects) {
+	case GIT_REMOTE_REDIRECT_INITIAL:
+		return (stream->service->initial == 1);
+	case GIT_REMOTE_REDIRECT_ALL:
+		return true;
+	default:
+		return false;
+	}
+}
 
 static int handle_response(
 	bool *complete,
@@ -233,7 +251,7 @@ static int handle_response(
 			return -1;
 		}
 
-		if (git_net_url_apply_redirect(&transport->server.url, response->location, false, stream->service->url) < 0) {
+		if (git_net_url_apply_redirect(&transport->server.url, response->location, allow_redirect(stream), stream->service->url) < 0) {
 			return -1;
 		}
 
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index fdc6cf2..8ec5b37 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -1189,8 +1189,10 @@ replay:
 			winhttp_stream_close(s);
 
 			if (!git__prefixcmp_icase(location8, prefix_https)) {
+				bool follow = (t->owner->connect_opts.follow_redirects != GIT_REMOTE_REDIRECT_NONE);
+
 				/* Upgrade to secure connection; disconnect and start over */
-				if (git_net_url_apply_redirect(&t->server.url, location8, false, s->service_url) < 0) {
+				if (git_net_url_apply_redirect(&t->server.url, location8, follow, s->service_url) < 0) {
 					git__free(location8);
 					return -1;
 				}
diff --git a/tests/online/clone.c b/tests/online/clone.c
index 8186dda..9f2580b 100644
--- a/tests/online/clone.c
+++ b/tests/online/clone.c
@@ -32,6 +32,8 @@ static char *_remote_proxy_user = NULL;
 static char *_remote_proxy_pass = NULL;
 static char *_remote_proxy_selfsigned = NULL;
 static char *_remote_expectcontinue = NULL;
+static char *_remote_redirect_initial = NULL;
+static char *_remote_redirect_subsequent = NULL;
 
 static int _orig_proxies_need_reset = 0;
 static char *_orig_http_proxy = NULL;
@@ -78,6 +80,8 @@ void test_online_clone__initialize(void)
 	_remote_proxy_pass = cl_getenv("GITTEST_REMOTE_PROXY_PASS");
 	_remote_proxy_selfsigned = cl_getenv("GITTEST_REMOTE_PROXY_SELFSIGNED");
 	_remote_expectcontinue = cl_getenv("GITTEST_REMOTE_EXPECTCONTINUE");
+	_remote_redirect_initial = cl_getenv("GITTEST_REMOTE_REDIRECT_INITIAL");
+	_remote_redirect_subsequent = cl_getenv("GITTEST_REMOTE_REDIRECT_SUBSEQUENT");
 
 	if (_remote_expectcontinue)
 		git_libgit2_opts(GIT_OPT_ENABLE_HTTP_EXPECT_CONTINUE, 1);
@@ -92,6 +96,8 @@ void test_online_clone__cleanup(void)
 		g_repo = NULL;
 	}
 	cl_fixture_cleanup("./foo");
+	cl_fixture_cleanup("./initial");
+	cl_fixture_cleanup("./subsequent");
 
 	git__free(_remote_url);
 	git__free(_remote_user);
@@ -107,6 +113,8 @@ void test_online_clone__cleanup(void)
 	git__free(_remote_proxy_pass);
 	git__free(_remote_proxy_selfsigned);
 	git__free(_remote_expectcontinue);
+	git__free(_remote_redirect_initial);
+	git__free(_remote_redirect_subsequent);
 
 	if (_orig_proxies_need_reset) {
 		cl_setenv("HTTP_PROXY", _orig_http_proxy);
@@ -938,3 +946,59 @@ void test_online_clone__path_whitespace(void)
 	cl_git_pass(git_clone(&g_repo, "https://libgit2@dev.azure.com/libgit2/test/_git/spaces%20in%20the%20name", "./foo", &g_options));
 	cl_assert(git_fs_path_exists("./foo/master.txt"));
 }
+
+void test_online_clone__redirect_default_succeeds_for_initial(void)
+{
+	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
+
+	if (!_remote_redirect_initial || !_remote_redirect_subsequent)
+		cl_skip();
+
+	cl_git_pass(git_clone(&g_repo, _remote_redirect_initial, "./initial", &options));
+}
+
+void test_online_clone__redirect_default_fails_for_subsequent(void)
+{
+	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
+
+	if (!_remote_redirect_initial || !_remote_redirect_subsequent)
+		cl_skip();
+
+	cl_git_fail(git_clone(&g_repo, _remote_redirect_subsequent, "./fail", &options));
+}
+
+void test_online_clone__redirect_none(void)
+{
+	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
+
+	if (!_remote_redirect_initial)
+		cl_skip();
+
+	options.fetch_opts.follow_redirects = GIT_REMOTE_REDIRECT_NONE;
+
+	cl_git_fail(git_clone(&g_repo, _remote_redirect_initial, "./fail", &options));
+}
+
+void test_online_clone__redirect_initial_succeeds_for_initial(void)
+{
+	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
+
+	if (!_remote_redirect_initial || !_remote_redirect_subsequent)
+		cl_skip();
+
+	options.fetch_opts.follow_redirects = GIT_REMOTE_REDIRECT_INITIAL;
+
+	cl_git_pass(git_clone(&g_repo, _remote_redirect_initial, "./initial", &options));
+}
+
+void test_online_clone__redirect_initial_fails_for_subsequent(void)
+{
+	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
+
+	if (!_remote_redirect_initial || !_remote_redirect_subsequent)
+		cl_skip();
+
+	options.fetch_opts.follow_redirects = GIT_REMOTE_REDIRECT_INITIAL;
+
+	cl_git_fail(git_clone(&g_repo, _remote_redirect_subsequent, "./fail", &options));
+}