Commit 6d0a0acafaf45a034502fc0a8cbdd39851b238c1
2015-06-11T23:20:28
Fixed some Secure Transport issues on OS X The read and write callbacks passed to SSLSetIOFuncs() have been rewritten to match the implementation used on opensource.apple.com and other open source projects like VLC. This change also fixes a bug where the read callback could get into an infinite loop when 0 bytes were read.
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
diff --git a/src/stransport_stream.c b/src/stransport_stream.c
index 429aa2d..d316c37 100644
--- a/src/stransport_stream.c
+++ b/src/stransport_stream.c
@@ -110,19 +110,26 @@ int stransport_certificate(git_cert **out, git_stream *stream)
return 0;
}
+/*
+ * Contrary to typical network IO callbacks, Secure Transport write callback is
+ * expected to write *all* passed data, not just as much as it can, and any
+ * other case would be considered a failure.
+ *
+ * This behavior is actually not specified in the Apple documentation, but is
+ * required for things to work correctly (and incidentally, that's also how
+ * Apple implements it in its projects at opensource.apple.com).
+ *
+ * Libgit2 streams happen to already have this very behavior so this is just
+ * passthrough.
+ */
static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
{
git_stream *io = (git_stream *) conn;
- ssize_t ret;
- ret = git_stream_write(io, data, *len, 0);
- if (ret < 0) {
- *len = 0;
- return -1;
+ if (git_stream_write(io, data, *len, 0) < 0) {
+ return -36; /* "ioErr" from MacErrors.h which is not available on iOS */
}
- *len = ret;
-
return noErr;
}
@@ -141,29 +148,38 @@ ssize_t stransport_write(git_stream *stream, const char *data, size_t len, int f
return processed;
}
+/*
+ * Contrary to typical network IO callbacks, Secure Transport read callback is
+ * expected to read *exactly* the requested number of bytes, not just as much
+ * as it can, and any other case would be considered a failure.
+ *
+ * This behavior is actually not specified in the Apple documentation, but is
+ * required for things to work correctly (and incidentally, that's also how
+ * Apple implements it in its projects at opensource.apple.com).
+ */
static OSStatus read_cb(SSLConnectionRef conn, void *data, size_t *len)
{
git_stream *io = (git_stream *) conn;
+ OSStatus error = noErr;
+ size_t off = 0;
ssize_t ret;
- size_t left, requested;
- requested = left = *len;
do {
- ret = git_stream_read(io, data + (requested - left), left);
+ ret = git_stream_read(io, data + off, *len - off);
if (ret < 0) {
- *len = 0;
- return -1;
+ error = -36; /* "ioErr" from MacErrors.h which is not available on iOS */
+ break;
+ }
+ if (ret == 0) {
+ error = errSSLClosedGraceful;
+ break;
}
- left -= ret;
- } while (left);
-
- *len = requested;
-
- if (ret == 0)
- return errSSLClosedGraceful;
+ off += ret;
+ } while (off < *len);
- return noErr;
+ *len = off;
+ return error;
}
ssize_t stransport_read(git_stream *stream, void *data, size_t len)