Commit 954f5357282233ecdfea226c819f3f3884949cdb

Edward Thomson 2019-05-21T14:33:37

Merge pull request #5062 from tiennou/fix/ssh-url-substitution Remote URL last-chance resolution

diff --git a/include/git2/remote.h b/include/git2/remote.h
index 5a82558..f757f67 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -475,6 +475,20 @@ typedef int GIT_CALLBACK(git_push_negotiation)(const git_push_update **updates, 
 typedef int GIT_CALLBACK(git_push_update_reference_cb)(const char *refname, const char *status, void *data);
 
 /**
+ * Callback to resolve URLs before connecting to remote
+ *
+ * If you return GIT_PASSTHROUGH, you don't need to write anything to
+ * url_resolved.
+ *
+ * @param url_resolved The buffer to write the resolved URL to
+ * @param url The URL to resolve
+ * @param direction GIT_DIRECTION_FETCH or GIT_DIRECTION_PUSH
+ * @param payload Payload provided by the caller
+ * @return 0 on success, GIT_PASSTHROUGH or an error
+ */
+typedef int GIT_CALLBACK(git_url_resolve_cb)(git_buf *url_resolved, const char *url, int direction, void *payload);
+
+/**
  * The callback settings structure
  *
  * Set the callbacks to be called by the remote when informing the user
@@ -562,6 +576,12 @@ struct git_remote_callbacks {
 	 * as the last parameter.
 	 */
 	void *payload;
+
+	/**
+	 * Resolve URL before connecting to remote.
+	 * The returned URL will be used to connect to the remote instead.
+	 */
+	git_url_resolve_cb resolve_url;
 };
 
 #define GIT_REMOTE_CALLBACKS_VERSION 1
diff --git a/src/remote.c b/src/remote.c
index 3d55556..3cea790 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -670,21 +670,44 @@ int git_remote_set_pushurl(git_repository *repo, const char *remote, const char*
 	return set_url(repo, remote, CONFIG_PUSHURL_FMT, url);
 }
 
-const char* git_remote__urlfordirection(git_remote *remote, int direction)
+static int resolve_url(git_buf *resolved_url, const char *url, int direction, const git_remote_callbacks *callbacks)
 {
-	assert(remote);
+	int status;
+
+	if (callbacks && callbacks->resolve_url) {
+		git_buf_clear(resolved_url);
+		status = callbacks->resolve_url(resolved_url, url, direction, callbacks->payload);
+		if (status != GIT_PASSTHROUGH) {
+			git_error_set_after_callback_function(status, "git_resolve_url_cb");
+			git_buf_sanitize(resolved_url);
+			return status;
+		}
+	}
 
+	return git_buf_sets(resolved_url, url);
+}
+
+int git_remote__urlfordirection(git_buf *url_out, struct git_remote *remote, int direction, const git_remote_callbacks *callbacks)
+{
+	const char *url = NULL;
+
+	assert(remote);
 	assert(direction == GIT_DIRECTION_FETCH || direction == GIT_DIRECTION_PUSH);
 
 	if (direction == GIT_DIRECTION_FETCH) {
-		return remote->url;
+		url = remote->url;
+	} else if (direction == GIT_DIRECTION_PUSH) {
+		url = remote->pushurl ? remote->pushurl : remote->url;
 	}
 
-	if (direction == GIT_DIRECTION_PUSH) {
-		return remote->pushurl ? remote->pushurl : remote->url;
+	if (!url) {
+		git_error_set(GIT_ERROR_INVALID,
+			"malformed remote '%s' - missing %s URL",
+			remote->name ? remote->name : "(anonymous)",
+			direction == GIT_DIRECTION_FETCH ? "fetch" : "push");
+		return GIT_EINVALID;
 	}
-
-	return NULL;
+	return resolve_url(url_out, url, direction, callbacks);
 }
 
 int set_transport_callbacks(git_transport *t, const git_remote_callbacks *cbs)
@@ -707,7 +730,7 @@ static int set_transport_custom_headers(git_transport *t, const git_strarray *cu
 int git_remote__connect(git_remote *remote, git_direction direction, const git_remote_callbacks *callbacks, const git_remote_connection_opts *conn)
 {
 	git_transport *t;
-	const char *url;
+	git_buf url = GIT_BUF_INIT;
 	int flags = GIT_TRANSPORTFLAGS_NONE;
 	int error;
 	void *payload = NULL;
@@ -728,39 +751,38 @@ int git_remote__connect(git_remote *remote, git_direction direction, const git_r
 
 	t = remote->transport;
 
-	url = git_remote__urlfordirection(remote, direction);
-	if (url == NULL) {
-		git_error_set(GIT_ERROR_INVALID,
-			"Malformed remote '%s' - missing %s URL",
-			remote->name ? remote->name : "(anonymous)",
-			direction == GIT_DIRECTION_FETCH ? "fetch" : "push");
-		return -1;
-	}
+	if ((error = git_remote__urlfordirection(&url, remote, direction, callbacks)) < 0)
+		goto on_error;
 
 	/* If we don't have a transport object yet, and the caller specified a
 	 * custom transport factory, use that */
 	if (!t && transport &&
 		(error = transport(&t, remote, payload)) < 0)
-		return error;
+		goto on_error;
 
 	/* If we still don't have a transport, then use the global
 	 * transport registrations which map URI schemes to transport factories */
-	if (!t && (error = git_transport_new(&t, remote, url)) < 0)
-		return error;
+	if (!t && (error = git_transport_new(&t, remote, url.ptr)) < 0)
+		goto on_error;
 
 	if ((error = set_transport_custom_headers(t, conn->custom_headers)) != 0)
 		goto on_error;
 
 	if ((error = set_transport_callbacks(t, callbacks)) < 0 ||
-	    (error = t->connect(t, url, credentials, payload, conn->proxy, direction, flags)) != 0)
+	    (error = t->connect(t, url.ptr, credentials, payload, conn->proxy, direction, flags)) != 0)
 		goto on_error;
 
 	remote->transport = t;
 
+	git_buf_dispose(&url);
+
 	return 0;
 
 on_error:
-	t->free(t);
+	if (t)
+		t->free(t);
+
+	git_buf_dispose(&url);
 
 	if (t == remote->transport)
 		remote->transport = NULL;
diff --git a/src/remote.h b/src/remote.h
index 15f8d30..df75ed3 100644
--- a/src/remote.h
+++ b/src/remote.h
@@ -45,7 +45,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);
 
-const char* git_remote__urlfordirection(struct git_remote *remote, int direction);
+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(git_remote *remote, bool use_ssl, char **proxy_url);
 
 git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname);
diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c
index 1051795..a29281d 100644
--- a/tests/network/remote/remotes.c
+++ b/tests/network/remote/remotes.c
@@ -28,28 +28,97 @@ void test_network_remote_remotes__cleanup(void)
 
 void test_network_remote_remotes__parsing(void)
 {
+	git_buf url = GIT_BUF_INIT;
 	git_remote *_remote2 = NULL;
 
 	cl_assert_equal_s(git_remote_name(_remote), "test");
 	cl_assert_equal_s(git_remote_url(_remote), "git://github.com/libgit2/libgit2");
 	cl_assert(git_remote_pushurl(_remote) == NULL);
 
-	cl_assert_equal_s(git_remote__urlfordirection(_remote, GIT_DIRECTION_FETCH),
-					  "git://github.com/libgit2/libgit2");
-	cl_assert_equal_s(git_remote__urlfordirection(_remote, GIT_DIRECTION_PUSH),
-					  "git://github.com/libgit2/libgit2");
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, NULL));
+	cl_assert_equal_s(url.ptr, "git://github.com/libgit2/libgit2");
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, NULL));
+	cl_assert_equal_s(url.ptr, "git://github.com/libgit2/libgit2");
 
 	cl_git_pass(git_remote_lookup(&_remote2, _repo, "test_with_pushurl"));
 	cl_assert_equal_s(git_remote_name(_remote2), "test_with_pushurl");
 	cl_assert_equal_s(git_remote_url(_remote2), "git://github.com/libgit2/fetchlibgit2");
 	cl_assert_equal_s(git_remote_pushurl(_remote2), "git://github.com/libgit2/pushlibgit2");
 
-	cl_assert_equal_s(git_remote__urlfordirection(_remote2, GIT_DIRECTION_FETCH),
-					  "git://github.com/libgit2/fetchlibgit2");
-	cl_assert_equal_s(git_remote__urlfordirection(_remote2, GIT_DIRECTION_PUSH),
-					  "git://github.com/libgit2/pushlibgit2");
+	cl_git_pass(git_remote__urlfordirection(&url, _remote2, GIT_DIRECTION_FETCH, NULL));
+	cl_assert_equal_s(url.ptr, "git://github.com/libgit2/fetchlibgit2");
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote2, GIT_DIRECTION_PUSH, NULL));
+	cl_assert_equal_s(url.ptr, "git://github.com/libgit2/pushlibgit2");
 
 	git_remote_free(_remote2);
+	git_buf_dispose(&url);
+}
+
+static int urlresolve_callback(git_buf *url_resolved, const char *url, int direction, void *payload)
+{
+	cl_assert(strcmp(url, "git://github.com/libgit2/libgit2") == 0);
+	cl_assert(strcmp(payload, "payload") == 0);
+	cl_assert(url_resolved->size == 0);
+	
+	if (direction == GIT_DIRECTION_PUSH)
+		git_buf_sets(url_resolved, "pushresolve");
+	if (direction == GIT_DIRECTION_FETCH)
+		git_buf_sets(url_resolved, "fetchresolve");
+
+	return GIT_OK;
+}
+
+void test_network_remote_remotes__urlresolve(void)
+{
+	git_buf url = GIT_BUF_INIT;
+
+	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
+	callbacks.resolve_url = urlresolve_callback;
+	callbacks.payload = "payload";
+
+	cl_assert_equal_s(git_remote_name(_remote), "test");
+	cl_assert_equal_s(git_remote_url(_remote), "git://github.com/libgit2/libgit2");
+	cl_assert(git_remote_pushurl(_remote) == NULL);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, &callbacks));
+	cl_assert_equal_s(url.ptr, "fetchresolve");
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, &callbacks));
+	cl_assert_equal_s(url.ptr, "pushresolve");
+
+	git_buf_dispose(&url);
+}
+
+static int urlresolve_passthrough_callback(git_buf *url_resolved, const char *url, int direction, void *payload)
+{
+	GIT_UNUSED(url_resolved);
+	GIT_UNUSED(url);
+	GIT_UNUSED(direction);
+	GIT_UNUSED(payload);
+	return GIT_PASSTHROUGH;
+}
+
+void test_network_remote_remotes__urlresolve_passthrough(void)
+{
+	git_buf url = GIT_BUF_INIT;
+	const char *orig_url = "git://github.com/libgit2/libgit2";
+
+	git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
+	callbacks.resolve_url = urlresolve_passthrough_callback;
+
+	cl_assert_equal_s(git_remote_name(_remote), "test");
+	cl_assert_equal_s(git_remote_url(_remote), orig_url);
+	cl_assert(git_remote_pushurl(_remote) == NULL);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_FETCH, &callbacks));
+	cl_assert_equal_s(url.ptr, orig_url);
+
+	cl_git_pass(git_remote__urlfordirection(&url, _remote, GIT_DIRECTION_PUSH, &callbacks));
+	cl_assert_equal_s(url.ptr, orig_url);
+
+	git_buf_dispose(&url);
 }
 
 void test_network_remote_remotes__pushurl(void)
diff --git a/tests/online/push_util.h b/tests/online/push_util.h
index 570873c..d829bbc 100644
--- a/tests/online/push_util.h
+++ b/tests/online/push_util.h
@@ -12,7 +12,7 @@ extern const git_oid OID_ZERO;
  * @param data pointer to a record_callbacks_data instance
  */
 #define RECORD_CALLBACKS_INIT(data) \
-	{ GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, cred_acquire_cb, NULL, NULL, record_update_tips_cb, NULL, NULL, NULL, NULL, NULL, data }
+	{ GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, cred_acquire_cb, NULL, NULL, record_update_tips_cb, NULL, NULL, NULL, NULL, NULL, data, NULL }
 
 typedef struct {
 	char *name;