Remove cronet extension nullness blacklist

PiperOrigin-RevId: 249432337
This commit is contained in:
olly 2019-05-22 13:55:46 +01:00 committed by Toni
parent f74d2294be
commit 118218cc73
3 changed files with 87 additions and 63 deletions

View File

@ -34,6 +34,7 @@ dependencies {
api 'org.chromium.net:cronet-embedded:73.3683.76' api 'org.chromium.net:cronet-embedded:73.3683.76'
implementation project(modulePrefix + 'library-core') implementation project(modulePrefix + 'library-core')
implementation 'androidx.annotation:annotation:1.0.2' implementation 'androidx.annotation:annotation:1.0.2'
compileOnly 'org.checkerframework:checker-qual:' + checkerframeworkVersion
testImplementation project(modulePrefix + 'library') testImplementation project(modulePrefix + 'library')
testImplementation project(modulePrefix + 'testutils-robolectric') testImplementation project(modulePrefix + 'testutils-robolectric')
} }

View File

@ -15,6 +15,8 @@
*/ */
package com.google.android.exoplayer2.ext.cronet; package com.google.android.exoplayer2.ext.cronet;
import static com.google.android.exoplayer2.util.Util.castNonNull;
import android.net.Uri; import android.net.Uri;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import android.text.TextUtils; import android.text.TextUtils;
@ -41,6 +43,7 @@ import java.util.Map.Entry;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
import org.chromium.net.CronetEngine; import org.chromium.net.CronetEngine;
import org.chromium.net.CronetException; import org.chromium.net.CronetException;
import org.chromium.net.NetworkException; import org.chromium.net.NetworkException;
@ -118,7 +121,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
private final int readTimeoutMs; private final int readTimeoutMs;
private final boolean resetTimeoutOnRedirects; private final boolean resetTimeoutOnRedirects;
private final boolean handleSetCookieRequests; private final boolean handleSetCookieRequests;
private final RequestProperties defaultRequestProperties; @Nullable private final RequestProperties defaultRequestProperties;
private final RequestProperties requestProperties; private final RequestProperties requestProperties;
private final ConditionVariable operation; private final ConditionVariable operation;
private final Clock clock; private final Clock clock;
@ -130,18 +133,18 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
// Written from the calling thread only. currentUrlRequest.start() calls ensure writes are visible // Written from the calling thread only. currentUrlRequest.start() calls ensure writes are visible
// to reads made by the Cronet thread. // to reads made by the Cronet thread.
private UrlRequest currentUrlRequest; @Nullable private UrlRequest currentUrlRequest;
private DataSpec currentDataSpec; @Nullable private DataSpec currentDataSpec;
// Reference written and read by calling thread only. Passed to Cronet thread as a local variable. // Reference written and read by calling thread only. Passed to Cronet thread as a local variable.
// operation.open() calls ensure writes into the buffer are visible to reads made by the calling // operation.open() calls ensure writes into the buffer are visible to reads made by the calling
// thread. // thread.
private ByteBuffer readBuffer; @Nullable private ByteBuffer readBuffer;
// Written from the Cronet thread only. operation.open() calls ensure writes are visible to reads // Written from the Cronet thread only. operation.open() calls ensure writes are visible to reads
// made by the calling thread. // made by the calling thread.
private UrlResponseInfo responseInfo; @Nullable private UrlResponseInfo responseInfo;
private IOException exception; @Nullable private IOException exception;
private boolean finished; private boolean finished;
private volatile long currentConnectTimeoutMs; private volatile long currentConnectTimeoutMs;
@ -197,7 +200,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
* @param connectTimeoutMs The connection timeout, in milliseconds. * @param connectTimeoutMs The connection timeout, in milliseconds.
* @param readTimeoutMs The read timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds.
* @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs.
* @param defaultRequestProperties The default request properties to be used. * @param defaultRequestProperties The optional default {@link RequestProperties} to be sent to
* the server as HTTP headers on every request.
*/ */
public CronetDataSource( public CronetDataSource(
CronetEngine cronetEngine, CronetEngine cronetEngine,
@ -206,7 +210,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
int connectTimeoutMs, int connectTimeoutMs,
int readTimeoutMs, int readTimeoutMs,
boolean resetTimeoutOnRedirects, boolean resetTimeoutOnRedirects,
RequestProperties defaultRequestProperties) { @Nullable RequestProperties defaultRequestProperties) {
this( this(
cronetEngine, cronetEngine,
executor, executor,
@ -232,7 +236,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
* @param connectTimeoutMs The connection timeout, in milliseconds. * @param connectTimeoutMs The connection timeout, in milliseconds.
* @param readTimeoutMs The read timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds.
* @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs.
* @param defaultRequestProperties The default request properties to be used. * @param defaultRequestProperties The optional default {@link RequestProperties} to be sent to
* the server as HTTP headers on every request.
* @param handleSetCookieRequests Whether "Set-Cookie" requests on redirect should be forwarded to * @param handleSetCookieRequests Whether "Set-Cookie" requests on redirect should be forwarded to
* the redirect url in the "Cookie" header. * the redirect url in the "Cookie" header.
*/ */
@ -243,7 +248,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
int connectTimeoutMs, int connectTimeoutMs,
int readTimeoutMs, int readTimeoutMs,
boolean resetTimeoutOnRedirects, boolean resetTimeoutOnRedirects,
RequestProperties defaultRequestProperties, @Nullable RequestProperties defaultRequestProperties,
boolean handleSetCookieRequests) { boolean handleSetCookieRequests) {
this( this(
cronetEngine, cronetEngine,
@ -265,7 +270,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
int readTimeoutMs, int readTimeoutMs,
boolean resetTimeoutOnRedirects, boolean resetTimeoutOnRedirects,
Clock clock, Clock clock,
RequestProperties defaultRequestProperties, @Nullable RequestProperties defaultRequestProperties,
boolean handleSetCookieRequests) { boolean handleSetCookieRequests) {
super(/* isNetwork= */ true); super(/* isNetwork= */ true);
this.urlRequestCallback = new UrlRequestCallback(); this.urlRequestCallback = new UrlRequestCallback();
@ -305,6 +310,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
} }
@Override @Override
@Nullable
public Uri getUri() { public Uri getUri() {
return responseInfo == null ? null : Uri.parse(responseInfo.getUrl()); return responseInfo == null ? null : Uri.parse(responseInfo.getUrl());
} }
@ -317,22 +323,23 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
operation.close(); operation.close();
resetConnectTimeout(); resetConnectTimeout();
currentDataSpec = dataSpec; currentDataSpec = dataSpec;
UrlRequest urlRequest;
try { try {
currentUrlRequest = buildRequestBuilder(dataSpec).build(); urlRequest = buildRequestBuilder(dataSpec).build();
currentUrlRequest = urlRequest;
} catch (IOException e) { } catch (IOException e) {
throw new OpenException(e, currentDataSpec, Status.IDLE); throw new OpenException(e, dataSpec, Status.IDLE);
} }
currentUrlRequest.start(); urlRequest.start();
transferInitializing(dataSpec); transferInitializing(dataSpec);
try { try {
boolean connectionOpened = blockUntilConnectTimeout(); boolean connectionOpened = blockUntilConnectTimeout();
if (exception != null) { if (exception != null) {
throw new OpenException(exception, currentDataSpec, getStatus(currentUrlRequest)); throw new OpenException(exception, dataSpec, getStatus(urlRequest));
} else if (!connectionOpened) { } else if (!connectionOpened) {
// The timeout was reached before the connection was opened. // The timeout was reached before the connection was opened.
throw new OpenException( throw new OpenException(new SocketTimeoutException(), dataSpec, getStatus(urlRequest));
new SocketTimeoutException(), dataSpec, getStatus(currentUrlRequest));
} }
} catch (InterruptedException e) { } catch (InterruptedException e) {
Thread.currentThread().interrupt(); Thread.currentThread().interrupt();
@ -340,6 +347,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
} }
// Check for a valid response code. // Check for a valid response code.
UrlResponseInfo responseInfo = Assertions.checkNotNull(this.responseInfo);
int responseCode = responseInfo.getHttpStatusCode(); int responseCode = responseInfo.getHttpStatusCode();
if (responseCode < 200 || responseCode > 299) { if (responseCode < 200 || responseCode > 299) {
InvalidResponseCodeException exception = InvalidResponseCodeException exception =
@ -347,7 +355,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
responseCode, responseCode,
responseInfo.getHttpStatusText(), responseInfo.getHttpStatusText(),
responseInfo.getAllHeaders(), responseInfo.getAllHeaders(),
currentDataSpec); dataSpec);
if (responseCode == 416) { if (responseCode == 416) {
exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE));
} }
@ -358,8 +366,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
if (contentTypePredicate != null) { if (contentTypePredicate != null) {
List<String> contentTypeHeaders = responseInfo.getAllHeaders().get(CONTENT_TYPE); List<String> contentTypeHeaders = responseInfo.getAllHeaders().get(CONTENT_TYPE);
String contentType = isEmpty(contentTypeHeaders) ? null : contentTypeHeaders.get(0); String contentType = isEmpty(contentTypeHeaders) ? null : contentTypeHeaders.get(0);
if (!contentTypePredicate.evaluate(contentType)) { if (contentType != null && !contentTypePredicate.evaluate(contentType)) {
throw new InvalidContentTypeException(contentType, currentDataSpec); throw new InvalidContentTypeException(contentType, dataSpec);
} }
} }
@ -378,7 +386,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
} else { } else {
// If the response is compressed then the content length will be that of the compressed data // If the response is compressed then the content length will be that of the compressed data
// which isn't what we want. Always use the dataSpec length in this case. // which isn't what we want. Always use the dataSpec length in this case.
bytesRemaining = currentDataSpec.length; bytesRemaining = dataSpec.length;
} }
opened = true; opened = true;
@ -397,15 +405,17 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
ByteBuffer readBuffer = this.readBuffer;
if (readBuffer == null) { if (readBuffer == null) {
readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES); readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES);
readBuffer.limit(0); readBuffer.limit(0);
this.readBuffer = readBuffer;
} }
while (!readBuffer.hasRemaining()) { while (!readBuffer.hasRemaining()) {
// Fill readBuffer with more data from Cronet. // Fill readBuffer with more data from Cronet.
operation.close(); operation.close();
readBuffer.clear(); readBuffer.clear();
currentUrlRequest.read(readBuffer); castNonNull(currentUrlRequest).read(readBuffer);
try { try {
if (!operation.block(readTimeoutMs)) { if (!operation.block(readTimeoutMs)) {
throw new SocketTimeoutException(); throw new SocketTimeoutException();
@ -413,20 +423,23 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
} catch (InterruptedException e) { } catch (InterruptedException e) {
// The operation is ongoing so replace readBuffer to avoid it being written to by this // The operation is ongoing so replace readBuffer to avoid it being written to by this
// operation during a subsequent request. // operation during a subsequent request.
readBuffer = null; this.readBuffer = null;
Thread.currentThread().interrupt(); Thread.currentThread().interrupt();
throw new HttpDataSourceException( throw new HttpDataSourceException(
new InterruptedIOException(e), currentDataSpec, HttpDataSourceException.TYPE_READ); new InterruptedIOException(e),
castNonNull(currentDataSpec),
HttpDataSourceException.TYPE_READ);
} catch (SocketTimeoutException e) { } catch (SocketTimeoutException e) {
// The operation is ongoing so replace readBuffer to avoid it being written to by this // The operation is ongoing so replace readBuffer to avoid it being written to by this
// operation during a subsequent request. // operation during a subsequent request.
readBuffer = null; this.readBuffer = null;
throw new HttpDataSourceException(e, currentDataSpec, HttpDataSourceException.TYPE_READ); throw new HttpDataSourceException(
e, castNonNull(currentDataSpec), HttpDataSourceException.TYPE_READ);
} }
if (exception != null) { if (exception != null) {
throw new HttpDataSourceException(exception, currentDataSpec, throw new HttpDataSourceException(
HttpDataSourceException.TYPE_READ); exception, castNonNull(currentDataSpec), HttpDataSourceException.TYPE_READ);
} else if (finished) { } else if (finished) {
bytesRemaining = 0; bytesRemaining = 0;
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
@ -631,7 +644,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
return statusHolder[0]; return statusHolder[0];
} }
private static boolean isEmpty(List<?> list) { @EnsuresNonNullIf(result = false, expression = "#1")
private static boolean isEmpty(@Nullable List<?> list) {
return list == null || list.isEmpty(); return list == null || list.isEmpty();
} }
@ -643,13 +657,15 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
if (request != currentUrlRequest) { if (request != currentUrlRequest) {
return; return;
} }
if (currentDataSpec.httpMethod == DataSpec.HTTP_METHOD_POST) { UrlRequest urlRequest = Assertions.checkNotNull(currentUrlRequest);
DataSpec dataSpec = Assertions.checkNotNull(currentDataSpec);
if (dataSpec.httpMethod == DataSpec.HTTP_METHOD_POST) {
int responseCode = info.getHttpStatusCode(); int responseCode = info.getHttpStatusCode();
// The industry standard is to disregard POST redirects when the status code is 307 or 308. // The industry standard is to disregard POST redirects when the status code is 307 or 308.
if (responseCode == 307 || responseCode == 308) { if (responseCode == 307 || responseCode == 308) {
exception = exception =
new InvalidResponseCodeException( new InvalidResponseCodeException(
responseCode, info.getHttpStatusText(), info.getAllHeaders(), currentDataSpec); responseCode, info.getHttpStatusText(), info.getAllHeaders(), dataSpec);
operation.open(); operation.open();
return; return;
} }
@ -658,13 +674,20 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
resetConnectTimeout(); resetConnectTimeout();
} }
Map<String, List<String>> headers = info.getAllHeaders(); if (!handleSetCookieRequests) {
if (!handleSetCookieRequests || isEmpty(headers.get(SET_COOKIE))) {
request.followRedirect(); request.followRedirect();
} else { return;
currentUrlRequest.cancel(); }
List<String> setCookieHeaders = info.getAllHeaders().get(SET_COOKIE);
if (isEmpty(setCookieHeaders)) {
request.followRedirect();
return;
}
urlRequest.cancel();
DataSpec redirectUrlDataSpec; DataSpec redirectUrlDataSpec;
if (currentDataSpec.httpMethod == DataSpec.HTTP_METHOD_POST) { if (dataSpec.httpMethod == DataSpec.HTTP_METHOD_POST) {
// For POST redirects that aren't 307 or 308, the redirect is followed but request is // For POST redirects that aren't 307 or 308, the redirect is followed but request is
// transformed into a GET. // transformed into a GET.
redirectUrlDataSpec = redirectUrlDataSpec =
@ -672,13 +695,13 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
Uri.parse(newLocationUrl), Uri.parse(newLocationUrl),
DataSpec.HTTP_METHOD_GET, DataSpec.HTTP_METHOD_GET,
/* httpBody= */ null, /* httpBody= */ null,
currentDataSpec.absoluteStreamPosition, dataSpec.absoluteStreamPosition,
currentDataSpec.position, dataSpec.position,
currentDataSpec.length, dataSpec.length,
currentDataSpec.key, dataSpec.key,
currentDataSpec.flags); dataSpec.flags);
} else { } else {
redirectUrlDataSpec = currentDataSpec.withUri(Uri.parse(newLocationUrl)); redirectUrlDataSpec = dataSpec.withUri(Uri.parse(newLocationUrl));
} }
UrlRequest.Builder requestBuilder; UrlRequest.Builder requestBuilder;
try { try {
@ -687,12 +710,11 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
exception = e; exception = e;
return; return;
} }
String cookieHeadersValue = parseCookies(headers.get(SET_COOKIE)); String cookieHeadersValue = parseCookies(setCookieHeaders);
attachCookies(requestBuilder, cookieHeadersValue); attachCookies(requestBuilder, cookieHeadersValue);
currentUrlRequest = requestBuilder.build(); currentUrlRequest = requestBuilder.build();
currentUrlRequest.start(); currentUrlRequest.start();
} }
}
@Override @Override
public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo info) { public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo info) {

View File

@ -60,6 +60,7 @@ android {
dependencies { dependencies {
implementation 'androidx.annotation:annotation:1.0.2' implementation 'androidx.annotation:annotation:1.0.2'
compileOnly 'org.checkerframework:checker-qual:' + checkerframeworkVersion compileOnly 'org.checkerframework:checker-qual:' + checkerframeworkVersion
compileOnly 'org.checkerframework:checker-compat-qual:' + checkerframeworkVersion
androidTestImplementation 'androidx.test:runner:' + androidXTestVersion androidTestImplementation 'androidx.test:runner:' + androidXTestVersion
androidTestImplementation 'androidx.test.ext:junit:' + androidXTestVersion androidTestImplementation 'androidx.test.ext:junit:' + androidXTestVersion
androidTestImplementation 'com.google.truth:truth:' + truthVersion androidTestImplementation 'com.google.truth:truth:' + truthVersion