Commit 5265b31cc5adb3ff27dddb8cbbc1f4b1c687f73e
2019-01-23T15:00:20
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171
diff --git a/src/stream.h b/src/stream.h
index 00220d5..f16b026 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 a999bb5..a79d3cb 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 8d5a9d9..9fd3b47 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 2168072..80ba5ba 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);