Skip to content

Commit 502e5d5

Browse files
committed
httpclient: use a 16kb read buffer for macOS
Use a 16kb read buffer for compatibility with macOS SecureTransport. SecureTransport `SSLRead` has the following behavior: 1. It will return _at most_ one TLS packet's worth of data, and 2. It will try to give you as much data as you asked for This means that if you call `SSLRead` with a buffer size that is smaller than what _it_ reads (in other words, the maximum size of a TLS packet), then it will buffer that data for subsequent calls. However, it will also attempt to give you as much data as you requested in your SSLRead call. This means that it will guarantee a network read in the event that it has buffered data. Consider our 8kb buffer and a server sending us 12kb of data on an HTTP Keep-Alive session. Our first `SSLRead` will read the TLS packet off the network. It will return us the 8kb that we requested and buffer the remaining 4kb. Our second `SSLRead` call will see the 4kb that's buffered and decide that it could give us an additional 4kb. So it will do a network read. But there's nothing left to read; that was the end of the data. The HTTP server is waiting for us to provide a new request. The server will eventually time out, our `read` system call will return, `SSLRead` can return back to us and we can make progress. While technically correct, this is wildly ineffecient. (Thanks, Tim Apple!) Moving us to use an internal buffer that is the maximum size of a TLS packet (16kb) ensures that `SSLRead` will never buffer and it will always return everything that it read (albeit decrypted).
1 parent cd6ed4e commit 502e5d5

File tree

1 file changed

+12
-1
lines changed

1 file changed

+12
-1
lines changed

src/transports/httpclient.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,18 @@ static git_http_auth_scheme auth_schemes[] = {
2929
{ GIT_HTTP_AUTH_BASIC, "Basic", GIT_CREDENTIAL_USERPASS_PLAINTEXT, git_http_auth_basic },
3030
};
3131

32-
#define GIT_READ_BUFFER_SIZE 8192
32+
/*
33+
* Use a 16kb read buffer to match the maximum size of a TLS packet. This
34+
* is critical for compatibility with SecureTransport, which will always do
35+
* a network read on every call, even if it has data buffered to return to
36+
* you. That buffered data may be the _end_ of a keep-alive response, so
37+
* if SecureTransport performs another network read, it will wait until the
38+
* server ultimately times out before it returns that buffered data to you.
39+
* Since SecureTransport only reads a single TLS packet at a time, by
40+
* calling it with a read buffer that is the maximum size of a TLS packet,
41+
* we ensure that it will never buffer.
42+
*/
43+
#define GIT_READ_BUFFER_SIZE (16 * 1024)
3344

3445
typedef struct {
3546
git_net_url url;

0 commit comments

Comments
 (0)