streams: sockets are non-blocking and can timeout

Make socket I/O non-blocking and add optional timeouts.

Users may now set `GIT_OPT_SET_SERVER_CONNECT_TIMEOUT` to set a shorter
connection timeout. (The connect timeout cannot be longer than the
operating system default.) Users may also now configure the socket read
and write timeouts with `GIT_OPT_SET_SERVER_TIMEOUT`.

By default, connects still timeout based on the operating system
defaults (typically 75 seconds) and socket read and writes block.

Add a test against our custom testing git server that ensures that we
can timeout reads against a slow server.

ci/test.sh

@@ -271,9 +271,13 @@ if [ -z "$SKIP_ONLINE_TESTS" ]; then
 
 	export GITTEST_REMOTE_REDIRECT_INITIAL="http://localhost:9000/initial-redirect/libgit2/TestGitRepository"
 	export GITTEST_REMOTE_REDIRECT_SUBSEQUENT="http://localhost:9000/subsequent-redirect/libgit2/TestGitRepository"
+	export GITTEST_REMOTE_SPEED_SLOW="http://localhost:9000/speed-9600/test.git"
+	export GITTEST_REMOTE_SPEED_TIMESOUT="http://localhost:9000/speed-0.5/test.git"
 	run_test online
 	unset GITTEST_REMOTE_REDIRECT_INITIAL
 	unset GITTEST_REMOTE_REDIRECT_SUBSEQUENT
+	unset GITTEST_REMOTE_SPEED_SLOW
+	unset GITTEST_REMOTE_SPEED_TIMESOUT
 
 	# Run the online tests that immutably change global state separately
 	# to avoid polluting the test environment.

include/git2/common.h

@@ -224,7 +224,11 @@ typedef enum {
 	GIT_OPT_GET_OWNER_VALIDATION,
 	GIT_OPT_SET_OWNER_VALIDATION,
 	GIT_OPT_GET_HOMEDIR,
-	GIT_OPT_SET_HOMEDIR
+	GIT_OPT_SET_HOMEDIR,
+	GIT_OPT_SET_SERVER_CONNECT_TIMEOUT,
+	GIT_OPT_GET_SERVER_CONNECT_TIMEOUT,
+	GIT_OPT_SET_SERVER_TIMEOUT,
+	GIT_OPT_GET_SERVER_TIMEOUT
 } git_libgit2_opt_t;
 
 /**
@@ -480,6 +484,27 @@ typedef enum {
  *      >
  *      > - `path` directory of home directory.
  *
+ *   opts(GIT_OPT_GET_SERVER_CONNECT_TIMEOUT, int *timeout)
+ *      > Gets the timeout (in milliseconds) to attempt connections to
+ *      > a remote server.
+ *
+ *   opts(GIT_OPT_SET_SERVER_CONNECT_TIMEOUT, int timeout)
+ *      > Sets the timeout (in milliseconds) to attempt connections to
+ *      > a remote server. This is supported only for HTTP(S) connections
+ *      > and is not supported by SSH. Set to 0 to use the system default.
+ *      > Note that this may not be able to be configured longer than the
+ *      > system default, typically 75 seconds.
+ *
+ *   opts(GIT_OPT_GET_SERVER_TIMEOUT, int *timeout)
+ *      > Gets the timeout (in milliseconds) for reading from and writing
+ *      > to a remote server.
+ *
+ *   opts(GIT_OPT_SET_SERVER_TIMEOUT, int timeout)
+ *      > Sets the timeout (in milliseconds) for reading from and writing
+ *      > to a remote server. This is supported only for HTTP(S)
+ *      > connections and is not supported by SSH. Set to 0 to use the
+ *      > system default.
+ *
  * @param option Option key
  * @param ... value to set the option
  * @return 0 on success, <0 on failure

include/git2/errors.h

@@ -58,7 +58,8 @@ typedef enum {
 	GIT_EMISMATCH       = -33,	/**< Hashsum mismatch in object */
 	GIT_EINDEXDIRTY     = -34,	/**< Unsaved changes in the index would be overwritten */
 	GIT_EAPPLYFAIL      = -35,	/**< Patch application failed */
-	GIT_EOWNER          = -36	/**< The object is not owned by the current user */
+	GIT_EOWNER          = -36,	/**< The object is not owned by the current user */
+	GIT_TIMEOUT         = -37	/**< The operation timed out */
 } git_error_code;
 
 /**

include/git2/sys/stream.h

@@ -29,8 +29,22 @@ GIT_BEGIN_DECL
 typedef struct git_stream {
 	int version;
 
-	int encrypted;
-	int proxy_support;
+	int encrypted : 1,
+	    proxy_support : 1;
+
+	/**
+	 * Timeout for read and write operations; can be set to `0` to
+	 * block indefinitely.
+	 */
+	int timeout;
+
+	/**
+	 * Timeout to connect to the remote server; can be set to `0`
+	 * to use the system defaults. This can be shorter than the
+	 * system default - often 75 seconds - but cannot be longer.
+	 */
+	int connect_timeout;
+
 	int GIT_CALLBACK(connect)(struct git_stream *);
 	int GIT_CALLBACK(certificate)(git_cert **, struct git_stream *);
 	int GIT_CALLBACK(set_proxy)(struct git_stream *, const git_proxy_options *proxy_opts);

src/libgit2/libgit2.c

@@ -48,6 +48,8 @@ extern size_t git_indexer__max_objects;
 extern bool git_disable_pack_keep_file_checks;
 extern int git_odb__packed_priority;
 extern int git_odb__loose_priority;
+extern int git_socket_stream__connect_timeout;
+extern int git_socket_stream__timeout;
 
 char *git__user_agent;
 char *git__ssl_ciphers;
@@ -436,6 +438,40 @@ int git_libgit2_opts(int key, ...)
 		error = git_sysdir_set(GIT_SYSDIR_HOME, va_arg(ap, const char *));
 		break;
 
+	case GIT_OPT_GET_SERVER_CONNECT_TIMEOUT:
+		*(va_arg(ap, int *)) = git_socket_stream__connect_timeout;
+		break;
+
+	case GIT_OPT_SET_SERVER_CONNECT_TIMEOUT:
+		{
+			int timeout = va_arg(ap, int);
+
+			if (timeout < 0) {
+				git_error_set(GIT_ERROR_INVALID, "invalid connect timeout");
+				error = -1;
+			} else {
+				git_socket_stream__connect_timeout = timeout;
+			}
+		}
+		break;
+
+	case GIT_OPT_GET_SERVER_TIMEOUT:
+		*(va_arg(ap, int *)) = git_socket_stream__timeout;
+		break;
+
+	case GIT_OPT_SET_SERVER_TIMEOUT:
+		{
+			int timeout = va_arg(ap, int);
+
+			if (timeout < 0) {
+				git_error_set(GIT_ERROR_INVALID, "invalid timeout");
+				error = -1;
+			} else {
+				git_socket_stream__timeout = timeout;
+			}
+		}
+		break;
+
 	default:
 		git_error_set(GIT_ERROR_INVALID, "invalid option key");
 		error = -1;

src/libgit2/streams/socket.c

@@ -20,6 +20,7 @@
 # include <netdb.h>
 # include <netinet/in.h>
 # include <arpa/inet.h>
+# include <poll.h>
 #else
 # include <winsock2.h>
 # include <ws2tcpip.h>
@@ -28,6 +29,9 @@
 # endif
 #endif
 
+int git_socket_stream__connect_timeout = 0;
+int git_socket_stream__timeout = 0;
+
 #ifdef GIT_WIN32
 static void net_set_error(const char *str)
 {
@@ -66,21 +70,105 @@ static int close_socket(GIT_SOCKET s)
 
 }
 
+static int set_nonblocking(GIT_SOCKET s)
+{
+#ifdef GIT_WIN32
+	unsigned long nonblocking = 1;
+
+	if (ioctlsocket(s, FIONBIO, &nonblocking) != 0) {
+		net_set_error("could not set socket non-blocking");
+		return -1;
+	}
+#else
+	int flags;
+
+	if ((flags = fcntl(s, F_GETFL, 0)) == -1) {
+		net_set_error("could not query socket flags");
+		return -1;
+	}
+
+	flags |= O_NONBLOCK;
+
+	if (fcntl(s, F_SETFL, flags) != 0) {
+		net_set_error("could not set socket non-blocking");
+		return -1;
+	}
+#endif
+
+	return 0;
+}
+
+/* Promote a sockerr to an errno for our error handling routines */
+static int handle_sockerr(GIT_SOCKET socket)
+{
+	int sockerr;
+	socklen_t errlen = sizeof(sockerr);
+
+	if (getsockopt(socket, SOL_SOCKET, SO_ERROR, &sockerr, &errlen) < 0)
+		return -1;
+
+	if (sockerr == ETIMEDOUT)
+		return GIT_TIMEOUT;
+
+	errno = sockerr;
+	return -1;
+}
+
+static int connect_with_timeout(
+	GIT_SOCKET socket,
+	const struct sockaddr *address,
+	socklen_t address_len,
+	int timeout)
+{
+	struct pollfd fd;
+	int error;
+
+	if (timeout && (error = set_nonblocking(socket)) < 0)
+		return error;
+
+	error = connect(socket, address, address_len);
+
+	if (error == 0 || (error == -1 && errno != EINPROGRESS))
+		return error;
+
+	fd.fd = socket;
+	fd.events = POLLOUT;
+	fd.revents = 0;
+
+	error = poll(&fd, 1, timeout);
+
+	if (error == 0) {
+		return GIT_TIMEOUT;
+	} else if (error != 1) {
+		return -1;
+	} else if ((fd.revents & (POLLHUP | POLLERR))) {
+		return handle_sockerr(socket);
+	} else if ((fd.revents & POLLOUT) != POLLOUT) {
+		git_error_set(GIT_ERROR_NET,
+			"unknown error while polling for connect: %d",
+			fd.revents);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int socket_connect(git_stream *stream)
 {
-	struct addrinfo *info = NULL, *p;
-	struct addrinfo hints;
 	git_socket_stream *st = (git_socket_stream *) stream;
 	GIT_SOCKET s = INVALID_SOCKET;
-	int ret;
+	struct addrinfo *info = NULL, *p;
+	struct addrinfo hints;
+	int error;
 
 	memset(&hints, 0x0, sizeof(struct addrinfo));
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_family = AF_UNSPEC;
 
-	if ((ret = p_getaddrinfo(st->host, st->port, &hints, &info)) != 0) {
+	if ((error = p_getaddrinfo(st->host, st->port, &hints, &info)) != 0) {
 		git_error_set(GIT_ERROR_NET,
-			   "failed to resolve address for %s: %s", st->host, p_gai_strerror(ret));
+			   "failed to resolve address for %s: %s",
+			   st->host, p_gai_strerror(error));
 		return -1;
 	}
 
@@ -90,51 +178,115 @@ static int socket_connect(git_stream *stream)
 		if (s == INVALID_SOCKET)
 			continue;
 
-		if (connect(s, p->ai_addr, (socklen_t)p->ai_addrlen) == 0)
+		error = connect_with_timeout(s, p->ai_addr,
+				(socklen_t)p->ai_addrlen,
+				st->parent.connect_timeout);
+
+		if (error == 0)
 			break;
 
 		/* If we can't connect, try the next one */
 		close_socket(s);
 		s = INVALID_SOCKET;
+
+		if (error == GIT_TIMEOUT)
+			break;
 	}
 
 	/* Oops, we couldn't connect to any address */
-	if (s == INVALID_SOCKET && p == NULL) {
-		git_error_set(GIT_ERROR_OS, "failed to connect to %s", st->host);
-		p_freeaddrinfo(info);
-		return -1;
+	if (s == INVALID_SOCKET) {
+		if (error == GIT_TIMEOUT)
+			git_error_set(GIT_ERROR_NET, "failed to connect to %s: Operation timed out", st->host);
+		else
+			git_error_set(GIT_ERROR_OS, "failed to connect to %s", st->host);
+		error = -1;
+		goto done;
 	}
 
+	if (st->parent.timeout && !st->parent.connect_timeout &&
+	    (error = set_nonblocking(s)) < 0)
+		return error;
+
 	st->s = s;
+	error = 0;
+
+done:
 	p_freeaddrinfo(info);
-	return 0;
+	return error;
 }
 
-static ssize_t socket_write(git_stream *stream, const char *data, size_t len, int flags)
+static ssize_t socket_write(
+	git_stream *stream,
+	const char *data,
+	size_t len,
+	int flags)
 {
 	git_socket_stream *st = (git_socket_stream *) stream;
-	ssize_t written;
+	struct pollfd fd;
+	ssize_t ret;
 
 	GIT_ASSERT(flags == 0);
 	GIT_UNUSED(flags);
 
-	errno = 0;
+	ret = p_send(st->s, data, len, 0);
+
+	if (st->parent.timeout && ret < 0 &&
+	    (errno == EAGAIN || errno != EWOULDBLOCK)) {
+		fd.fd = st->s;
+		fd.events = POLLOUT;
+		fd.revents = 0;
+
+		ret = poll(&fd, 1, st->parent.timeout);
 
-	if ((written = p_send(st->s, data, len, 0)) < 0) {
-		net_set_error("error sending data");
+		if (ret == 1) {
+			ret = p_send(st->s, data, len, 0);
+		} else if (ret == 0) {
+			git_error_set(GIT_ERROR_NET,
+				"could not write to socket: timed out");
+			return GIT_TIMEOUT;
+		}
+	}
+
+	if (ret < 0) {
+		net_set_error("error receiving data from socket");
 		return -1;
 	}
 
-	return written;
+	return ret;
 }
 
-static ssize_t socket_read(git_stream *stream, void *data, size_t len)
+static ssize_t socket_read(
+	git_stream *stream,
+	void *data,
+	size_t len)
 {
-	ssize_t ret;
 	git_socket_stream *st = (git_socket_stream *) stream;
+	struct pollfd fd;
+	ssize_t ret;
+
+	ret = p_recv(st->s, data, len, 0);
+
+	if (st->parent.timeout && ret < 0 &&
+	    (errno == EAGAIN || errno != EWOULDBLOCK)) {
+		fd.fd = st->s;
+		fd.events = POLLIN;
+		fd.revents = 0;
 
-	if ((ret = p_recv(st->s, data, len, 0)) < 0)
-		net_set_error("error receiving socket data");
+		ret = poll(&fd, 1, st->parent.timeout);
+
+		if (ret == 1) {
+			ret = p_recv(st->s, data, len, 0);
+		} else if (ret == 0) {
+			git_error_set(GIT_ERROR_NET,
+				"could not read from socket: timed out");
+			return GIT_TIMEOUT;
+		}
+	}
+
+	if (ret < 0) {
+		net_set_error("error receiving data from socket");
+		return -1;
+	}
 
 	return ret;
 }
@@ -182,6 +334,8 @@ static int default_socket_stream_new(
 	}
 
 	st->parent.version = GIT_STREAM_VERSION;
+	st->parent.timeout = git_socket_stream__timeout;
+	st->parent.connect_timeout = git_socket_stream__connect_timeout;
 	st->parent.connect = socket_connect;
 	st->parent.write = socket_write;
 	st->parent.read = socket_read;
@@ -248,7 +402,7 @@ int git_socket_stream_global_init(void)
 
 	return git_runtime_shutdown_register(socket_stream_global_shutdown);
 }
- 
+
 #else
 
 #include "stream.h"

src/libgit2/streams/stransport.c

@@ -161,7 +161,9 @@ static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
 
 	if (ret < 0) {
 		st->error = ret;
-		return -36; /* ioErr */
+		return (ret == GIT_TIMEOUT) ?
+		       errSSLNetworkTimeout :
+		       -36 /* ioErr */;
 	}
 
 	return noErr;
@@ -176,8 +178,12 @@ static ssize_t stransport_write(git_stream *stream, const char *data, size_t len
 	GIT_UNUSED(flags);
 
 	data_len = min(len, SSIZE_MAX);
-	if ((ret = SSLWrite(st->ctx, data, data_len, &processed)) != noErr)
+	if ((ret = SSLWrite(st->ctx, data, data_len, &processed)) != noErr) {
+		if (st->error == GIT_TIMEOUT)
+			return GIT_TIMEOUT;
+
 		return stransport_error(ret);
+	}
 
 	GIT_ASSERT(processed < SSIZE_MAX);
 	return (ssize_t)processed;
@@ -207,7 +213,9 @@ static OSStatus read_cb(SSLConnectionRef conn, void *data, size_t *len)
 
 		if (ret < 0) {
 			st->error = ret;
-			error = -36; /* ioErr */
+			error = (ret == GIT_TIMEOUT) ?
+			        errSSLNetworkTimeout :
+			        -36 /* ioErr */;
 			break;
 		} else if (ret == 0) {
 			error = errSSLClosedGraceful;
@@ -228,6 +236,9 @@ static ssize_t stransport_read(git_stream *stream, void *data, size_t len)
 	OSStatus ret;
 
 	if ((ret = SSLRead(st->ctx, data, len, &processed)) != noErr) {
+		if (st->error == GIT_TIMEOUT)
+			return GIT_TIMEOUT;
+
 		return stransport_error(ret);
 	}
 

tests/libgit2/CMakeLists.txt

@@ -65,7 +65,7 @@ endif()
 
 include(AddClarTest)
 add_clar_test(libgit2_tests offline             -v -xonline)
-add_clar_test(libgit2_tests invasive            -v -sfilter::stream::bigfile -sodb::largefiles -siterator::workdir::filesystem_gunk -srepo::init -srepo::init::at_filesystem_root)
+add_clar_test(libgit2_tests invasive            -v -sfilter::stream::bigfile -sodb::largefiles -siterator::workdir::filesystem_gunk -srepo::init -srepo::init::at_filesystem_root -sonline::clone::connect_timeout_default)
 add_clar_test(libgit2_tests online              -v -sonline -xonline::customcert)
 add_clar_test(libgit2_tests online_customcert   -v -sonline::customcert)
 add_clar_test(libgit2_tests gitdaemon           -v -sonline::push)

tests/libgit2/online/clone.c

@@ -35,6 +35,8 @@ static char *_remote_proxy_selfsigned = NULL;
 static char *_remote_expectcontinue = NULL;
 static char *_remote_redirect_initial = NULL;
 static char *_remote_redirect_subsequent = NULL;
+static char *_remote_speed_timesout = NULL;
+static char *_remote_speed_slow = NULL;
 
 static char *_github_ssh_pubkey = NULL;
 static char *_github_ssh_privkey = NULL;
@@ -89,6 +91,8 @@ void test_online_clone__initialize(void)
 	_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");
+	_remote_speed_timesout = cl_getenv("GITTEST_REMOTE_SPEED_TIMESOUT");
+	_remote_speed_slow = cl_getenv("GITTEST_REMOTE_SPEED_SLOW");
 
 	_github_ssh_pubkey = cl_getenv("GITTEST_GITHUB_SSH_PUBKEY");
 	_github_ssh_privkey = cl_getenv("GITTEST_GITHUB_SSH_KEY");
@@ -128,6 +132,8 @@ void test_online_clone__cleanup(void)
 	git__free(_remote_expectcontinue);
 	git__free(_remote_redirect_initial);
 	git__free(_remote_redirect_subsequent);
+	git__free(_remote_speed_timesout);
+	git__free(_remote_speed_slow);
 
 	git__free(_github_ssh_pubkey);
 	git__free(_github_ssh_privkey);
@@ -145,6 +151,8 @@ void test_online_clone__cleanup(void)
 	}
 
 	git_libgit2_opts(GIT_OPT_SET_SSL_CERT_LOCATIONS, NULL, NULL);
+	git_libgit2_opts(GIT_OPT_SET_SERVER_TIMEOUT, 0);
+	git_libgit2_opts(GIT_OPT_SET_SERVER_CONNECT_TIMEOUT, 0);
 }
 
 void test_online_clone__network_full(void)
@@ -1207,3 +1215,55 @@ void test_online_clone__sha256(void)
 	git_reference_free(head);
 #endif
 }
+
+void test_online_clone__connect_timeout_configurable(void)
+{
+	uint64_t start, finish;
+
+	start = git_time_monotonic();
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SERVER_CONNECT_TIMEOUT, 1));
+	cl_git_fail(git_clone(&g_repo, "http://www.google.com:8000/", "./timedout", NULL));
+	cl_assert(git_error_last() && strstr(git_error_last()->message, "timed out"));
+
+	finish = git_time_monotonic();
+
+	cl_assert(finish - start < 1000);
+}
+
+void test_online_clone__connect_timeout_default(void)
+{
+	/* This test takes ~ 75 seconds on Unix. */
+	if (!cl_is_env_set("GITTEST_INVASIVE_SPEED"))
+		cl_skip();
+
+	/*
+	 * Use a host/port pair that blackholes packets and does not
+	 * send an RST.
+	 */
+	cl_git_fail_with(GIT_TIMEOUT, git_clone(&g_repo, "http://www.google.com:8000/", "./refused", NULL));
+	cl_assert(git_error_last() && strstr(git_error_last()->message, "timed out"));
+}
+
+void test_online_clone__timeout_configurable_times_out(void)
+{
+	git_repository *failed_repo;
+
+	if (!_remote_speed_timesout)
+		cl_skip();
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SERVER_TIMEOUT, 1000));
+
+	cl_git_fail_with(GIT_TIMEOUT, git_clone(&failed_repo, _remote_speed_timesout, "./timedout", NULL));
+	cl_assert(git_error_last() && strstr(git_error_last()->message, "timed out"));
+}
+
+void test_online_clone__timeout_configurable_succeeds_slowly(void)
+{
+	if (!_remote_speed_slow)
+		cl_skip();
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SERVER_TIMEOUT, 1000));
+
+	cl_git_pass(git_clone(&g_repo, _remote_speed_slow, "./slow-but-successful", NULL));
+}