Skip to content

SDK / exoplayer

Go to a project
Commit 675b81e5 by olly Committed by Ian Baker
Options

DataSource: Tighten contract to return -1 on premature end-of-input 

PiperOrigin-RevId: 363001266
parent 6c9f9f9d
Expand all
Inline Side-by-side
Showing with 37 additions and 99 deletions
extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java
...@@ -33,7 +33,6 @@ import com.google.android.exoplayer2.util.Assertions; ...@@ -33,7 +33,6 @@ import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.net.HttpHeaders; import com.google.common.net.HttpHeaders;
import java.io.EOFException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.InterruptedIOException; import java.io.InterruptedIOException;
...@@ -478,10 +477,6 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { ...@@ -478,10 +477,6 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
int read = castNonNull(responseByteStream).read(buffer, offset, readLength); int read = castNonNull(responseByteStream).read(buffer, offset, readLength);
if (read == -1) { if (read == -1) {
if (bytesToRead != C.LENGTH_UNSET) {
// End of stream reached having not read sufficient data.
throw new EOFException();
}
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
......
library/common/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java
...@@ -29,7 +29,6 @@ import com.google.android.exoplayer2.util.Log; ...@@ -29,7 +29,6 @@ import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.net.HttpHeaders; import com.google.common.net.HttpHeaders;
import java.io.EOFException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.InterruptedIOException; import java.io.InterruptedIOException;
...@@ -692,10 +691,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou ...@@ -692,10 +691,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
int read = castNonNull(inputStream).read(buffer, offset, readLength); int read = castNonNull(inputStream).read(buffer, offset, readLength);
if (read == -1) { if (read == -1) {
if (bytesToRead != C.LENGTH_UNSET) {
// End of stream reached having not read sufficient data.
throw new EOFException();
}
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
......
library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java
...@@ -108,7 +108,6 @@ public final class ProgressiveDownloader implements Downloader { ...@@ -108,7 +108,6 @@ public final class ProgressiveDownloader implements Downloader {
new CacheWriter( new CacheWriter(
dataSource, dataSource,
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
progressListener); progressListener);
priorityTaskManager = cacheDataSourceFactory.getUpstreamPriorityTaskManager(); priorityTaskManager = cacheDataSourceFactory.getUpstreamPriorityTaskManager();
......
library/core/src/main/java/com/google/android/exoplayer2/offline/SegmentDownloader.java
...@@ -472,7 +472,6 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme ...@@ -472,7 +472,6 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
new CacheWriter( new CacheWriter(
dataSource, dataSource,
segment.dataSpec, segment.dataSpec,
/* allowShortContent= */ false,
temporaryBuffer, temporaryBuffer,
progressNotifier); progressNotifier);
} }
......
library/core/src/main/java/com/google/android/exoplayer2/upstream/AssetDataSource.java
...@@ -24,7 +24,6 @@ import android.net.Uri; ...@@ -24,7 +24,6 @@ import android.net.Uri;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import java.io.EOFException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
...@@ -111,10 +110,6 @@ public final class AssetDataSource extends BaseDataSource { ...@@ -111,10 +110,6 @@ public final class AssetDataSource extends BaseDataSource {
} }
if (bytesRead == -1) { if (bytesRead == -1) {
if (bytesRemaining != C.LENGTH_UNSET) {
// End of stream reached having not read sufficient data.
throw new AssetDataSourceException(new EOFException());
}
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
if (bytesRemaining != C.LENGTH_UNSET) { if (bytesRemaining != C.LENGTH_UNSET) {
......
library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java
...@@ -24,7 +24,6 @@ import android.content.res.AssetFileDescriptor; ...@@ -24,7 +24,6 @@ import android.content.res.AssetFileDescriptor;
import android.net.Uri; import android.net.Uri;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import java.io.EOFException;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
...@@ -147,10 +146,6 @@ public final class ContentDataSource extends BaseDataSource { ...@@ -147,10 +146,6 @@ public final class ContentDataSource extends BaseDataSource {
} }
if (bytesRead == -1) { if (bytesRead == -1) {
if (bytesRemaining != C.LENGTH_UNSET) {
// End of stream reached having not read sufficient data.
throw new ContentDataSourceException(new EOFException());
}
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
if (bytesRemaining != C.LENGTH_UNSET) { if (bytesRemaining != C.LENGTH_UNSET) {
......
library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheWriter.java
...@@ -23,7 +23,6 @@ import com.google.android.exoplayer2.upstream.DataSpec; ...@@ -23,7 +23,6 @@ import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.util.PriorityTaskManager; import com.google.android.exoplayer2.util.PriorityTaskManager;
import com.google.android.exoplayer2.util.PriorityTaskManager.PriorityTooLowException; import com.google.android.exoplayer2.util.PriorityTaskManager.PriorityTooLowException;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import java.io.EOFException;
import java.io.IOException; import java.io.IOException;
import java.io.InterruptedIOException; import java.io.InterruptedIOException;
...@@ -51,7 +50,6 @@ public final class CacheWriter { ...@@ -51,7 +50,6 @@ public final class CacheWriter {
private final CacheDataSource dataSource; private final CacheDataSource dataSource;
private final Cache cache; private final Cache cache;
private final DataSpec dataSpec; private final DataSpec dataSpec;
private final boolean allowShortContent;
private final String cacheKey; private final String cacheKey;
private final byte[] temporaryBuffer; private final byte[] temporaryBuffer;
@Nullable private final ProgressListener progressListener; @Nullable private final ProgressListener progressListener;
...@@ -65,10 +63,6 @@ public final class CacheWriter { ...@@ -65,10 +63,6 @@ public final class CacheWriter {
/** /**
* @param dataSource A {@link CacheDataSource} that writes to the target cache. * @param dataSource A {@link CacheDataSource} that writes to the target cache.
* @param dataSpec Defines the data to be written. * @param dataSpec Defines the data to be written.
* @param allowShortContent Whether it's allowed for the content to end before the request as
* defined by the {@link DataSpec}. If {@code true} and the request exceeds the length of the
* content, then the content will be cached to the end. If {@code false} and the request
* exceeds the length of the content, {@link #cache} will throw an {@link IOException}.
* @param temporaryBuffer A temporary buffer to be used during caching, or {@code null} if the * @param temporaryBuffer A temporary buffer to be used during caching, or {@code null} if the
* writer should instantiate its own internal temporary buffer. * writer should instantiate its own internal temporary buffer.
* @param progressListener An optional progress listener. * @param progressListener An optional progress listener.
...@@ -76,13 +70,11 @@ public final class CacheWriter { ...@@ -76,13 +70,11 @@ public final class CacheWriter {
public CacheWriter( public CacheWriter(
CacheDataSource dataSource, CacheDataSource dataSource,
DataSpec dataSpec, DataSpec dataSpec,
boolean allowShortContent,
@Nullable byte[] temporaryBuffer, @Nullable byte[] temporaryBuffer,
@Nullable ProgressListener progressListener) { @Nullable ProgressListener progressListener) {
this.dataSource = dataSource; this.dataSource = dataSource;
this.cache = dataSource.getCache(); this.cache = dataSource.getCache();
this.dataSpec = dataSpec; this.dataSpec = dataSpec;
this.allowShortContent = allowShortContent;
this.temporaryBuffer = this.temporaryBuffer =
temporaryBuffer == null ? new byte[DEFAULT_BUFFER_SIZE_BYTES] : temporaryBuffer; temporaryBuffer == null ? new byte[DEFAULT_BUFFER_SIZE_BYTES] : temporaryBuffer;
this.progressListener = progressListener; this.progressListener = progressListener;
...@@ -143,14 +135,6 @@ public final class CacheWriter { ...@@ -143,14 +135,6 @@ public final class CacheWriter {
nextPosition += readBlockToCache(nextPosition, nextRequestLength); nextPosition += readBlockToCache(nextPosition, nextRequestLength);
} }
} }
// TODO: Remove allowShortContent parameter, this code block, and return the number of bytes
// cached. The caller can then check whether fewer bytes were cached than were requested.
if (!allowShortContent
&& dataSpec.length != C.LENGTH_UNSET
&& nextPosition != dataSpec.position + dataSpec.length) {
throw new EOFException();
}
} }
/** /**
...@@ -176,9 +160,11 @@ public final class CacheWriter { ...@@ -176,9 +160,11 @@ public final class CacheWriter {
isDataSourceOpen = true; isDataSourceOpen = true;
} catch (IOException e) { } catch (IOException e) {
Util.closeQuietly(dataSource); Util.closeQuietly(dataSource);
if (allowShortContent // TODO: This exception handling may be required for interop with current HttpDataSource
&& isLastBlock // implementations that (incorrectly) throw a position-out-of-range when opened exactly one
&& DataSourceException.isCausedByPositionOutOfRange(e)) { // byte beyond the end of the resource. It should be removed when the HttpDataSource
// implementations are fixed.
if (isLastBlock && DataSourceException.isCausedByPositionOutOfRange(e)) {
// The length of the request exceeds the length of the content. If we allow shorter // The length of the request exceeds the length of the content. If we allow shorter
// content and are reading the last block, fall through and try again with an unbounded // content and are reading the last block, fall through and try again with an unbounded
// request to read up to the end of the content. // request to read up to the end of the content.
......
library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java
...@@ -364,7 +364,6 @@ public final class CacheDataSourceTest { ...@@ -364,7 +364,6 @@ public final class CacheDataSourceTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, upstream2), new CacheDataSource(cache, upstream2),
unboundedDataSpec, unboundedDataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
/* progressListener= */ null); /* progressListener= */ null);
cacheWriter.cache(); cacheWriter.cache();
...@@ -414,7 +413,6 @@ public final class CacheDataSourceTest { ...@@ -414,7 +413,6 @@ public final class CacheDataSourceTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, upstream2), new CacheDataSource(cache, upstream2),
unboundedDataSpec, unboundedDataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
/* progressListener= */ null); /* progressListener= */ null);
cacheWriter.cache(); cacheWriter.cache();
...@@ -439,7 +437,6 @@ public final class CacheDataSourceTest { ...@@ -439,7 +437,6 @@ public final class CacheDataSourceTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, upstream), new CacheDataSource(cache, upstream),
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
/* progressListener= */ null); /* progressListener= */ null);
cacheWriter.cache(); cacheWriter.cache();
...@@ -476,7 +473,6 @@ public final class CacheDataSourceTest { ...@@ -476,7 +473,6 @@ public final class CacheDataSourceTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, upstream), new CacheDataSource(cache, upstream),
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
/* progressListener= */ null); /* progressListener= */ null);
cacheWriter.cache(); cacheWriter.cache();
......
library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheWriterTest.java
...@@ -30,7 +30,6 @@ import com.google.android.exoplayer2.testutil.TestUtil; ...@@ -30,7 +30,6 @@ import com.google.android.exoplayer2.testutil.TestUtil;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.FileDataSource; import com.google.android.exoplayer2.upstream.FileDataSource;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import java.io.EOFException;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
...@@ -72,7 +71,6 @@ public final class CacheWriterTest { ...@@ -72,7 +71,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")), new DataSpec(Uri.parse("test_data")),
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -94,7 +92,6 @@ public final class CacheWriterTest { ...@@ -94,7 +92,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -106,7 +103,6 @@ public final class CacheWriterTest { ...@@ -106,7 +103,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
new DataSpec(testUri), new DataSpec(testUri),
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -129,7 +125,6 @@ public final class CacheWriterTest { ...@@ -129,7 +125,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -153,7 +148,6 @@ public final class CacheWriterTest { ...@@ -153,7 +148,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
dataSpec, dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -165,7 +159,6 @@ public final class CacheWriterTest { ...@@ -165,7 +159,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
new DataSpec(testUri), new DataSpec(testUri),
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -187,7 +180,6 @@ public final class CacheWriterTest { ...@@ -187,7 +180,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
dataSpec, dataSpec,
/* allowShortContent= */ true,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
...@@ -197,24 +189,6 @@ public final class CacheWriterTest { ...@@ -197,24 +189,6 @@ public final class CacheWriterTest {
} }
@Test @Test
public void cacheThrowEOFException() throws Exception {
FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100);
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
Uri testUri = Uri.parse("test_data");
DataSpec dataSpec = new DataSpec(testUri, /* position= */ 0, /* length= */ 1000);
CacheWriter cacheWriter =
new CacheWriter(
new CacheDataSource(cache, dataSource),
dataSpec,
/* allowShortContent= */ false,
/* temporaryBuffer= */ null,
/* progressListener= */ null);
assertThrows(EOFException.class, cacheWriter::cache);
}
@Test
public void cache_afterFailureOnClose_succeeds() throws Exception { public void cache_afterFailureOnClose_succeeds() throws Exception {
FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100); FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100);
FakeDataSource upstreamDataSource = new FakeDataSource(fakeDataSet); FakeDataSource upstreamDataSource = new FakeDataSource(fakeDataSet);
...@@ -237,7 +211,6 @@ public final class CacheWriterTest { ...@@ -237,7 +211,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
cacheDataSource, cacheDataSource,
new DataSpec(Uri.parse("test_data")), new DataSpec(Uri.parse("test_data")),
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
...@@ -276,7 +249,6 @@ public final class CacheWriterTest { ...@@ -276,7 +249,6 @@ public final class CacheWriterTest {
new CacheWriter( new CacheWriter(
new CacheDataSource(cache, dataSource), new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")), new DataSpec(Uri.parse("test_data")),
/* allowShortContent= */ false,
/* temporaryBuffer= */ null, /* temporaryBuffer= */ null,
counters); counters);
cacheWriter.cache(); cacheWriter.cache();
......
testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java
...@@ -232,24 +232,27 @@ public abstract class DataSourceContractTest { ...@@ -232,24 +232,27 @@ 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 {
long length = dataSource.open(dataSpec);
// 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
// fewer bytes will be read in this case.
if (length != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(0);
}
try { try {
byte[] data = long length = dataSource.open(dataSpec);
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource); // The DataSource.open() contract requires the returned length to equal the length in the
assertThat(data).isEmpty(); // DataSpec if set. This is true even though the DataSource implementation may know that
// fewer bytes will be read in this case.
if (length != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(0);
}
try {
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
assertThat(data).isEmpty();
} catch (IOException e) {
// TODO: Remove this catch once the one below is removed.
throw new RuntimeException(e);
}
} catch (IOException e) { } catch (IOException e) {
// TODO: Remove this catch and require that implementations do not throw. // TODO: Remove this catch and require that implementations do not throw.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
} }
} 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();
} }
...@@ -275,22 +278,25 @@ public abstract class DataSourceContractTest { ...@@ -275,22 +278,25 @@ public abstract class DataSourceContractTest {
.setLength(1) .setLength(1)
.build(); .build();
try { try {
long length = dataSource.open(dataSpec);
// 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
// fewer bytes will be read in this case.
assertThat(length).isEqualTo(1);
try { try {
byte[] data = long length = dataSource.open(dataSpec);
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource); // The DataSource.open() contract requires the returned length to equal the length in the
assertThat(data).isEmpty(); // DataSpec if set. This is true even though the DataSource implementation may know that
// fewer bytes will be read in this case.
assertThat(length).isEqualTo(1);
try {
byte[] data =
unboundedReadsAreIndefinite() ? Util.EMPTY_BYTE_ARRAY : Util.readToEnd(dataSource);
assertThat(data).isEmpty();
} catch (IOException e) {
// TODO: Remove this catch once the one below is removed.
throw new RuntimeException(e);
}
} catch (IOException e) { } catch (IOException e) {
// TODO: Remove this catch and require that implementations do not throw. // TODO: Remove this catch and require that implementations do not throw.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
} }
} 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