Add option for handling "Set-Cookie" requests in CronetDataSource.

This time, we avoid using the problematic CookieManager and HttpCookie framework APIs by just forwarding the cookie request only when the client has enabled the feature and the server responds with a "Set-Cookie" response header.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173532023
This commit is contained in:
falhassen 2017-10-26 06:54:39 -07:00 committed by Oliver Woodman
parent cf38900d20
commit 46bf254033
2 changed files with 183 additions and 23 deletions

View File

@ -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<Object>() {
@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<Object>() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
dataSourceUnderTest.onResponseStarted(
mockUrlRequest,
testUrlResponseInfo);
return null;
}
}).when(mockUrlRequest).followRedirect();
}
private void mockResponseStartFailure() {
doAnswer(new Answer<Object>() {
@Override
@ -850,16 +955,16 @@ public final class CronetDataSourceTest {
private void mockReadFailure() {
doAnswer(
new Answer<Object>() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
dataSourceUnderTest.onFailed(
mockUrlRequest,
createUrlResponseInfo(500), // statusCode
mockNetworkException);
return null;
}
})
new Answer<Object>() {
@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<Object>() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
startedCondition.open();
return null;
}
})
new Answer<Object>() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
startedCondition.open();
return null;
}
})
.when(mockUrlRequest)
.read(any(ByteBuffer.class));
return startedCondition;

View File

@ -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<String> contentTypePredicate, TransferListener<? super CronetDataSource> 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<String> contentTypePredicate, TransferListener<? super CronetDataSource> 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<String> contentTypePredicate, TransferListener<? super CronetDataSource> 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<String, List<String>> 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<String> 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];