Skip to content

Commit 25dc535

Browse files
Googlertonihei
Googler
authored and
tonihei
committed
Add CookieHandler support to HttpEngineDataSource.
The original behaviour of the `handleSetCookieRequests` is preserved. A bug is addressed, where it woulld set `Set-Cookie` header in the client request (as opposed to the expected `Cookies`). PiperOrigin-RevId: 732945338 (cherry picked from commit 814d368)
1 parent 53a3144 commit 25dc535

File tree

2 files changed

+156
-33
lines changed

2 files changed

+156
-33
lines changed

‎libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java

Lines changed: 90 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,21 @@
3636
import androidx.media3.common.util.Assertions;
3737
import androidx.media3.common.util.Clock;
3838
import androidx.media3.common.util.ConditionVariable;
39+
import androidx.media3.common.util.Log;
3940
import androidx.media3.common.util.UnstableApi;
4041
import androidx.media3.common.util.Util;
4142
import com.google.common.base.Ascii;
4243
import com.google.common.base.Predicate;
44+
import com.google.common.collect.ImmutableMap;
4345
import com.google.common.net.HttpHeaders;
4446
import com.google.common.primitives.Longs;
4547
import com.google.errorprone.annotations.CanIgnoreReturnValue;
4648
import java.io.IOException;
4749
import java.io.InterruptedIOException;
50+
import java.net.CookieHandler;
51+
import java.net.CookieManager;
4852
import java.net.SocketTimeoutException;
53+
import java.net.URI;
4954
import java.net.UnknownHostException;
5055
import java.nio.ByteBuffer;
5156
import java.util.Arrays;
@@ -321,6 +326,8 @@ public OpenException(
321326
// The size of read buffer passed to cronet UrlRequest.read().
322327
private static final int READ_BUFFER_SIZE_BYTES = 32 * 1024;
323328

329+
private static final String TAG = "HttpEngineDataSource";
330+
324331
private final HttpEngine httpEngine;
325332
private final Executor executor;
326333
private final int requestPriority;
@@ -709,7 +716,7 @@ public synchronized void close() {
709716
@UnstableApi
710717
@VisibleForTesting
711718
@Nullable
712-
UrlRequest.Callback getCurrentUrlRequestCallback() {
719+
UrlRequestCallback getCurrentUrlRequestCallback() {
713720
return currentUrlRequestWrapper == null
714721
? null
715722
: currentUrlRequestWrapper.getUrlRequestCallback();
@@ -932,14 +939,6 @@ private static boolean isCompressed(UrlResponseInfo info) {
932939
return false;
933940
}
934941

935-
@Nullable
936-
private static String parseCookies(@Nullable List<String> setCookieHeaders) {
937-
if (setCookieHeaders == null || setCookieHeaders.isEmpty()) {
938-
return null;
939-
}
940-
return TextUtils.join(";", setCookieHeaders);
941-
}
942-
943942
@Nullable
944943
private static String getFirstHeader(Map<String, List<String>> allHeaders, String headerName) {
945944
@Nullable List<String> headers = allHeaders.get(headerName);
@@ -957,6 +956,55 @@ private static int copyByteBuffer(ByteBuffer src, ByteBuffer dst) {
957956
return remaining;
958957
}
959958

959+
/**
960+
* Stores the cookie headers from the response in the default {@link CookieHandler}.
961+
*
962+
*

This is a no-op if the {@link CookieHandler} is not set .

963+
*/
964+
@VisibleForTesting
965+
/* private */ static void storeCookiesFromHeaders(UrlResponseInfo info) {
966+
CookieHandler cookieHandler = CookieHandler.getDefault();
967+
if (cookieHandler != null) {
968+
try {
969+
cookieHandler.put(new URI(info.getUrl()), info.getHeaders().getAsMap());
970+
} catch (Exception e) {
971+
Log.w(TAG, "Failed to store cookies in CookieHandler", e);
972+
}
973+
}
974+
}
975+
976+
@VisibleForTesting
977+
/* private */ static String getCookieHeader(String url) {
978+
return getCookieHeader(url, ImmutableMap.of());
979+
}
980+
981+
// getCookieHeader maps Set-Cookie2 (RFC 2965) to Cookie just like CookieManager does.
982+
@VisibleForTesting
983+
/* private */ static String getCookieHeader(String url, Map<String, List<String>> headers) {
984+
CookieHandler cookieHandler = CookieHandler.getDefault();
985+
if (cookieHandler == null) {
986+
return "";
987+
}
988+
989+
Map<String, List<String>> cookieHeaders = ImmutableMap.of();
990+
try {
991+
cookieHeaders = cookieHandler.get(new URI(url), headers);
992+
} catch (Exception e) {
993+
Log.w(TAG, "Failed to read cookies from CookieHandler", e);
994+
}
995+
996+
StringBuilder cookies = new StringBuilder();
997+
if (cookieHeaders.containsKey(HttpHeaders.COOKIE)) {
998+
List<String> cookiesList = cookieHeaders.get(HttpHeaders.COOKIE);
999+
if (cookiesList != null) {
1000+
for (String cookie : cookiesList) {
1001+
cookies.append(cookie).append("; ");
1002+
}
1003+
}
1004+
}
1005+
return cookies.toString().stripTrailing();
1006+
}
1007+
9601008
/**
9611009
* A wrapper class that manages a {@link UrlRequest} and the {@link UrlRequestCallback} associated
9621010
* with that request.
@@ -984,7 +1032,7 @@ public void close() {
9841032
urlRequest.cancel();
9851033
}
9861034

987-
public UrlRequest.Callback getUrlRequestCallback() {
1035+
public UrlRequestCallback getUrlRequestCallback() {
9881036
return urlRequestCallback;
9891037
}
9901038

@@ -1004,14 +1052,18 @@ public void onStatus(int status) {
10041052
}
10051053
}
10061054

1007-
private final class UrlRequestCallback implements UrlRequest.Callback {
1008-
1055+
final class UrlRequestCallback implements UrlRequest.Callback {
10091056
private volatile boolean isClosed = false;
10101057

10111058
public void close() {
10121059
this.isClosed = true;
10131060
}
10141061

1062+
@SuppressWarnings("argument.type.incompatible")
1063+
private void resetDefaultCookieHandler() {
1064+
CookieHandler.setDefault(null);
1065+
}
1066+
10151067
@Override
10161068
public synchronized void onRedirectReceived(
10171069
UrlRequest request, UrlResponseInfo info, String newLocationUrl) {
@@ -1040,24 +1092,34 @@ public synchronized void onRedirectReceived(
10401092
resetConnectTimeout();
10411093
}
10421094

1095+
boolean hasDefaultCookieHandler = CookieHandler.getDefault() != null;
1096+
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
1097+
// a temporary CookieManager is created for the duration of this request - this guarantees
1098+
// redirects preserve the cookies correctly.
1099+
CookieManager cm = new CookieManager();
1100+
CookieHandler.setDefault(cm);
1101+
}
1102+
1103+
storeCookiesFromHeaders(info);
1104+
String cookieHeaders = getCookieHeader(info.getUrl(), info.getHeaders().getAsMap());
1105+
1106+
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
1107+
resetDefaultCookieHandler();
1108+
}
1109+
10431110
boolean shouldKeepPost =
10441111
keepPostFor302Redirects
10451112
&& dataSpec.httpMethod == DataSpec.HTTP_METHOD_POST
10461113
&& responseCode == 302;
10471114

10481115
// request.followRedirect() transforms a POST request into a GET request, so if we want to
10491116
// keep it as a POST we need to fall through to the manual redirect logic below.
1050-
if (!shouldKeepPost && !handleSetCookieRequests) {
1051-
request.followRedirect();
1052-
return;
1053-
}
1054-
1055-
@Nullable
1056-
String cookieHeadersValue =
1057-
parseCookies(info.getHeaders().getAsMap().get(HttpHeaders.SET_COOKIE));
1058-
if (!shouldKeepPost && TextUtils.isEmpty(cookieHeadersValue)) {
1059-
request.followRedirect();
1060-
return;
1117+
if (!shouldKeepPost) {
1118+
// No cookies, or we're not handling them - so just follow the redirect.
1119+
if (!handleSetCookieRequests || TextUtils.isEmpty(cookieHeaders)) {
1120+
request.followRedirect();
1121+
return;
1122+
}
10611123
}
10621124

10631125
request.cancel();
@@ -1075,13 +1137,15 @@ public synchronized void onRedirectReceived(
10751137
} else {
10761138
redirectUrlDataSpec = dataSpec.withUri(Uri.parse(newLocationUrl));
10771139
}
1078-
if (!TextUtils.isEmpty(cookieHeadersValue)) {
1140+
1141+
if (!TextUtils.isEmpty(cookieHeaders)) {
10791142
Map<String, String> requestHeaders = new HashMap<>();
10801143
requestHeaders.putAll(dataSpec.httpRequestHeaders);
1081-
requestHeaders.put(HttpHeaders.COOKIE, cookieHeadersValue);
1144+
requestHeaders.put(HttpHeaders.COOKIE, cookieHeaders);
10821145
redirectUrlDataSpec =
10831146
redirectUrlDataSpec.buildUpon().setHttpRequestHeaders(requestHeaders).build();
10841147
}
1148+
10851149
UrlRequestWrapper redirectUrlRequestWrapper;
10861150
try {
10871151
redirectUrlRequestWrapper = buildRequestWrapper(redirectUrlDataSpec);
@@ -1101,6 +1165,7 @@ public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo i
11011165
if (isClosed) {
11021166
return;
11031167
}
1168+
storeCookiesFromHeaders(info);
11041169
responseInfo = info;
11051170
operation.open();
11061171
}

‎libraries/datasource/src/test/java/androidx/media3/datasource/HttpEngineDataSourceTest.java

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,14 @@
4545
import androidx.media3.datasource.HttpDataSource.HttpDataSourceException;
4646
import androidx.media3.datasource.HttpDataSource.InvalidResponseCodeException;
4747
import androidx.test.ext.junit.runners.AndroidJUnit4;
48+
import com.google.common.collect.ImmutableList;
49+
import com.google.common.collect.ImmutableMap;
4850
import java.io.IOException;
4951
import java.io.InterruptedIOException;
52+
import java.net.CookieHandler;
53+
import java.net.CookieManager;
5054
import java.net.SocketTimeoutException;
55+
import java.net.URI;
5156
import java.net.UnknownHostException;
5257
import java.nio.ByteBuffer;
5358
import java.util.ArrayList;
@@ -80,8 +85,14 @@ public final class HttpEngineDataSourceTest {
8085

8186
private static final int TEST_CONNECT_TIMEOUT_MS = 100;
8287
private static final int TEST_READ_TIMEOUT_MS = 100;
83-
private static final String TEST_URL = "http://google.com";
88+
private static final String TEST_URL = "http://google.com/video/";
8489
private static final String TEST_CONTENT_TYPE = "test/test";
90+
private static final String TEST_REQUEST_COOKIE = "foo=bar";
91+
private static final String TEST_REQUEST_COOKIE_2 = "baz=qux";
92+
private static final String TEST_RESPONSE_SET_COOKIE =
93+
TEST_REQUEST_COOKIE + ";path=/video; expires 31-12-2099 23:59:59 GMT";
94+
private static final String TEST_RESPONSE_SET_COOKIE_2 =
95+
TEST_REQUEST_COOKIE_2 + ";path=/; expires 31-12-2099 23:59:59 GMT";
8596
private static final byte[] TEST_POST_BODY = Util.getUtf8Bytes("test post body");
8697
private static final long TEST_CONTENT_LENGTH = 16000L;
8798

@@ -141,6 +152,8 @@ public void setUp() {
141152
// This value can be anything since the DataSpec is unset.
142153
testResponseHeader.put("Content-Length", Long.toString(TEST_CONTENT_LENGTH));
143154
testUrlResponseInfo = createUrlResponseInfo(/* statusCode= */ 200);
155+
156+
CookieHandler.setDefault(null);
144157
}
145158

146159
@After
@@ -272,15 +285,15 @@ public void requestSetsRangeHeader() throws HttpDataSourceException {
272285
@Test
273286
public void requestHeadersSet() throws HttpDataSourceException {
274287
Map<String, String> headersSet = new HashMap<>();
275-
doAnswer(
288+
when(mockUrlRequestBuilder.addHeader(
289+
ArgumentMatchers.anyString(), ArgumentMatchers.anyString()))
290+
.thenAnswer(
276291
(invocation) -> {
277292
String key = invocation.getArgument(0);
278293
String value = invocation.getArgument(1);
279294
headersSet.put(key, value);
280295
return null;
281-
})
282-
.when(mockUrlRequestBuilder)
283-
.addHeader(ArgumentMatchers.anyString(), ArgumentMatchers.anyString());
296+
});
284297

285298
dataSourceUnderTest.setRequestProperty("defaultHeader2", "dataSourceOverridesDefault");
286299
dataSourceUnderTest.setRequestProperty("dataSourceHeader1", "dataSourceValue1");
@@ -447,8 +460,7 @@ public void requestOpenValidatesContentTypePredicate() {
447460
assertThat(e).isInstanceOf(HttpDataSource.InvalidContentTypeException.class);
448461
// Check for connection not automatically closed.
449462
verify(mockUrlRequest, never()).cancel();
450-
assertThat(testedContentTypes).hasSize(1);
451-
assertThat(testedContentTypes.get(0)).isEqualTo(TEST_CONTENT_TYPE);
463+
assertThat(testedContentTypes).containsExactly(TEST_CONTENT_TYPE);
452464
}
453465
}
454466

@@ -1277,7 +1289,7 @@ public void redirect302ChangesPostToGet() throws HttpDataSourceException {
12771289
.createDataSource();
12781290
mockSingleRedirectSuccess(/* responseCode= */ 302);
12791291
dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE);
1280-
testResponseHeader.put("Set-Cookie", "testcookie=testcookie; Path=/video");
1292+
testResponseHeader.put("Set-Cookie", TEST_RESPONSE_SET_COOKIE);
12811293

12821294
dataSourceUnderTest.open(testPostDataSpec);
12831295

@@ -1449,6 +1461,52 @@ public void allowDirectExecutor() throws HttpDataSourceException {
14491461
verify(mockUrlRequestBuilder).setDirectExecutorAllowed(true);
14501462
}
14511463

1464+
@Test
1465+
public void getCookieHeader_noCookieHandler() {
1466+
assertThat(HttpEngineDataSource.getCookieHeader(TEST_URL)).isEmpty();
1467+
assertThat(CookieHandler.getDefault()).isNull();
1468+
}
1469+
1470+
@Test
1471+
public void getCookieHeader_emptyCookieHandler() {
1472+
CookieHandler.setDefault(new CookieManager());
1473+
assertThat(HttpEngineDataSource.getCookieHeader(TEST_URL)).isEmpty();
1474+
}
1475+
1476+
@Test
1477+
public void getCookieHeader_cookieHandler() throws Exception {
1478+
CookieManager cm = new CookieManager();
1479+
cm.put(
1480+
new URI(TEST_URL),
1481+
ImmutableMap.of(
1482+
"Set-Cookie", ImmutableList.of(TEST_RESPONSE_SET_COOKIE, TEST_RESPONSE_SET_COOKIE_2)));
1483+
CookieHandler.setDefault(cm);
1484+
1485+
assertThat(HttpEngineDataSource.getCookieHeader(TEST_URL))
1486+
.isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";");
1487+
}
1488+
1489+
@Test
1490+
public void getCookieHeader_cookieHandlerCookie2() throws Exception {
1491+
CookieManager cm = new CookieManager();
1492+
cm.put(
1493+
new URI(TEST_URL),
1494+
ImmutableMap.of(
1495+
"Set-Cookie2", ImmutableList.of(TEST_RESPONSE_SET_COOKIE, TEST_RESPONSE_SET_COOKIE_2)));
1496+
CookieHandler.setDefault(cm);
1497+
1498+
// This asserts the surprising behavior of CookieManager - Set-Cookie2 is translated to Cookie,
1499+
// not Cookie2.
1500+
assertThat(cm.get(new URI(TEST_URL), ImmutableMap.of("", ImmutableList.of()))).isNotEmpty();
1501+
assertThat(cm.get(new URI(TEST_URL), ImmutableMap.of("", ImmutableList.of())).get("Cookie"))
1502+
.containsExactly(TEST_REQUEST_COOKIE, TEST_REQUEST_COOKIE_2);
1503+
assertThat(cm.get(new URI(TEST_URL), ImmutableMap.of("", ImmutableList.of())))
1504+
.doesNotContainKey("Cookie2");
1505+
1506+
assertThat(HttpEngineDataSource.getCookieHeader(TEST_URL))
1507+
.isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";");
1508+
}
1509+
14521510
// Helper methods.
14531511

14541512
private void mockStatusResponse() {

0 commit comments

Comments
 (0)