diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java index 3e7dcbdba..e07de7fa7 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java @@ -401,16 +401,25 @@ public class HttpEngine { } } + /** + * Initialize the response content stream from the response transfer stream. + * These two streams are the same unless we're doing transparent gzip, in + * which case the content stream is decompressed. + * + *

Whenever we do transparent gzip we also strip the corresponding headers. + * We strip the Content-Encoding header to prevent the application from + * attempting to double decompress. We strip the Content-Length header because + * it is the length of the compressed content, but the application is only + * interested in the length of the uncompressed content. + * + *

This method should only be used for non-empty response bodies. Response + * codes like "304 Not Modified" can include "Content-Encoding: gzip" without + * a response body and we will crash if we attempt to decompress the zero-byte + * stream. + */ private void initContentStream(InputStream transferStream) throws IOException { responseTransferIn = transferStream; if (transparentGzip && "gzip".equalsIgnoreCase(response.header("Content-Encoding"))) { - // If the response was transparently gzipped, remove the gzip header field - // so clients don't double decompress. http://b/3009828 - // - // Also remove the Content-Length in this case because it contains the - // length of the gzipped response. This isn't terribly useful and is - // dangerous because clients can query the content length, but not the - // content encoding. response = response.newBuilder() .removeHeader("Content-Encoding") .removeHeader("Content-Length") @@ -561,10 +570,14 @@ public class HttpEngine { } } - if (hasResponseBody()) { - maybeCache(); + if (!hasResponseBody()) { + // Don't call initContentStream() when the response doesn't have any content. + responseTransferIn = transport.getTransferStream(cacheRequest); + responseBodyIn = responseTransferIn; + return; } + maybeCache(); initContentStream(transport.getTransferStream(cacheRequest)); } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 5704b6b5c..7b9eb8d09 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -17,6 +17,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.HttpResponseCache; +import com.squareup.okhttp.OkAuthenticator.Credential; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Protocol; import com.squareup.okhttp.internal.RecordingAuthenticator; @@ -75,7 +76,6 @@ import org.junit.Before; import org.junit.Ignore; import org.junit.Test; -import static com.squareup.okhttp.OkAuthenticator.Credential; import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT; import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_END; import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_START; @@ -2572,6 +2572,38 @@ public final class URLConnectionTest { assertEquals(Long.toString(contentLength), request.getHeader("Content-Length")); } + /** + * We had a bug where we attempted to gunzip responses that didn't have a + * body. This only came up with 304s since that response code can include + * headers (like "Content-Encoding") without any content to go along with it. + * https://github.com/square/okhttp/issues/358 + */ + @Test public void noTransparentGzipFor304NotModified() throws Exception { + server.enqueue(new MockResponse() + .clearHeaders() + .setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED) + .addHeader("Content-Encoding: gzip")); + server.enqueue(new MockResponse().setBody("b")); + + server.play(); + + HttpURLConnection connection1 = client.open(server.getUrl("/")); + assertEquals(HttpURLConnection.HTTP_NOT_MODIFIED, connection1.getResponseCode()); + assertContent("", connection1); + connection1.getInputStream().close(); + + HttpURLConnection connection2 = client.open(server.getUrl("/")); + assertEquals(HttpURLConnection.HTTP_OK, connection2.getResponseCode()); + assertContent("b", connection2); + connection2.getInputStream().close(); + + RecordedRequest requestA = server.takeRequest(); + assertEquals(0, requestA.getSequenceNumber()); + + RecordedRequest requestB = server.takeRequest(); + assertEquals(1, requestB.getSequenceNumber()); + } + /** Returns a gzipped copy of {@code bytes}. */ public byte[] gzip(byte[] bytes) throws IOException { ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();