mirror of
https://github.com/square/okhttp.git
synced 2026-01-27 04:22:07 +03:00
Fix connection recycling for TLS+gzip+chunked.
We had a bug where the underlying SSLInputStream always returned 0 for InputStream.available(). This prevented us from ever reading the "end of stream" chunk, which prevented connection recycling. http://code.google.com/p/android/issues/detail?id=38817 Also fix a nearby bug where we were fast-forwarding the gzipped stream when we should have been fast-forwarding the transfer stream when we were preparing to recycle a connection.
This commit is contained in:
@@ -214,9 +214,14 @@ final class HttpConnection {
|
||||
throw new IOException("Hostname '" + address.uriHost + "' was not verified");
|
||||
}
|
||||
|
||||
// SSL success. Prepare to hand out Transport instances.
|
||||
/*
|
||||
* Buffer the input to mask SSL InputStream's degenerate available()
|
||||
* implementation. That way we can read the end of a chunked response
|
||||
* without blocking and will recycle the connection more reliably.
|
||||
* http://code.google.com/p/android/issues/detail?id=38817
|
||||
*/
|
||||
sslOutputStream = sslSocket.getOutputStream();
|
||||
sslInputStream = sslSocket.getInputStream();
|
||||
sslInputStream = new BufferedInputStream(sslSocket.getInputStream(), 128);
|
||||
|
||||
byte[] selectedProtocol;
|
||||
if (tlsTolerant
|
||||
|
||||
@@ -101,6 +101,7 @@ public class HttpEngine {
|
||||
|
||||
private Transport transport;
|
||||
|
||||
private InputStream responseTransferIn;
|
||||
private InputStream responseBodyIn;
|
||||
|
||||
private final ResponseCache responseCache = ResponseCache.getDefault();
|
||||
@@ -421,7 +422,7 @@ public class HttpEngine {
|
||||
if (!connectionReleased && connection != null) {
|
||||
connectionReleased = true;
|
||||
|
||||
if (!reusable || !transport.makeReusable(requestBodyOut, responseBodyIn)) {
|
||||
if (!reusable || !transport.makeReusable(requestBodyOut, responseTransferIn)) {
|
||||
connection.closeSocketAndStreams();
|
||||
connection = null;
|
||||
} else if (automaticallyReleaseConnectionToPool) {
|
||||
@@ -432,6 +433,7 @@ public class HttpEngine {
|
||||
}
|
||||
|
||||
private void initContentStream(InputStream transferStream) throws IOException {
|
||||
responseTransferIn = transferStream;
|
||||
if (transparentGzip && responseHeaders.isContentEncodingGzip()) {
|
||||
/*
|
||||
* If the response was transparently gzipped, remove the gzip header field
|
||||
|
||||
@@ -501,12 +501,12 @@ final class HttpTransport implements Transport {
|
||||
cacheWrite(buffer, offset, read);
|
||||
|
||||
/*
|
||||
* If we're at the end of a chunk and the next chunk size is readable,
|
||||
* read it! Reading the last chunk causes the underlying connection to
|
||||
* be recycled and we want to do that as early as possible. Otherwise
|
||||
* self-delimiting streams like gzip will never be recycled.
|
||||
* http://code.google.com/p/android/issues/detail?id=7059
|
||||
*/
|
||||
* If we're at the end of a chunk and the next chunk size is readable,
|
||||
* read it! Reading the last chunk causes the underlying connection to
|
||||
* be recycled and we want to do that as early as possible. Otherwise
|
||||
* self-delimiting streams like gzip will never be recycled.
|
||||
* http://code.google.com/p/android/issues/detail?id=7059
|
||||
*/
|
||||
if (bytesRemainingInChunk == 0 && in.available() >= MIN_LAST_CHUNK_LENGTH) {
|
||||
readChunkSize();
|
||||
}
|
||||
|
||||
@@ -1024,11 +1024,19 @@ public final class URLConnectionTest extends TestCase {
|
||||
}
|
||||
|
||||
public void testGzipAndConnectionReuseWithFixedLength() throws Exception {
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.FIXED_LENGTH);
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.FIXED_LENGTH, false);
|
||||
}
|
||||
|
||||
public void testGzipAndConnectionReuseWithChunkedEncoding() throws Exception {
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.CHUNKED);
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.CHUNKED, false);
|
||||
}
|
||||
|
||||
public void testGzipAndConnectionReuseWithFixedLengthAndTls() throws Exception {
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.FIXED_LENGTH, true);
|
||||
}
|
||||
|
||||
public void testGzipAndConnectionReuseWithChunkedEncodingAndTls() throws Exception {
|
||||
testClientConfiguredGzipContentEncodingAndConnectionReuse(TransferKind.CHUNKED, true);
|
||||
}
|
||||
|
||||
public void testClientConfiguredCustomContentEncoding() throws Exception {
|
||||
@@ -1047,11 +1055,20 @@ public final class URLConnectionTest extends TestCase {
|
||||
|
||||
/**
|
||||
* Test a bug where gzip input streams weren't exhausting the input stream,
|
||||
* which corrupted the request that followed.
|
||||
* which corrupted the request that followed or prevented connection reuse.
|
||||
* http://code.google.com/p/android/issues/detail?id=7059
|
||||
* http://code.google.com/p/android/issues/detail?id=38817
|
||||
*/
|
||||
private void testClientConfiguredGzipContentEncodingAndConnectionReuse(
|
||||
TransferKind transferKind) throws Exception {
|
||||
TransferKind transferKind, boolean tls) throws Exception {
|
||||
SSLSocketFactory socketFactory = null;
|
||||
RecordingHostnameVerifier hostnameVerifier = null;
|
||||
if (tls) {
|
||||
socketFactory = sslContext.getSocketFactory();
|
||||
hostnameVerifier = new RecordingHostnameVerifier();
|
||||
server.useHttps(socketFactory, false);
|
||||
}
|
||||
|
||||
MockResponse responseOne = new MockResponse();
|
||||
responseOne.addHeader("Content-Encoding: gzip");
|
||||
transferKind.setBody(responseOne, gzip("one (gzipped)".getBytes("UTF-8")), 5);
|
||||
@@ -1062,12 +1079,20 @@ public final class URLConnectionTest extends TestCase {
|
||||
server.play();
|
||||
|
||||
URLConnection connection = openConnection(server.getUrl("/"));
|
||||
if (tls) {
|
||||
((OkHttpsConnection) connection).setSSLSocketFactory(socketFactory);
|
||||
((OkHttpsConnection) connection).setHostnameVerifier(hostnameVerifier);
|
||||
}
|
||||
connection.addRequestProperty("Accept-Encoding", "gzip");
|
||||
InputStream gunzippedIn = new GZIPInputStream(connection.getInputStream());
|
||||
assertEquals("one (gzipped)", readAscii(gunzippedIn, Integer.MAX_VALUE));
|
||||
assertEquals(0, server.takeRequest().getSequenceNumber());
|
||||
|
||||
connection = openConnection(server.getUrl("/"));
|
||||
if (tls) {
|
||||
((OkHttpsConnection) connection).setSSLSocketFactory(socketFactory);
|
||||
((OkHttpsConnection) connection).setHostnameVerifier(hostnameVerifier);
|
||||
}
|
||||
assertEquals("two (identity)", readAscii(connection.getInputStream(), Integer.MAX_VALUE));
|
||||
assertEquals(1, server.takeRequest().getSequenceNumber());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user