Merge branch 'pks/stream-truncated-writes'

7 files changed, 73 insertions, 70 deletions

diff --git a/src/stream.h b/src/stream.h
index 00220d50e..f16b026fb 100644
--- a/src/stream.h
+++ b/src/stream.h
@@ -55,6 +55,21 @@ GIT_INLINE(ssize_t) git_stream_write(git_stream *st, const char *data, size_t le
 	return st->write(st, data, len, flags);
 }
 
+GIT_INLINE(int) git_stream__write_full(git_stream *st, const char *data, size_t len, int flags)
+{
+	size_t total_written = 0;
+
+	while (total_written < len) {
+		ssize_t written = git_stream_write(st, data + total_written, len - total_written, flags);
+		if (written <= 0)
+			return -1;
+
+		total_written += written;
+	}
+
+	return 0;
+}
+
 GIT_INLINE(int) git_stream_close(git_stream *st)
 {
 	return st->close(st);
diff --git a/src/streams/mbedtls.c b/src/streams/mbedtls.c
index 48d21dd69..cbe2f681a 100644
--- a/src/streams/mbedtls.c
+++ b/src/streams/mbedtls.c
@@ -42,9 +42,9 @@
 #define GIT_SSL_DEFAULT_CIPHERS "TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256:TLS-ECDHE-RSA-WITH-AES-128-GCM-SHA256:TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384:TLS-ECDHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-DSS-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-DSS-WITH-AES-256-GCM-SHA384:TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256:TLS-ECDHE-RSA-WITH-AES-128-CBC-SHA256:TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA:TLS-ECDHE-RSA-WITH-AES-128-CBC-SHA:TLS-ECDHE-ECDSA-WITH-AES-256-CBC-SHA384:TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA384:TLS-ECDHE-ECDSA-WITH-AES-256-CBC-SHA:TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-RSA-WITH-AES-128-CBC-SHA256:TLS-DHE-RSA-WITH-AES-256-CBC-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA:TLS-DHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-DSS-WITH-AES-128-CBC-SHA256:TLS-DHE-DSS-WITH-AES-256-CBC-SHA256:TLS-DHE-DSS-WITH-AES-128-CBC-SHA:TLS-DHE-DSS-WITH-AES-256-CBC-SHA:TLS-RSA-WITH-AES-128-GCM-SHA256:TLS-RSA-WITH-AES-256-GCM-SHA384:TLS-RSA-WITH-AES-128-CBC-SHA256:TLS-RSA-WITH-AES-256-CBC-SHA256:TLS-RSA-WITH-AES-128-CBC-SHA:TLS-RSA-WITH-AES-256-CBC-SHA"
 #define GIT_SSL_DEFAULT_CIPHERS_COUNT 30
 
-mbedtls_ssl_config *git__ssl_conf;
+static mbedtls_ssl_config *git__ssl_conf;
 static int ciphers_list[GIT_SSL_DEFAULT_CIPHERS_COUNT];
-mbedtls_entropy_context *mbedtls_entropy;
+static mbedtls_entropy_context *mbedtls_entropy;
 
 /**
  * This function aims to clean-up the SSL context which
@@ -166,18 +166,16 @@ cleanup:
 	return -1;
 }
 
-mbedtls_ssl_config *git__ssl_conf;
-
 static int bio_read(void *b, unsigned char *buf, size_t len)
 {
 	git_stream *io = (git_stream *) b;
-	return (int) git_stream_read(io, buf, len);
+	return (int) git_stream_read(io, buf, min(len, INT_MAX));
 }
 
 static int bio_write(void *b, const unsigned char *buf, size_t len)
 {
 	git_stream *io = (git_stream *) b;
-	return (int) git_stream_write(io, (const char *)buf, len, 0);
+	return (int) git_stream_write(io, (const char *)buf, min(len, INT_MAX), 0);
 }
 
 static int ssl_set_error(mbedtls_ssl_context *ssl, int error)
@@ -246,7 +244,7 @@ typedef struct {
 } mbedtls_stream;
 
 
-int mbedtls_connect(git_stream *stream)
+static int mbedtls_connect(git_stream *stream)
 {
 	int ret;
 	mbedtls_stream *st = (mbedtls_stream *) stream;
@@ -266,7 +264,7 @@ int mbedtls_connect(git_stream *stream)
 	return verify_server_cert(st->ssl);
 }
 
-int mbedtls_certificate(git_cert **out, git_stream *stream)
+static int mbedtls_certificate(git_cert **out, git_stream *stream)
 {
 	unsigned char *encoded_cert;
 	mbedtls_stream *st = (mbedtls_stream *) stream;
@@ -303,25 +301,27 @@ static int mbedtls_set_proxy(git_stream *stream, const git_proxy_options *proxy_
 	return git_stream_set_proxy(st->io, proxy_options);
 }
 
-ssize_t mbedtls_stream_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t mbedtls_stream_write(git_stream *stream, const char *data, size_t len, int flags)
 {
-	ssize_t written = 0, len = min(data_len, SSIZE_MAX);
 	mbedtls_stream *st = (mbedtls_stream *) stream;
+	int written;
 
 	GIT_UNUSED(flags);
 
-	do {
-		int error = mbedtls_ssl_write(st->ssl, (const unsigned char *)data + written, len - written);
-		if (error <= 0) {
-			return ssl_set_error(st->ssl, error);
-		}
-		written += error;
-	} while (written < len);
+	/*
+	 * `mbedtls_ssl_write` can only represent INT_MAX bytes
+	 * written via its return value. We thus need to clamp
+	 * the maximum number of bytes written.
+	 */
+	len = min(len, INT_MAX);
+
+	if ((written = mbedtls_ssl_write(st->ssl, (const unsigned char *)data, len)) <= 0)
+		return ssl_set_error(st->ssl, written);
 
 	return written;
 }
 
-ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
+static ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 	int ret;
@@ -332,7 +332,7 @@ ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int mbedtls_stream_close(git_stream *stream)
+static int mbedtls_stream_close(git_stream *stream)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 	int ret = 0;
@@ -345,7 +345,7 @@ int mbedtls_stream_close(git_stream *stream)
 	return st->owned ? git_stream_close(st->io) : 0;
 }
 
-void mbedtls_stream_free(git_stream *stream)
+static void mbedtls_stream_free(git_stream *stream)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 
diff --git a/src/streams/openssl.c b/src/streams/openssl.c
index 589b8d1f3..fe5f79cce 100644
--- a/src/streams/openssl.c
+++ b/src/streams/openssl.c
@@ -572,9 +572,7 @@ typedef struct {
 	git_cert_x509 cert_info;
 } openssl_stream;
 
-int openssl_close(git_stream *stream);
-
-int openssl_connect(git_stream *stream)
+static int openssl_connect(git_stream *stream)
 {
 	int ret;
 	BIO *bio;
@@ -602,7 +600,7 @@ int openssl_connect(git_stream *stream)
 	return verify_server_cert(st->ssl, st->host);
 }
 
-int openssl_certificate(git_cert **out, git_stream *stream)
+static int openssl_certificate(git_cert **out, git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int len;
@@ -644,21 +642,20 @@ static int openssl_set_proxy(git_stream *stream, const git_proxy_options *proxy_
 	return git_stream_set_proxy(st->io, proxy_opts);
 }
 
-ssize_t openssl_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t openssl_write(git_stream *stream, const char *data, size_t data_len, int flags)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret, len = min(data_len, INT_MAX);
 
 	GIT_UNUSED(flags);
 
-	if ((ret = SSL_write(st->ssl, data, len)) <= 0) {
+	if ((ret = SSL_write(st->ssl, data, len)) <= 0)
 		return ssl_set_error(st->ssl, ret);
-	}
 
 	return ret;
 }
 
-ssize_t openssl_read(git_stream *stream, void *data, size_t len)
+static ssize_t openssl_read(git_stream *stream, void *data, size_t len)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret;
@@ -669,7 +666,7 @@ ssize_t openssl_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int openssl_close(git_stream *stream)
+static int openssl_close(git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret;
@@ -682,7 +679,7 @@ int openssl_close(git_stream *stream)
 	return st->owned ? git_stream_close(st->io) : 0;
 }
 
-void openssl_free(git_stream *stream)
+static void openssl_free(git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 
diff --git a/src/streams/socket.c b/src/streams/socket.c
index e46fcd219..066580f64 100644
--- a/src/streams/socket.c
+++ b/src/streams/socket.c
@@ -69,7 +69,7 @@ static int close_socket(GIT_SOCKET s)
 
 }
 
-int socket_connect(git_stream *stream)
+static int socket_connect(git_stream *stream)
 {
 	struct addrinfo *info = NULL, *p;
 	struct addrinfo hints;
@@ -130,26 +130,22 @@ int socket_connect(git_stream *stream)
 	return 0;
 }
 
-ssize_t socket_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t socket_write(git_stream *stream, const char *data, size_t len, int flags)
 {
-	ssize_t ret, off = 0, len = min(data_len, SSIZE_MAX);
 	git_socket_stream *st = (git_socket_stream *) stream;
+	ssize_t written;
 
-	while (off < len) {
-		errno = 0;
-		ret = p_send(st->s, data + off, len - off, flags);
-		if (ret < 0) {
-			net_set_error("Error sending data");
-			return -1;
-		}
+	errno = 0;
 
-		off += ret;
+	if ((written = p_send(st->s, data, len, flags)) < 0) {
+		net_set_error("Error sending data");
+		return -1;
 	}
 
-	return off;
+	return written;
 }
 
-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;
@@ -160,7 +156,7 @@ ssize_t socket_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int socket_close(git_stream *stream)
+static int socket_close(git_stream *stream)
 {
 	git_socket_stream *st = (git_socket_stream *) stream;
 	int error;
@@ -171,7 +167,7 @@ int socket_close(git_stream *stream)
 	return error;
 }
 
-void socket_free(git_stream *stream)
+static void socket_free(git_stream *stream)
 {
 	git_socket_stream *st = (git_socket_stream *) stream;
 
diff --git a/src/streams/stransport.c b/src/streams/stransport.c
index a999bb5a0..a79d3cbf0 100644
--- a/src/streams/stransport.c
+++ b/src/streams/stransport.c
@@ -149,9 +149,8 @@ static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
 {
 	git_stream *io = (git_stream *) conn;
 
-	if (git_stream_write(io, data, *len, 0) < 0) {
+	if (git_stream__write_full(io, data, *len, 0) < 0)
 		return -36; /* "ioErr" from MacErrors.h which is not available on iOS */
-	}
 
 	return noErr;
 }
diff --git a/src/transports/git.c b/src/transports/git.c
index 8d5a9d903..9fd3b47fc 100644
--- a/src/transports/git.c
+++ b/src/transports/git.c
@@ -76,18 +76,15 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url)
 static int send_command(git_proto_stream *s)
 {
 	git_buf request = GIT_BUF_INIT;
-	size_t write_size;
 	int error;
 
-	error = gen_proto(&request, s->cmd, s->url);
-	if (error < 0)
+	if ((error = gen_proto(&request, s->cmd, s->url)) < 0)
 		goto cleanup;
 
-	write_size = min(request.size, INT_MAX);
-	error = (int)git_stream_write(s->io, request.ptr, write_size, 0);
+	if ((error = git_stream__write_full(s->io, request.ptr, request.size, 0)) < 0)
+		goto cleanup;
 
-	if (error >= 0)
-		s->sent_command = 1;
+	s->sent_command = 1;
 
 cleanup:
 	git_buf_dispose(&request);
@@ -122,16 +119,15 @@ static int git_proto_stream_read(
 static int git_proto_stream_write(
 	git_smart_subtransport_stream *stream,
 	const char *buffer,
-	size_t buffer_len)
+	size_t len)
 {
 	git_proto_stream *s = (git_proto_stream *)stream;
-	size_t len = min(buffer_len, INT_MAX);
 	int error;
 
 	if (!s->sent_command && (error = send_command(s)) < 0)
 		return error;
 
-	return (int)git_stream_write(s->io, buffer, len, 0);
+	return git_stream__write_full(s->io, buffer, len, 0);
 }
 
 static void git_proto_stream_free(git_smart_subtransport_stream *stream)
diff --git a/src/transports/http.c b/src/transports/http.c
index 2168072f2..80ba5ba73 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -643,7 +643,7 @@ static int write_chunk(git_stream *io, const char *buffer, size_t len)
 	if (git_buf_oom(&buf))
 		return -1;
 
-	if (git_stream_write(io, buf.ptr, buf.size, 0) < 0) {
+	if (git_stream__write_full(io, buf.ptr, buf.size, 0) < 0) {
 		git_buf_dispose(&buf);
 		return -1;
 	}
@@ -651,11 +651,11 @@ static int write_chunk(git_stream *io, const char *buffer, size_t len)
 	git_buf_dispose(&buf);
 
 	/* Chunk body */
-	if (len > 0 && git_stream_write(io, buffer, len, 0) < 0)
+	if (len > 0 && git_stream__write_full(io, buffer, len, 0) < 0)
 		return -1;
 
 	/* Chunk footer */
-	if (git_stream_write(io, "\r\n", 2, 0) < 0)
+	if (git_stream__write_full(io, "\r\n", 2, 0) < 0)
 		return -1;
 
 	return 0;
@@ -853,8 +853,8 @@ replay:
 	if ((error = gen_connect_req(&request, t)) < 0)
 		goto done;
 
-	if ((error = git_stream_write(proxy_stream,
-	    request.ptr, request.size, 0)) < 0)
+	if ((error = git_stream__write_full(proxy_stream, request.ptr,
+					    request.size, 0)) < 0)
 		goto done;
 
 	git_buf_dispose(&request);
@@ -1034,8 +1034,8 @@ replay:
 		if (gen_request(&request, s, 0) < 0)
 			return -1;
 
-		if (git_stream_write(t->server.stream,
-		    request.ptr, request.size, 0) < 0) {
+		if (git_stream__write_full(t->server.stream, request.ptr,
+					   request.size, 0) < 0) {
 			git_buf_dispose(&request);
 			return -1;
 		}
@@ -1058,7 +1058,8 @@ replay:
 			s->chunk_buffer_len = 0;
 
 			/* Write the final chunk. */
-			if (git_stream_write(t->server.stream, "0\r\n\r\n", 5, 0) < 0)
+			if (git_stream__write_full(t->server.stream,
+						   "0\r\n\r\n", 5, 0) < 0)
 				return -1;
 		}
 
@@ -1157,8 +1158,8 @@ static int http_stream_write_chunked(
 		if (gen_request(&request, s, 0) < 0)
 			return -1;
 
-		if (git_stream_write(t->server.stream,
-		    request.ptr, request.size, 0) < 0) {
+		if (git_stream__write_full(t->server.stream, request.ptr,
+					   request.size, 0) < 0) {
 			git_buf_dispose(&request);
 			return -1;
 		}
@@ -1233,11 +1234,10 @@ static int http_stream_write_single(
 	if (gen_request(&request, s, len) < 0)
 		return -1;
 
-	if (git_stream_write(t->server.stream,
-	    request.ptr, request.size, 0) < 0)
+	if (git_stream__write_full(t->server.stream, request.ptr, request.size, 0) < 0)
 		goto on_error;
 
-	if (len && git_stream_write(t->server.stream, buffer, len, 0) < 0)
+	if (len && git_stream__write_full(t->server.stream, buffer, len, 0) < 0)
 		goto on_error;
 
 	git_buf_dispose(&request);