From f2cf086d760dfc3c0ccdc41edb628c97110075d7 Mon Sep 17 00:00:00 2001 From: falhassen Date: Thu, 22 Sep 2016 16:00:43 -0700 Subject: [PATCH] Fix content length calculation for gzipped files ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=134011959 --- .../ext/cronet/CronetDataSourceTest.java | 76 +++++++++++++------ .../ext/cronet/CronetDataSource.java | 40 +++++++--- 2 files changed, 85 insertions(+), 31 deletions(-) diff --git a/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java b/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java index f2364ac257..ccd4dec191 100644 --- a/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java +++ b/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java @@ -109,7 +109,7 @@ public final class CronetDataSourceTest { @Mock private Predicate mockContentTypePredicate; @Mock - private TransferListener mockTransferListener; + private TransferListener mockTransferListener; @Mock private Clock mockClock; @Mock @@ -172,8 +172,8 @@ public final class CronetDataSourceTest { } @Test(expected = IllegalStateException.class) - public void testOpeningTwiceThrows() throws HttpDataSourceException, IllegalStateException { - mockResponesStartSuccess(); + public void testOpeningTwiceThrows() throws HttpDataSourceException { + mockResponseStartSuccess(); assertConnectionState(CronetDataSource.IDLE_CONNECTION); dataSourceUnderTest.open(testDataSpec); @@ -183,7 +183,7 @@ public final class CronetDataSourceTest { @Test public void testCallbackFromPreviousRequest() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); dataSourceUnderTest.open(testDataSpec); dataSourceUnderTest.close(); @@ -217,7 +217,7 @@ public final class CronetDataSourceTest { @Test public void testRequestStartCalled() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); dataSourceUnderTest.open(testDataSpec); verify(mockCronetEngine).createRequest( @@ -234,7 +234,7 @@ public final class CronetDataSourceTest { @Test public void testRequestHeadersSet() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); testResponseHeader.put("Content-Length", Long.toString(5000L)); @@ -252,13 +252,29 @@ public final class CronetDataSourceTest { @Test public void testRequestOpen() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); assertEquals(TEST_CONTENT_LENGTH, dataSourceUnderTest.open(testDataSpec)); assertConnectionState(CronetDataSource.OPEN_CONNECTION); verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec); } + + @Test + public void testRequestOpenGzippedCompressedReturnsDataSpecLength() + throws HttpDataSourceException { + testResponseHeader.put("Content-Encoding", "gzip"); + testUrlResponseInfo = createUrlResponseInfo(200); // statusCode + mockResponseStartSuccess(); + + // Data spec's requested length, 5000. Test response's length, 16,000. + testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); + + assertEquals(5000 /* contentLength */, dataSourceUnderTest.open(testDataSpec)); + assertConnectionState(CronetDataSource.OPEN_CONNECTION); + verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec); + } + @Test public void testRequestOpenFail() { mockResponseStartFailure(); @@ -295,7 +311,7 @@ public final class CronetDataSourceTest { @Test public void testRequestOpenValidatesStatusCode() { - mockResponesStartSuccess(); + mockResponseStartSuccess(); testUrlResponseInfo = createUrlResponseInfo(500); // statusCode try { @@ -312,7 +328,7 @@ public final class CronetDataSourceTest { @Test public void testRequestOpenValidatesContentTypePredicate() { - mockResponesStartSuccess(); + mockResponseStartSuccess(); when(mockContentTypePredicate.evaluate(anyString())).thenReturn(false); try { @@ -329,7 +345,7 @@ public final class CronetDataSourceTest { @Test public void testRequestOpenValidatesContentLength() { - mockResponesStartSuccess(); + mockResponseStartSuccess(); // Data spec's requested length, 5000. Test response's length, 16,000. testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); @@ -348,7 +364,7 @@ public final class CronetDataSourceTest { @Test public void testPostRequestOpen() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE); assertEquals(TEST_CONTENT_LENGTH, dataSourceUnderTest.open(testPostDataSpec)); @@ -358,7 +374,7 @@ public final class CronetDataSourceTest { @Test public void testPostRequestOpenValidatesContentType() { - mockResponesStartSuccess(); + mockResponseStartSuccess(); try { dataSourceUnderTest.open(testPostDataSpec); @@ -370,7 +386,7 @@ public final class CronetDataSourceTest { @Test public void testPostRequestOpenRejects307Redirects() { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockResponseStartRedirect(); try { @@ -384,7 +400,7 @@ public final class CronetDataSourceTest { @Test public void testRequestReadTwice() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadSuccess(); dataSourceUnderTest.open(testDataSpec); @@ -406,7 +422,7 @@ public final class CronetDataSourceTest { @Test public void testSecondRequestNoContentLength() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadSuccess(); byte[] returnedBuffer = new byte[8]; @@ -437,7 +453,23 @@ public final class CronetDataSourceTest { @Test public void testReadWithOffset() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); + mockReadSuccess(); + + dataSourceUnderTest.open(testDataSpec); + + byte[] returnedBuffer = new byte[16]; + int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8); + assertArrayEquals(prefixZeros(buildTestDataArray(0, 8), 16), returnedBuffer); + assertEquals(8, bytesRead); + verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8); + } + + @Test + public void testReadWithUnsetLength() throws HttpDataSourceException { + testResponseHeader.remove("Content-Length"); + testUrlResponseInfo = createUrlResponseInfo(200); // statusCode + mockResponseStartSuccess(); mockReadSuccess(); dataSourceUnderTest.open(testDataSpec); @@ -451,7 +483,7 @@ public final class CronetDataSourceTest { @Test public void testReadReturnsWhatItCan() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadSuccess(); dataSourceUnderTest.open(testDataSpec); @@ -465,7 +497,7 @@ public final class CronetDataSourceTest { @Test public void testClosedMeansClosed() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadSuccess(); int bytesRead = 0; @@ -493,7 +525,7 @@ public final class CronetDataSourceTest { @Test public void testOverread() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadSuccess(); // Ask for 16 bytes @@ -680,7 +712,7 @@ public final class CronetDataSourceTest { @Test public void testExceptionFromTransferListener() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); // Make mockTransferListener throw an exception in CronetDataSource.close(). Ensure that // the subsequent open() call succeeds. @@ -699,7 +731,7 @@ public final class CronetDataSourceTest { @Test public void testReadFailure() throws HttpDataSourceException { - mockResponesStartSuccess(); + mockResponseStartSuccess(); mockReadFailure(); dataSourceUnderTest.open(testDataSpec); @@ -726,7 +758,7 @@ public final class CronetDataSourceTest { }).when(mockUrlRequest).getStatus(any(UrlRequest.StatusListener.class)); } - private void mockResponesStartSuccess() { + private void mockResponseStartSuccess() { doAnswer(new Answer() { @Override public Object answer(InvocationOnMock invocation) throws Throwable { diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index 401941addc..fe8fd13abe 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -300,15 +300,20 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou try { validateResponse(info); responseInfo = info; - // Check content length. - contentLength = getContentLength(info.getAllHeaders()); - // If a specific length is requested and a specific length is returned but the 2 don't match - // it's an error. - if (currentDataSpec.length != C.LENGTH_UNSET - && contentLength != C.LENGTH_UNSET - && currentDataSpec.length != contentLength) { - throw new OpenException("Content length did not match requested length", currentDataSpec, - getCurrentRequestStatus()); + + if (isCompressed(info)) { + contentLength = currentDataSpec.length; + } else { + // Check content length. + contentLength = getContentLength(info.getAllHeaders()); + // If a specific length is requested and a specific length is returned but the 2 don't match + // it's an error. + if (currentDataSpec.length != C.LENGTH_UNSET + && contentLength != C.LENGTH_UNSET + && currentDataSpec.length != contentLength) { + throw new OpenException("Content length did not match requested length", currentDataSpec, + getCurrentRequestStatus()); + } } if (contentLength > 0) { @@ -326,6 +331,23 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } } + /** + * Returns {@code true} iff the content is compressed. + * + *

If {@code true}, clients cannot use the value of content length from the request headers to + * read the data, since Cronet returns the uncompressed data and this content length reflects the + * compressed content length. + */ + private boolean isCompressed(UrlResponseInfo info) { + for (Map.Entry entry : info.getAllHeadersAsList()) { + if (entry.getKey().equalsIgnoreCase("Content-Encoding")) { + return !entry.getValue().equalsIgnoreCase("identity"); + } + } + + return false; + } + private void validateResponse(UrlResponseInfo info) throws HttpDataSourceException { // Check for a valid response code. int responseCode = info.getHttpStatusCode();