Skip to content

Commit 53a3144

Browse files
icbakertonihei
authored andcommitted
Rollback of 3b38a7a
PiperOrigin-RevId: 731248658 (cherry picked from commit a6debf4)
1 parent 89cfee6 commit 53a3144

File tree

2 files changed

+31
-151
lines changed

2 files changed

+31
-151
lines changed

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

Lines changed: 23 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,16 @@
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;
4039
import androidx.media3.common.util.UnstableApi;
4140
import androidx.media3.common.util.Util;
4241
import com.google.common.base.Ascii;
4342
import com.google.common.base.Predicate;
44-
import com.google.common.collect.ImmutableMap;
4543
import com.google.common.net.HttpHeaders;
4644
import com.google.common.primitives.Longs;
4745
import com.google.errorprone.annotations.CanIgnoreReturnValue;
4846
import java.io.IOException;
4947
import java.io.InterruptedIOException;
50-
import java.net.CookieHandler;
51-
import java.net.CookieManager;
5248
import java.net.SocketTimeoutException;
53-
import java.net.URI;
5449
import java.net.UnknownHostException;
5550
import java.nio.ByteBuffer;
5651
import java.util.Arrays;
@@ -937,6 +932,14 @@ private static boolean isCompressed(UrlResponseInfo info) {
937932
return false;
938933
}
939934

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+
940943
@Nullable
941944
private static String getFirstHeader(Map<String, List<String>> allHeaders, String headerName) {
942945
@Nullable List<String> headers = allHeaders.get(headerName);
@@ -958,8 +961,7 @@ private static int copyByteBuffer(ByteBuffer src, ByteBuffer dst) {
958961
* A wrapper class that manages a {@link UrlRequest} and the {@link UrlRequestCallback} associated
959962
* with that request.
960963
*/
961-
@VisibleForTesting
962-
/* private */ static final class UrlRequestWrapper {
964+
private static final class UrlRequestWrapper {
963965

964966
private final UrlRequest urlRequest;
965967
private final UrlRequestCallback urlRequestCallback;
@@ -1002,21 +1004,14 @@ public void onStatus(int status) {
10021004
}
10031005
}
10041006

1005-
@VisibleForTesting
1006-
/* private */ final class UrlRequestCallback implements UrlRequest.Callback {
1007+
private final class UrlRequestCallback implements UrlRequest.Callback {
10071008

1008-
private static final String TAG = "HttpEngineDataSource";
10091009
private volatile boolean isClosed = false;
10101010

10111011
public void close() {
10121012
this.isClosed = true;
10131013
}
10141014

1015-
@SuppressWarnings("argument.type.incompatible")
1016-
private void resetDefaultCookieHandler() {
1017-
CookieHandler.setDefault(null);
1018-
}
1019-
10201015
@Override
10211016
public synchronized void onRedirectReceived(
10221017
UrlRequest request, UrlResponseInfo info, String newLocationUrl) {
@@ -1045,34 +1040,24 @@ public synchronized void onRedirectReceived(
10451040
resetConnectTimeout();
10461041
}
10471042

1048-
boolean hasDefaultCookieHandler = CookieHandler.getDefault() != null;
1049-
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
1050-
// a temporary CookieManager is created for the duration of this request - this guarantees
1051-
// redirects preserve the cookies correctly.
1052-
CookieManager cm = new CookieManager();
1053-
CookieHandler.setDefault(cm);
1054-
}
1055-
1056-
storeCookiesFromHeaders(info);
1057-
String cookieHeaders = getCookieHeader(info.getUrl(), info.getHeaders().getAsMap());
1058-
1059-
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
1060-
resetDefaultCookieHandler();
1061-
}
1062-
10631043
boolean shouldKeepPost =
10641044
keepPostFor302Redirects
10651045
&& dataSpec.httpMethod == DataSpec.HTTP_METHOD_POST
10661046
&& responseCode == 302;
10671047

10681048
// request.followRedirect() transforms a POST request into a GET request, so if we want to
10691049
// keep it as a POST we need to fall through to the manual redirect logic below.
1070-
if (!shouldKeepPost) {
1071-
// No cookies, or we're not handling them - so just follow the redirect.
1072-
if (!handleSetCookieRequests || TextUtils.isEmpty(cookieHeaders)) {
1073-
request.followRedirect();
1074-
return;
1075-
}
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;
10761061
}
10771062

10781063
request.cancel();
@@ -1090,15 +1075,13 @@ public synchronized void onRedirectReceived(
10901075
} else {
10911076
redirectUrlDataSpec = dataSpec.withUri(Uri.parse(newLocationUrl));
10921077
}
1093-
1094-
if (!TextUtils.isEmpty(cookieHeaders)) {
1078+
if (!TextUtils.isEmpty(cookieHeadersValue)) {
10951079
Map<String, String> requestHeaders = new HashMap<>();
10961080
requestHeaders.putAll(dataSpec.httpRequestHeaders);
1097-
requestHeaders.put(HttpHeaders.COOKIE, cookieHeaders);
1081+
requestHeaders.put(HttpHeaders.COOKIE, cookieHeadersValue);
10981082
redirectUrlDataSpec =
10991083
redirectUrlDataSpec.buildUpon().setHttpRequestHeaders(requestHeaders).build();
11001084
}
1101-
11021085
UrlRequestWrapper redirectUrlRequestWrapper;
11031086
try {
11041087
redirectUrlRequestWrapper = buildRequestWrapper(redirectUrlDataSpec);
@@ -1113,56 +1096,11 @@ public synchronized void onRedirectReceived(
11131096
currentUrlRequestWrapper.start();
11141097
}
11151098

1116-
/**
1117-
* Stores the cookie headers from the response in the default {@link CookieHandler}.
1118-
*
1119-
*

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

1120-
*/
1121-
static void storeCookiesFromHeaders(UrlResponseInfo info) {
1122-
CookieHandler cookieHandler = CookieHandler.getDefault();
1123-
if (cookieHandler != null) {
1124-
try {
1125-
cookieHandler.put(new URI(info.getUrl()), info.getHeaders().getAsMap());
1126-
} catch (Exception e) {
1127-
Log.w(TAG, "Failed to store cookies in CookieHandler", e);
1128-
}
1129-
}
1130-
}
1131-
1132-
static String getCookieHeader(String url) {
1133-
return getCookieHeader(url, ImmutableMap.of());
1134-
}
1135-
1136-
// getCookieHeader maps Set-Cookie2 (RFC 2965) to Cookie just like CookieManager does.
1137-
static String getCookieHeader(String url, Map<String, List<String>> headers) {
1138-
CookieHandler cookieHandler = CookieHandler.getDefault();
1139-
if (cookieHandler == null) {
1140-
return "";
1141-
}
1142-
1143-
Map<String, List<String>> cookieHeaders = ImmutableMap.of();
1144-
try {
1145-
cookieHeaders = cookieHandler.get(new URI(url), headers);
1146-
} catch (Exception e) {
1147-
Log.w(TAG, "Failed to read cookies from CookieHandler", e);
1148-
}
1149-
1150-
StringBuilder cookies = new StringBuilder();
1151-
if (cookieHeaders.containsKey(HttpHeaders.COOKIE)) {
1152-
List<String> cookiesList = cookieHeaders.get(HttpHeaders.COOKIE);
1153-
for (String cookie : cookiesList) {
1154-
cookies.append(cookie).append("; ");
1155-
}
1156-
}
1157-
return cookies.toString().stripTrailing();
1158-
}
1159-
11601099
@Override
11611100
public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo info) {
11621101
if (isClosed) {
11631102
return;
11641103
}
1165-
storeCookiesFromHeaders(info);
11661104
responseInfo = info;
11671105
operation.open();
11681106
}

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

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,9 @@
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;
5048
import java.io.IOException;
5149
import java.io.InterruptedIOException;
52-
import java.net.CookieHandler;
53-
import java.net.CookieManager;
5450
import java.net.SocketTimeoutException;
55-
import java.net.URI;
5651
import java.net.UnknownHostException;
5752
import java.nio.ByteBuffer;
5853
import java.util.ArrayList;
@@ -85,14 +80,8 @@ public final class HttpEngineDataSourceTest {
8580

8681
private static final int TEST_CONNECT_TIMEOUT_MS = 100;
8782
private static final int TEST_READ_TIMEOUT_MS = 100;
88-
private static final String TEST_URL = "http://google.com/video/";
83+
private static final String TEST_URL = "http://google.com";
8984
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";
9685
private static final byte[] TEST_POST_BODY = Util.getUtf8Bytes("test post body");
9786
private static final long TEST_CONTENT_LENGTH = 16000L;
9887

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

159146
@After
@@ -285,15 +272,15 @@ public void requestSetsRangeHeader() throws HttpDataSourceException {
285272
@Test
286273
public void requestHeadersSet() throws HttpDataSourceException {
287274
Map<String, String> headersSet = new HashMap<>();
288-
when(mockUrlRequestBuilder.addHeader(
289-
ArgumentMatchers.anyString(), ArgumentMatchers.anyString()))
290-
.thenAnswer(
275+
doAnswer(
291276
(invocation) -> {
292277
String key = invocation.getArgument(0);
293278
String value = invocation.getArgument(1);
294279
headersSet.put(key, value);
295280
return null;
296-
});
281+
})
282+
.when(mockUrlRequestBuilder)
283+
.addHeader(ArgumentMatchers.anyString(), ArgumentMatchers.anyString());
297284

298285
dataSourceUnderTest.setRequestProperty("defaultHeader2", "dataSourceOverridesDefault");
299286
dataSourceUnderTest.setRequestProperty("dataSourceHeader1", "dataSourceValue1");
@@ -460,7 +447,8 @@ public void requestOpenValidatesContentTypePredicate() {
460447
assertThat(e).isInstanceOf(HttpDataSource.InvalidContentTypeException.class);
461448
// Check for connection not automatically closed.
462449
verify(mockUrlRequest, never()).cancel();
463-
assertThat(testedContentTypes).containsExactly(TEST_CONTENT_TYPE);
450+
assertThat(testedContentTypes).hasSize(1);
451+
assertThat(testedContentTypes.get(0)).isEqualTo(TEST_CONTENT_TYPE);
464452
}
465453
}
466454

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

12941282
dataSourceUnderTest.open(testPostDataSpec);
12951283

@@ -1461,52 +1449,6 @@ public void allowDirectExecutor() throws HttpDataSourceException {
14611449
verify(mockUrlRequestBuilder).setDirectExecutorAllowed(true);
14621450
}
14631451

1464-
@Test
1465-
public void getCookieHeader_noCookieHandler() {
1466-
assertThat(HttpEngineDataSource.UrlRequestCallback.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.UrlRequestCallback.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.UrlRequestCallback.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.UrlRequestCallback.getCookieHeader(TEST_URL))
1507-
.isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";");
1508-
}
1509-
15101452
// Helper methods.
15111453

15121454
private void mockStatusResponse() {

0 commit comments

Comments
 (0)