loliball/media
1
0
Fork 0
mirror of https://github.com/androidx/media.git synced 2025-04-30 06:46:50 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix a race condition in HttpEngineDataSource

Browse Source 
Pass the temporary CookieHandler as a parameter instead of setting it as a temporary process-default. This avoids a rare race condition, where the player is sending a request with preserveCookies option and runs the `CookieHandler.getDefault()` before a different thread sets a default cookie handler. Over 5 or so lines of code the new default might be reverted to null - this is now fixed.

PiperOrigin-RevId: 738052152
This commit is contained in:
Googler 2025-03-18 10:14:07 -07:00 committed by Copybara-Service
parent 06c0f5549e
commit d0d76f214a
2 changed files with 43 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java
View File

@ -956,32 +956,38 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa
return remaining; return remaining;
} }
/** // Stores the cookie headers from the response in the default {@link CookieHandler}.
* Stores the cookie headers from the response in the default {@link CookieHandler}. private static void storeCookiesFromHeaders(UrlResponseInfo info) {
* storeCookiesFromHeaders(info, CookieHandler.getDefault());
* <p>This is a no-op if the {@link CookieHandler} is not set . }
*/
@VisibleForTesting // Stores the cookie headers from the response in the provided {@link CookieHandler}.
/* private */ static void storeCookiesFromHeaders(UrlResponseInfo info) { private static void storeCookiesFromHeaders(
CookieHandler cookieHandler = CookieHandler.getDefault(); UrlResponseInfo info, @Nullable CookieHandler cookieHandler) {
if (cookieHandler != null) { if (cookieHandler == null) {
return;
}
try { try {
cookieHandler.put(new URI(info.getUrl()), info.getHeaders().getAsMap()); cookieHandler.put(new URI(info.getUrl()), info.getHeaders().getAsMap());
} catch (Exception e) { } catch (Exception e) {
Log.w(TAG, "Failed to store cookies in CookieHandler", e); Log.w(TAG, "Failed to store cookies in CookieHandler", e);
} }
} }
}
@VisibleForTesting @VisibleForTesting
/* private */ static String getCookieHeader(String url) { /* private */ static String getCookieHeader(String url) {
return getCookieHeader(url, ImmutableMap.of()); return getCookieHeader(url, ImmutableMap.of(), CookieHandler.getDefault());
}
@VisibleForTesting
/* private */ static String getCookieHeader(String url, @Nullable CookieHandler cookieHandler) {
return getCookieHeader(url, ImmutableMap.of(), cookieHandler);
} }
// getCookieHeader maps Set-Cookie2 (RFC 2965) to Cookie just like CookieManager does. // getCookieHeader maps Set-Cookie2 (RFC 2965) to Cookie just like CookieManager does.
@VisibleForTesting private static String getCookieHeader(
/* private */ static String getCookieHeader(String url, Map<String, List<String>> headers) { String url, Map<String, List<String>> headers, @Nullable CookieHandler cookieHandler) {
CookieHandler cookieHandler = CookieHandler.getDefault();
if (cookieHandler == null) { if (cookieHandler == null) {
return ""; return "";
} }
@ -1059,11 +1065,6 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa
this.isClosed = true; this.isClosed = true;
} }
@SuppressWarnings("argument.type.incompatible")
private void resetDefaultCookieHandler() {
CookieHandler.setDefault(null);
}
@Override @Override
public synchronized void onRedirectReceived( public synchronized void onRedirectReceived(
UrlRequest request, UrlResponseInfo info, String newLocationUrl) { UrlRequest request, UrlResponseInfo info, String newLocationUrl) {
@ -1092,20 +1093,17 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa
resetConnectTimeout(); resetConnectTimeout();
} }
boolean hasDefaultCookieHandler = CookieHandler.getDefault() != null; CookieHandler cookieHandler = CookieHandler.getDefault();
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
if (cookieHandler == null && handleSetCookieRequests) {
// a temporary CookieManager is created for the duration of this request - this guarantees // a temporary CookieManager is created for the duration of this request - this guarantees
// redirects preserve the cookies correctly. // redirects preserve the cookies correctly.
CookieManager cm = new CookieManager(); cookieHandler = new CookieManager();
CookieHandler.setDefault(cm);
} }
storeCookiesFromHeaders(info); storeCookiesFromHeaders(info, cookieHandler);
String cookieHeaders = getCookieHeader(info.getUrl(), info.getHeaders().getAsMap()); String cookieHeaders =
getCookieHeader(info.getUrl(), info.getHeaders().getAsMap(), cookieHandler);
if (!hasDefaultCookieHandler && handleSetCookieRequests) {
resetDefaultCookieHandler();
}
boolean shouldKeepPost = boolean shouldKeepPost =
keepPostFor302Redirects keepPostFor302Redirects

12
libraries/datasource/src/test/java/androidx/media3/datasource/HttpEngineDataSourceTest.java
View File

@ -1486,6 +1486,18 @@ public final class HttpEngineDataSourceTest {
.isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";"); .isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";");
} }
@Test
public void getCookieHeader_cookieHandlerCustomHandler() throws Exception {
CookieManager cm = new CookieManager();
cm.put(
new URI(TEST_URL),
ImmutableMap.of(
"Set-Cookie", ImmutableList.of(TEST_RESPONSE_SET_COOKIE, TEST_RESPONSE_SET_COOKIE_2)));
assertThat(HttpEngineDataSource.getCookieHeader(TEST_URL, cm))
.isEqualTo(TEST_REQUEST_COOKIE + "; " + TEST_REQUEST_COOKIE_2 + ";");
}
@Test @Test
public void getCookieHeader_cookieHandlerCookie2() throws Exception { public void getCookieHeader_cookieHandlerCookie2() throws Exception {
CookieManager cm = new CookieManager(); CookieManager cm = new CookieManager();