Merge pull request #548 from square/jwilson_0222_self_terminating_gzips

Make GzipSource exhaust its input.

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/GzipSource.java

@@ -85,6 +85,14 @@ public final class GzipSource implements Source {
     if (section == SECTION_TRAILER) {
       consumeTrailer();
       section = SECTION_DONE;
+
+      // Gzip streams self-terminate: they return -1 before their underlying
+      // source returns -1. Here we attempt to force the underlying stream to
+      // return -1 which may trigger it to release its resources. If it doesn't
+      // return -1, then our Gzip data finished prematurely!
+      if (!source.exhausted(deadline)) {
+        throw new IOException("gzip finished without exhausting source");
+      }
     }
 
     return -1;

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/bytes/GzipSourceTest.java

@@ -22,6 +22,8 @@ import java.util.zip.CRC32;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class GzipSourceTest {
@@ -149,6 +151,50 @@ public class GzipSourceTest {
     }
   }
 
+  @Test public void gunzipExhaustsSource() throws Exception {
+    byte[] abcGzipped = {
+        (byte) 0x1f, (byte) 0x8b, (byte) 0x08, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
+        (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x4b, (byte) 0x4c, (byte) 0x4a, (byte) 0x06,
+        (byte) 0x00, (byte) 0xc2, (byte) 0x41, (byte) 0x24, (byte) 0x35, (byte) 0x03, (byte) 0x00,
+        (byte) 0x00, (byte) 0x00
+    };
+    OkBuffer gzippedSource = new OkBuffer();
+    gzippedSource.write(abcGzipped, 0, abcGzipped.length);
+
+    ExhaustableSource exhaustableSource = new ExhaustableSource(gzippedSource);
+    BufferedSource gunzippedSource = new BufferedSource(new GzipSource(exhaustableSource));
+
+    assertEquals('a', gunzippedSource.readByte());
+    assertEquals('b', gunzippedSource.readByte());
+    assertEquals('c', gunzippedSource.readByte());
+    assertFalse(exhaustableSource.exhausted);
+    assertEquals(-1, gunzippedSource.read(new OkBuffer(), 1, Deadline.NONE));
+    assertTrue(exhaustableSource.exhausted);
+  }
+
+  @Test public void gunzipThrowsIfSourceIsNotExhausted() throws Exception {
+    byte[] abcGzipped = {
+        (byte) 0x1f, (byte) 0x8b, (byte) 0x08, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
+        (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x4b, (byte) 0x4c, (byte) 0x4a, (byte) 0x06,
+        (byte) 0x00, (byte) 0xc2, (byte) 0x41, (byte) 0x24, (byte) 0x35, (byte) 0x03, (byte) 0x00,
+        (byte) 0x00, (byte) 0x00
+    };
+    OkBuffer gzippedSource = new OkBuffer();
+    gzippedSource.write(abcGzipped, 0, abcGzipped.length);
+    gzippedSource.writeByte('d'); // This byte shouldn't be here!
+
+    BufferedSource gunzippedSource = new BufferedSource(new GzipSource(gzippedSource));
+
+    assertEquals('a', gunzippedSource.readByte());
+    assertEquals('b', gunzippedSource.readByte());
+    assertEquals('c', gunzippedSource.readByte());
+    try {
+      gunzippedSource.readByte();
+      fail();
+    } catch (IOException expected) {
+    }
+  }
+
   private byte[] gzipHeaderWithFlags(byte flags) {
     byte[] result = Arrays.copyOf(gzipHeader, gzipHeader.length);
     result[3] = flags;
@@ -180,4 +226,25 @@ public class GzipSourceTest {
     }
     return result;
   }
+
+  /** This source keeps track of whether its read have returned -1. */
+  static class ExhaustableSource implements Source {
+    private final Source source;
+    private boolean exhausted;
+
+    ExhaustableSource(Source source) {
+      this.source = source;
+    }
+
+    @Override public long read(OkBuffer sink, long byteCount, Deadline deadline)
+        throws IOException {
+      long result = source.read(sink, byteCount, deadline);
+      if (result == -1) exhausted = true;
+      return result;
+    }
+
+    @Override public void close(Deadline deadline) throws IOException {
+      source.close(deadline);
+    }
+  }
 }

okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpOverSpdyTest.java

@@ -124,7 +124,7 @@ public abstract class HttpOverSpdyTest {
   byte[] postBytes = "FGHIJ".getBytes(Util.UTF_8);
 
   /** An output stream can be written to more than once, so we can't guess content length. */
-   @Test public void noDefaultContentLengthOnPost() throws Exception {
+  @Test public void noDefaultContentLengthOnPost() throws Exception {
     MockResponse response = new MockResponse().setBody("ABCDE");
     server.enqueue(response);
     server.play();