Commit 1affbf93 by olly Committed by Ian Baker

DataSources: Enforce that opening at end-of-resource succeeds

- Update the three `HttpDataSource` implementations to use the
  Content-Range response header to determine when this is the
  case. The Content-Range header is included when the status
  code is 416. See [here](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/416).
- Update `ByteArrayDataSource` to conform to the requirement.
- Update `DataSourceContractTest` to enforce the requirement.

PiperOrigin-RevId: 363642114
parent 8337991b
...@@ -565,6 +565,16 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { ...@@ -565,6 +565,16 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
int responseCode = responseInfo.getHttpStatusCode(); int responseCode = responseInfo.getHttpStatusCode();
Map<String, List<String>> responseHeaders = responseInfo.getAllHeaders(); Map<String, List<String>> responseHeaders = responseInfo.getAllHeaders();
if (responseCode < 200 || responseCode > 299) { if (responseCode < 200 || responseCode > 299) {
if (responseCode == 416) {
long documentSize =
HttpUtil.getDocumentSize(getFirstHeader(responseHeaders, HttpHeaders.CONTENT_RANGE));
if (dataSpec.position == documentSize) {
opened = true;
transferStarted(dataSpec);
return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0;
}
}
byte[] responseBody; byte[] responseBody;
try { try {
responseBody = readResponseBody(); responseBody = readResponseBody();
......
...@@ -28,6 +28,7 @@ import com.google.android.exoplayer2.upstream.DataSource; ...@@ -28,6 +28,7 @@ import com.google.android.exoplayer2.upstream.DataSource;
import com.google.android.exoplayer2.upstream.DataSourceException; import com.google.android.exoplayer2.upstream.DataSourceException;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.HttpDataSource; import com.google.android.exoplayer2.upstream.HttpDataSource;
import com.google.android.exoplayer2.upstream.HttpUtil;
import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.upstream.TransferListener;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
...@@ -273,7 +274,8 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { ...@@ -273,7 +274,8 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
@Override @Override
public long open(DataSpec dataSpec) throws HttpDataSourceException { public long open(DataSpec dataSpec) throws HttpDataSourceException {
this.dataSpec = dataSpec; this.dataSpec = dataSpec;
this.bytesRead = 0; bytesRead = 0;
bytesToRead = 0;
transferInitializing(dataSpec); transferInitializing(dataSpec);
Request request = makeRequest(dataSpec); Request request = makeRequest(dataSpec);
...@@ -298,6 +300,16 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { ...@@ -298,6 +300,16 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
// Check for a valid response code. // Check for a valid response code.
if (!response.isSuccessful()) { if (!response.isSuccessful()) {
if (responseCode == 416) {
long documentSize =
HttpUtil.getDocumentSize(response.headers().get(HttpHeaders.CONTENT_RANGE));
if (dataSpec.position == documentSize) {
opened = true;
transferStarted(dataSpec);
return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0;
}
}
byte[] errorResponseBody; byte[] errorResponseBody;
try { try {
errorResponseBody = Util.toByteArray(Assertions.checkNotNull(responseByteStream)); errorResponseBody = Util.toByteArray(Assertions.checkNotNull(responseByteStream));
......
...@@ -330,7 +330,8 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou ...@@ -330,7 +330,8 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
@Override @Override
public long open(DataSpec dataSpec) throws HttpDataSourceException { public long open(DataSpec dataSpec) throws HttpDataSourceException {
this.dataSpec = dataSpec; this.dataSpec = dataSpec;
this.bytesRead = 0; bytesRead = 0;
bytesToRead = 0;
transferInitializing(dataSpec); transferInitializing(dataSpec);
try { try {
...@@ -359,6 +360,16 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou ...@@ -359,6 +360,16 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
// Check for a valid response code. // Check for a valid response code.
if (responseCode < 200 || responseCode > 299) { if (responseCode < 200 || responseCode > 299) {
Map<String, List<String>> headers = connection.getHeaderFields(); Map<String, List<String>> headers = connection.getHeaderFields();
if (responseCode == 416) {
long documentSize =
HttpUtil.getDocumentSize(connection.getHeaderField(HttpHeaders.CONTENT_RANGE));
if (dataSpec.position == documentSize) {
opened = true;
transferStarted(dataSpec);
return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0;
}
}
@Nullable InputStream errorStream = connection.getErrorStream(); @Nullable InputStream errorStream = connection.getErrorStream();
byte[] errorResponseBody; byte[] errorResponseBody;
try { try {
...@@ -371,7 +382,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou ...@@ -371,7 +382,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
InvalidResponseCodeException exception = InvalidResponseCodeException exception =
new InvalidResponseCodeException( new InvalidResponseCodeException(
responseCode, responseMessage, headers, dataSpec, errorResponseBody); responseCode, responseMessage, headers, dataSpec, errorResponseBody);
if (responseCode == 416) { if (responseCode == 416) {
exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE));
} }
......
...@@ -32,6 +32,8 @@ public final class HttpUtil { ...@@ -32,6 +32,8 @@ public final class HttpUtil {
private static final String TAG = "HttpUtil"; private static final String TAG = "HttpUtil";
private static final Pattern CONTENT_RANGE_WITH_START_AND_END = private static final Pattern CONTENT_RANGE_WITH_START_AND_END =
Pattern.compile("bytes (\\d+)-(\\d+)/(?:\\d+|\\*)"); Pattern.compile("bytes (\\d+)-(\\d+)/(?:\\d+|\\*)");
private static final Pattern CONTENT_RANGE_WITH_SIZE =
Pattern.compile("bytes (?:(?:\\d+-\\d+)|\\*)/(\\d+)");
/** Class only contains static methods. */ /** Class only contains static methods. */
private HttpUtil() {} private HttpUtil() {}
...@@ -60,6 +62,22 @@ public final class HttpUtil { ...@@ -60,6 +62,22 @@ public final class HttpUtil {
} }
/** /**
* Attempts to parse the document size from a {@link HttpHeaders#CONTENT_RANGE Content-Range
* header}.
*
* @param contentRangeHeader The {@link HttpHeaders#CONTENT_RANGE Content-Range header}, or {@code
* null} if not set.
* @return The document size, or {@link C#LENGTH_UNSET} if it could not be determined.
*/
public static long getDocumentSize(@Nullable String contentRangeHeader) {
if (TextUtils.isEmpty(contentRangeHeader)) {
return C.LENGTH_UNSET;
}
Matcher matcher = CONTENT_RANGE_WITH_SIZE.matcher(contentRangeHeader);
return matcher.matches() ? Long.parseLong(checkNotNull(matcher.group(1))) : C.LENGTH_UNSET;
}
/**
* Attempts to parse the length of a response body from the corresponding response headers. * Attempts to parse the length of a response body from the corresponding response headers.
* *
* @param contentLengthHeader The {@link HttpHeaders#CONTENT_LENGTH Content-Length header}, or * @param contentLengthHeader The {@link HttpHeaders#CONTENT_LENGTH Content-Length header}, or
......
...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.upstream; ...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.upstream;
import static com.google.android.exoplayer2.upstream.HttpUtil.buildRangeRequestHeader; import static com.google.android.exoplayer2.upstream.HttpUtil.buildRangeRequestHeader;
import static com.google.android.exoplayer2.upstream.HttpUtil.getContentLength; import static com.google.android.exoplayer2.upstream.HttpUtil.getContentLength;
import static com.google.android.exoplayer2.upstream.HttpUtil.getDocumentSize;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.ext.junit.runners.AndroidJUnit4;
...@@ -78,4 +79,22 @@ public class HttpUtilTest { ...@@ -78,4 +79,22 @@ public class HttpUtilTest {
assertThat(getContentLength(null, "unhandled 5-9/100")).isEqualTo(C.LENGTH_UNSET); assertThat(getContentLength(null, "unhandled 5-9/100")).isEqualTo(C.LENGTH_UNSET);
assertThat(getContentLength("10", "unhandled 0-4/100")).isEqualTo(10); assertThat(getContentLength("10", "unhandled 0-4/100")).isEqualTo(10);
} }
@Test
public void getDocumentSize_noHeader_returnsUnset() {
assertThat(getDocumentSize(null)).isEqualTo(C.LENGTH_UNSET);
assertThat(getDocumentSize("")).isEqualTo(C.LENGTH_UNSET);
}
@Test
public void getDocumentSize_returnsSize() {
assertThat(getDocumentSize("bytes */20")).isEqualTo(20);
assertThat(getDocumentSize("bytes 0-4/20")).isEqualTo(20);
}
@Test
public void getDocumentSize_ignoresUnhandledRangeUnits() {
assertThat(getDocumentSize("unhandled */20")).isEqualTo(C.LENGTH_UNSET);
assertThat(getDocumentSize("unhandled 0-4/20")).isEqualTo(C.LENGTH_UNSET);
}
} }
...@@ -47,16 +47,17 @@ public final class ByteArrayDataSource extends BaseDataSource { ...@@ -47,16 +47,17 @@ public final class ByteArrayDataSource extends BaseDataSource {
public long open(DataSpec dataSpec) throws IOException { public long open(DataSpec dataSpec) throws IOException {
uri = dataSpec.uri; uri = dataSpec.uri;
transferInitializing(dataSpec); transferInitializing(dataSpec);
if (dataSpec.position >= data.length) { if (dataSpec.position > data.length) {
throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
readPosition = (int) dataSpec.position; readPosition = (int) dataSpec.position;
bytesRemaining = bytesRemaining = data.length - (int) dataSpec.position;
(int) if (dataSpec.length != C.LENGTH_UNSET) {
(dataSpec.length == C.LENGTH_UNSET ? data.length - dataSpec.position : dataSpec.length); bytesRemaining = (int) min(bytesRemaining, dataSpec.length);
}
opened = true; opened = true;
transferStarted(dataSpec); transferStarted(dataSpec);
return bytesRemaining; return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : bytesRemaining;
} }
@Override @Override
......
...@@ -74,17 +74,17 @@ public final class ByteArrayDataSourceTest { ...@@ -74,17 +74,17 @@ public final class ByteArrayDataSourceTest {
// And with bound. // And with bound.
readTestData(TEST_DATA, 1, 6, 1, 0, 1, false); readTestData(TEST_DATA, 1, 6, 1, 0, 1, false);
// Read from the last possible offset without bound. // Read from the last possible offset without bound.
readTestData(TEST_DATA, TEST_DATA.length - 1, C.LENGTH_UNSET, 1, 0, 1, false); readTestData(TEST_DATA, TEST_DATA.length, C.LENGTH_UNSET, 1, 0, 1, false);
// And with bound. // And with bound.
readTestData(TEST_DATA, TEST_DATA.length - 1, 1, 1, 0, 1, false); readTestData(TEST_DATA, TEST_DATA.length, 1, 1, 0, 1, false);
} }
@Test @Test
public void readFromInvalidOffsets() { public void readFromInvalidOffsets() {
// Read from first invalid offset and check failure without bound. // Read from first invalid offset and check failure without bound.
readTestData(TEST_DATA, TEST_DATA.length, C.LENGTH_UNSET, 1, 0, 1, true); readTestData(TEST_DATA, TEST_DATA.length + 1, C.LENGTH_UNSET, 1, 0, 1, true);
// And with bound. // And with bound.
readTestData(TEST_DATA, TEST_DATA.length, 1, 1, 0, 1, true); readTestData(TEST_DATA, TEST_DATA.length + 1, 1, 1, 0, 1, true);
} }
/** /**
...@@ -100,8 +100,10 @@ public final class ByteArrayDataSourceTest { ...@@ -100,8 +100,10 @@ public final class ByteArrayDataSourceTest {
*/ */
private void readTestData(byte[] testData, int dataOffset, int dataLength, int outputBufferLength, private void readTestData(byte[] testData, int dataOffset, int dataLength, int outputBufferLength,
int writeOffset, int maxReadLength, boolean expectFailOnOpen) { int writeOffset, int maxReadLength, boolean expectFailOnOpen) {
int expectedFinalBytesRead = int expectedFinalBytesRead = testData.length - dataOffset;
dataLength == C.LENGTH_UNSET ? (testData.length - dataOffset) : dataLength; if (dataLength != C.LENGTH_UNSET) {
expectedFinalBytesRead = min(expectedFinalBytesRead, dataLength);
}
ByteArrayDataSource dataSource = new ByteArrayDataSource(testData); ByteArrayDataSource dataSource = new ByteArrayDataSource(testData);
boolean opened = false; boolean opened = false;
try { try {
...@@ -111,7 +113,8 @@ public final class ByteArrayDataSourceTest { ...@@ -111,7 +113,8 @@ public final class ByteArrayDataSourceTest {
assertThat(expectFailOnOpen).isFalse(); assertThat(expectFailOnOpen).isFalse();
// Verify the resolved length is as we expect. // Verify the resolved length is as we expect.
assertThat(length).isEqualTo(expectedFinalBytesRead); assertThat(length)
.isEqualTo(dataLength != C.LENGTH_UNSET ? dataLength : expectedFinalBytesRead);
byte[] outputBuffer = new byte[outputBufferLength]; byte[] outputBuffer = new byte[outputBufferLength];
int accumulatedBytesRead = 0; int accumulatedBytesRead = 0;
......
...@@ -232,27 +232,17 @@ public abstract class DataSourceContractTest { ...@@ -232,27 +232,17 @@ public abstract class DataSourceContractTest {
DataSpec dataSpec = DataSpec dataSpec =
new DataSpec.Builder().setUri(resource.getUri()).setPosition(resourceLength).build(); new DataSpec.Builder().setUri(resource.getUri()).setPosition(resourceLength).build();
try { try {
try {
long length = dataSource.open(dataSpec); long length = dataSource.open(dataSpec);
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
// The DataSource.open() contract requires the returned length to equal the length in the // The DataSource.open() contract requires the returned length to equal the length in the
// DataSpec if set. This is true even though the DataSource implementation may know that // DataSpec if set. This is true even though the DataSource implementation may know that
// fewer bytes will be read in this case. // fewer bytes will be read in this case.
if (length != C.LENGTH_UNSET) { if (length != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(0); assertThat(length).isEqualTo(0);
} }
try {
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
assertThat(data).isEmpty(); assertThat(data).isEmpty();
} catch (IOException e) {
// TODO: Remove this catch once the one below is removed.
throw new RuntimeException(e);
}
} catch (IOException e) {
// TODO: Remove this catch and require that implementations do not throw.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
}
} finally { } finally {
dataSource.close(); dataSource.close();
} }
...@@ -278,25 +268,15 @@ public abstract class DataSourceContractTest { ...@@ -278,25 +268,15 @@ public abstract class DataSourceContractTest {
.setLength(1) .setLength(1)
.build(); .build();
try { try {
try {
long length = dataSource.open(dataSpec); long length = dataSource.open(dataSpec);
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
// The DataSource.open() contract requires the returned length to equal the length in the // The DataSource.open() contract requires the returned length to equal the length in the
// DataSpec if set. This is true even though the DataSource implementation may know that // DataSpec if set. This is true even though the DataSource implementation may know that
// fewer bytes will be read in this case. // fewer bytes will be read in this case.
assertThat(length).isEqualTo(1); assertThat(length).isEqualTo(1);
try {
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
assertThat(data).isEmpty(); assertThat(data).isEmpty();
} catch (IOException e) {
// TODO: Remove this catch once the one below is removed.
throw new RuntimeException(e);
}
} catch (IOException e) {
// TODO: Remove this catch and require that implementations do not throw.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
}
} finally { } finally {
dataSource.close(); dataSource.close();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment