diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/GzipSource.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/GzipSource.java index 7ece47593..67a09db73 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/GzipSource.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/GzipSource.java @@ -83,8 +83,16 @@ public final class GzipSource implements Source { // trailer before returning a -1 exhausted result; that way if you read to // the end of a GzipSource you guarantee that the CRC has been checked. if (section == SECTION_TRAILER) { - consumeTrailer(deadline); + 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; @@ -149,7 +157,7 @@ public final class GzipSource implements Source { } } - private void consumeTrailer(Deadline deadline) throws IOException { + private void consumeTrailer() throws IOException { // Read the eight-byte trailer. Confirm the body's CRC and size. // +---+---+---+---+---+---+---+---+ // | CRC32 | ISIZE | diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/bytes/GzipSourceTest.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/bytes/GzipSourceTest.java index d4b59b6c0..c38a4b4a7 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/bytes/GzipSourceTest.java +++ b/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); + } + } } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpOverSpdyTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpOverSpdyTest.java index 83cabdece..18d4f7588 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpOverSpdyTest.java +++ b/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();