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 ea12edaf53..62b5fb261a 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 @@ -103,6 +103,7 @@ public final class CronetDataSourceTest { @Mock private CronetEngine mockCronetEngine; private CronetDataSource dataSourceUnderTest; + private boolean redirectCalled; @Before public void setUp() throws Exception { @@ -119,10 +120,11 @@ public final class CronetDataSourceTest { TEST_READ_TIMEOUT_MS, true, // resetTimeoutOnRedirects mockClock, - null)); + null, + false)); when(mockContentTypePredicate.evaluate(anyString())).thenReturn(true); when(mockCronetEngine.newUrlRequestBuilder( - anyString(), any(UrlRequest.Callback.class), any(Executor.class))) + anyString(), any(UrlRequest.Callback.class), any(Executor.class))) .thenReturn(mockUrlRequestBuilder); when(mockUrlRequestBuilder.allowDirectExecutor()).thenReturn(mockUrlRequestBuilder); when(mockUrlRequestBuilder.build()).thenReturn(mockUrlRequest); @@ -698,6 +700,77 @@ public final class CronetDataSourceTest { assertEquals(1, openExceptions.get()); } + @Test + public void testRedirectParseAndAttachCookie_dataSourceDoesNotHandleSetCookie_followsRedirect() + throws HttpDataSourceException { + mockSingleRedirectSuccess(); + mockFollowRedirectSuccess(); + + testResponseHeader.put("Set-Cookie", "testcookie=testcookie; Path=/video"); + + dataSourceUnderTest.open(testDataSpec); + verify(mockUrlRequestBuilder, never()).addHeader(eq("Cookie"), any(String.class)); + verify(mockUrlRequest).followRedirect(); + } + + @Test + public void testRedirectParseAndAttachCookie_dataSourceHandlesSetCookie() + throws HttpDataSourceException { + dataSourceUnderTest = spy( + new CronetDataSource( + mockCronetEngine, + mockExecutor, + mockContentTypePredicate, + mockTransferListener, + TEST_CONNECT_TIMEOUT_MS, + TEST_READ_TIMEOUT_MS, + true, // resetTimeoutOnRedirects + mockClock, + null, + true)); + mockSingleRedirectSuccess(); + + testResponseHeader.put("Set-Cookie", "testcookie=testcookie; Path=/video"); + + dataSourceUnderTest.open(testDataSpec); + verify(mockUrlRequestBuilder).addHeader(eq("Cookie"), any(String.class)); + verify(mockUrlRequest, never()).followRedirect(); + verify(mockUrlRequest, times(2)).start(); + } + + @Test + public void testRedirectNoSetCookieFollowsRedirect() throws HttpDataSourceException { + mockSingleRedirectSuccess(); + mockFollowRedirectSuccess(); + + dataSourceUnderTest.open(testDataSpec); + verify(mockUrlRequestBuilder, never()).addHeader(eq("Cookie"), any(String.class)); + verify(mockUrlRequest).followRedirect(); + } + + @Test + public void testRedirectNoSetCookieFollowsRedirect_dataSourceHandlesSetCookie() + throws HttpDataSourceException { + dataSourceUnderTest = spy( + new CronetDataSource( + mockCronetEngine, + mockExecutor, + mockContentTypePredicate, + mockTransferListener, + TEST_CONNECT_TIMEOUT_MS, + TEST_READ_TIMEOUT_MS, + true, // resetTimeoutOnRedirects + mockClock, + null, + true)); + mockSingleRedirectSuccess(); + mockFollowRedirectSuccess(); + + dataSourceUnderTest.open(testDataSpec); + verify(mockUrlRequestBuilder, never()).addHeader(eq("Cookie"), any(String.class)); + verify(mockUrlRequest).followRedirect(); + } + @Test public void testExceptionFromTransferListener() throws HttpDataSourceException { mockResponseStartSuccess(); @@ -812,6 +885,38 @@ public final class CronetDataSourceTest { }).when(mockUrlRequest).start(); } + private void mockSingleRedirectSuccess() { + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + if (!redirectCalled) { + redirectCalled = true; + dataSourceUnderTest.onRedirectReceived( + mockUrlRequest, + createUrlResponseInfoWithUrl("http://example.com/video", 300), + "http://example.com/video/redirect"); + } else { + dataSourceUnderTest.onResponseStarted( + mockUrlRequest, + testUrlResponseInfo); + } + return null; + } + }).when(mockUrlRequest).start(); + } + + private void mockFollowRedirectSuccess() { + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + dataSourceUnderTest.onResponseStarted( + mockUrlRequest, + testUrlResponseInfo); + return null; + } + }).when(mockUrlRequest).followRedirect(); + } + private void mockResponseStartFailure() { doAnswer(new Answer() { @Override @@ -850,16 +955,16 @@ public final class CronetDataSourceTest { private void mockReadFailure() { doAnswer( - new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - dataSourceUnderTest.onFailed( - mockUrlRequest, - createUrlResponseInfo(500), // statusCode - mockNetworkException); - return null; - } - }) + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + dataSourceUnderTest.onFailed( + mockUrlRequest, + createUrlResponseInfo(500), // statusCode + mockNetworkException); + return null; + } + }) .when(mockUrlRequest) .read(any(ByteBuffer.class)); } @@ -867,13 +972,13 @@ public final class CronetDataSourceTest { private ConditionVariable buildReadStartedCondition() { final ConditionVariable startedCondition = new ConditionVariable(); doAnswer( - new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - startedCondition.open(); - return null; - } - }) + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + startedCondition.open(); + return null; + } + }) .when(mockUrlRequest) .read(any(ByteBuffer.class)); return startedCondition; 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 5f93bc5e23..546aba159d 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 @@ -97,6 +97,9 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou private static final String TAG = "CronetDataSource"; private static final String CONTENT_TYPE = "Content-Type"; + private static final String SET_COOKIE = "Set-Cookie"; + private static final String COOKIE = "Cookie"; + private static final Pattern CONTENT_RANGE_HEADER_PATTERN = Pattern.compile("^bytes (\\d+)-(\\d+)/(\\d+)$"); // The size of read buffer passed to cronet UrlRequest.read(). @@ -109,6 +112,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou private final int connectTimeoutMs; private final int readTimeoutMs; private final boolean resetTimeoutOnRedirects; + private final boolean handleSetCookieRequests; private final RequestProperties defaultRequestProperties; private final RequestProperties requestProperties; private final ConditionVariable operation; @@ -152,7 +156,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou public CronetDataSource(CronetEngine cronetEngine, Executor executor, Predicate contentTypePredicate, TransferListener listener) { this(cronetEngine, executor, contentTypePredicate, listener, DEFAULT_CONNECT_TIMEOUT_MILLIS, - DEFAULT_READ_TIMEOUT_MILLIS, false, null); + DEFAULT_READ_TIMEOUT_MILLIS, false, null, false); } /** @@ -176,13 +180,40 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, RequestProperties defaultRequestProperties) { this(cronetEngine, executor, contentTypePredicate, listener, connectTimeoutMs, - readTimeoutMs, resetTimeoutOnRedirects, Clock.DEFAULT, defaultRequestProperties); + readTimeoutMs, resetTimeoutOnRedirects, Clock.DEFAULT, defaultRequestProperties, false); + } + + /** + * @param cronetEngine A CronetEngine. + * @param executor The {@link java.util.concurrent.Executor} that will handle responses. + * This may be a direct executor (i.e. executes tasks on the calling thread) in order + * to avoid a thread hop from Cronet's internal network thread to the response handling + * thread. However, to avoid slowing down overall network performance, care must be taken + * to make sure response handling is a fast operation when using a direct executor. + * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the + * predicate then an {@link InvalidContentTypeException} is thrown from + * {@link #open(DataSpec)}. + * @param listener An optional listener. + * @param connectTimeoutMs The connection timeout, in milliseconds. + * @param readTimeoutMs The read timeout, in milliseconds. + * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. + * @param defaultRequestProperties The default request properties to be used. + * @param handleSetCookieRequests Whether "Set-Cookie" requests on redirect should be forwarded to + * the redirect url in the "Cookie" header. + */ + public CronetDataSource(CronetEngine cronetEngine, Executor executor, + Predicate contentTypePredicate, TransferListener listener, + int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, + RequestProperties defaultRequestProperties, boolean handleSetCookieRequests) { + this(cronetEngine, executor, contentTypePredicate, listener, connectTimeoutMs, + readTimeoutMs, resetTimeoutOnRedirects, Clock.DEFAULT, defaultRequestProperties, + handleSetCookieRequests); } /* package */ CronetDataSource(CronetEngine cronetEngine, Executor executor, Predicate contentTypePredicate, TransferListener listener, int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, Clock clock, - RequestProperties defaultRequestProperties) { + RequestProperties defaultRequestProperties, boolean handleSetCookieRequests) { this.cronetEngine = Assertions.checkNotNull(cronetEngine); this.executor = Assertions.checkNotNull(executor); this.contentTypePredicate = contentTypePredicate; @@ -192,6 +223,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou this.resetTimeoutOnRedirects = resetTimeoutOnRedirects; this.clock = Assertions.checkNotNull(clock); this.defaultRequestProperties = defaultRequestProperties; + this.handleSetCookieRequests = handleSetCookieRequests; requestProperties = new RequestProperties(); operation = new ConditionVariable(); } @@ -402,7 +434,19 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (resetTimeoutOnRedirects) { resetConnectTimeout(); } - request.followRedirect(); + + Map> headers = info.getAllHeaders(); + if (!handleSetCookieRequests || isEmpty(headers.get(SET_COOKIE))) { + request.followRedirect(); + } else { + UrlRequest.Builder requestBuilder = + cronetEngine.newUrlRequestBuilder(newLocationUrl, /* callback= */ this, executor); + currentUrlRequest.cancel(); + String cookieHeadersValue = parseCookies(headers.get(SET_COOKIE)); + attachCookies(requestBuilder, cookieHeadersValue); + currentUrlRequest = requestBuilder.build(); + currentUrlRequest.start(); + } } @Override @@ -567,6 +611,17 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return contentLength; } + private static String parseCookies(List setCookieHeaders) { + return TextUtils.join(";", setCookieHeaders); + } + + private static void attachCookies(UrlRequest.Builder requestBuilder, String cookies) { + if (TextUtils.isEmpty(cookies)) { + return; + } + requestBuilder.addHeader(COOKIE, cookies); + } + private static int getStatus(UrlRequest request) throws InterruptedException { final ConditionVariable conditionVariable = new ConditionVariable(); final int[] statusHolder = new int[1];