summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEdward Thomson <ethomson@edwardthomson.com>2023-05-12 20:48:30 +0100
committerEdward Thomson <ethomson@edwardthomson.com>2023-05-13 16:42:04 +0100
commitfad90428970e332153027773b517a1606c0efa1f (patch)
tree499ad297bc61c02e7151aa76e0cf205142fa150d
parent933b04c219b49d0f9cd779d024bfa39d806839ac (diff)
downloadlibgit2-fad90428970e332153027773b517a1606c0efa1f.tar.gz
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.
-rwxr-xr-xci/test.sh4
-rw-r--r--include/git2/common.h27
-rw-r--r--include/git2/errors.h3
-rw-r--r--include/git2/sys/stream.h18
-rw-r--r--src/libgit2/libgit2.c36
-rw-r--r--src/libgit2/streams/socket.c198
-rw-r--r--src/libgit2/streams/stransport.c17
-rw-r--r--tests/libgit2/CMakeLists.txt2
-rw-r--r--tests/libgit2/online/clone.c60
9 files changed, 335 insertions, 30 deletions
diff --git a/ci/test.sh b/ci/test.sh
index a6735231f..ee6801a79 100755
--- a/ci/test.sh
+++ b/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.
diff --git a/include/git2/common.h b/include/git2/common.h
index f968deb23..ab6bc1333 100644
--- a/include/git2/common.h
+++ b/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
diff --git a/include/git2/errors.h b/include/git2/errors.h
index e634a97c1..7180852f9 100644
--- a/include/git2/errors.h
+++ b/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;
/**
diff --git a/include/git2/sys/stream.h b/include/git2/sys/stream.h
index e0e03a2d7..3d28d09b3 100644
--- a/include/git2/sys/stream.h
+++ b/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);
diff --git a/src/libgit2/libgit2.c b/src/libgit2/libgit2.c
index 178880c9e..ce287147a 100644
--- a/src/libgit2/libgit2.c
+++ b/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;
diff --git a/src/libgit2/streams/socket.c b/src/libgit2/streams/socket.c
index 6994d58f2..0e0aa6934 100644
--- a/src/libgit2/streams/socket.c
+++ b/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"
diff --git a/src/libgit2/streams/stransport.c b/src/libgit2/streams/stransport.c
index 74ee0d1ee..d956df84d 100644
--- a/src/libgit2/streams/stransport.c
+++ b/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);
}
diff --git a/tests/libgit2/CMakeLists.txt b/tests/libgit2/CMakeLists.txt
index 866122d1c..af70f55a7 100644
--- a/tests/libgit2/CMakeLists.txt
+++ b/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)
diff --git a/tests/libgit2/online/clone.c b/tests/libgit2/online/clone.c
index b635739b6..92fa4d351 100644
--- a/tests/libgit2/online/clone.c
+++ b/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));
+}