streams: fix callers potentially only writing partial data

Similar to the write(3) function, implementations of `git_stream_write`
do not guarantee that all bytes are written. Instead, they return the
number of bytes that actually have been written, which may be smaller
than the total number of bytes. Furthermore, due to an interface design
issue, we cannot ever write more than `SSIZE_MAX` bytes at once, as
otherwise we cannot represent the number of bytes written to the caller.

Unfortunately, no caller of `git_stream_write` ever checks the return
value, except to verify that no error occurred. Due to this, they are
susceptible to the case where only partial data has been written.

Fix this by introducing a new function `git_stream__write_full`. In
contrast to `git_stream_write`, it will always return either success or
failure, without returning the number of bytes written. Thus, it is able
to write all `SIZE_MAX` bytes and loop around `git_stream_write` until
all data has been written. Adjust all callers except the BIO callbacks
in our mbedtls and OpenSSL streams, which already do the right thing and
require the amount of bytes written.

4 files changed, 35 insertions, 25 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/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);