Merge pull request #474 from square/jwilson_0121_gzip

Don't unzip if there isn't a response body.

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.
+   *
+   * <p>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.
+   *
+   * <p>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));
   }
 

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();