Improving handling of atoms with size less than header in FragmentedMp4Extractor.

These currently lead to cryptic ArrayIndexOutOfBoundsExceptions being thrown from System.arraycopy() so my proposal is to throw a more useful ParserException instead.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=142087132
This commit is contained in:
cblay 2016-12-14 17:37:27 -08:00 committed by Oliver Woodman
parent 86adc64403
commit 37317520f4
4 changed files with 94 additions and 6 deletions

View file

@ -16,6 +16,7 @@
package com.google.android.exoplayer2.extractor.mp4;
import android.test.InstrumentationTestCase;
import com.google.android.exoplayer2.ParserException;
import com.google.android.exoplayer2.extractor.Extractor;
import com.google.android.exoplayer2.testutil.TestUtil;
@ -24,13 +25,21 @@ import com.google.android.exoplayer2.testutil.TestUtil;
*/
public final class FragmentedMp4ExtractorTest extends InstrumentationTestCase {
private static final TestUtil.ExtractorFactory EXTRACTOR_FACTORY =
new TestUtil.ExtractorFactory() {
@Override
public Extractor create() {
return new FragmentedMp4Extractor();
}
};
public void testSample() throws Exception {
TestUtil.assertOutput(new TestUtil.ExtractorFactory() {
@Override
public Extractor create() {
return new FragmentedMp4Extractor();
}
}, "mp4/sample_fragmented.mp4", getInstrumentation());
TestUtil.assertOutput(EXTRACTOR_FACTORY, "mp4/sample_fragmented.mp4", getInstrumentation());
}
public void testAtomWithZeroSize() throws Exception {
TestUtil.assertThrows(EXTRACTOR_FACTORY, "mp4/sample_fragmented_zero_size_atom.mp4",
getInstrumentation(), ParserException.class);
}
}

View file

@ -257,6 +257,10 @@ public final class FragmentedMp4Extractor implements Extractor {
atomSize = atomHeader.readUnsignedLongToLong();
}
if (atomSize < atomHeaderBytesRead) {
throw new ParserException("Atom size less than header length (unsupported).");
}
long atomPosition = input.getPosition() - atomHeaderBytesRead;
if (atomType == Atom.TYPE_moof) {
// The data positions may be updated when parsing the tfhd/trun.

View file

@ -300,6 +300,81 @@ public class TestUtil {
return extractorOutput;
}
/**
* Calls {@link #assertThrows(Extractor, byte[], Class, boolean, boolean, boolean)} with all
* possible combinations of "simulate" parameters.
*
* @param factory An {@link ExtractorFactory} which creates instances of the {@link Extractor}
* class which is to be tested.
* @param sampleFile The path to the input sample.
* @param instrumentation To be used to load the sample file.
* @param expectedThrowable Expected {@link Throwable} class.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
* @see #assertThrows(Extractor, byte[], Class, boolean, boolean, boolean)
*/
public static void assertThrows(ExtractorFactory factory, String sampleFile,
Instrumentation instrumentation, Class<? extends Throwable> expectedThrowable)
throws IOException, InterruptedException {
byte[] fileData = getByteArray(instrumentation, sampleFile);
assertThrows(factory, fileData, expectedThrowable);
}
/**
* Calls {@link #assertThrows(Extractor, byte[], Class, boolean, boolean, boolean)} with all
* possible combinations of "simulate" parameters.
*
* @param factory An {@link ExtractorFactory} which creates instances of the {@link Extractor}
* class which is to be tested.
* @param fileData Content of the input file.
* @param expectedThrowable Expected {@link Throwable} class.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
* @see #assertThrows(Extractor, byte[], Class, boolean, boolean, boolean)
*/
public static void assertThrows(ExtractorFactory factory, byte[] fileData,
Class<? extends Throwable> expectedThrowable) throws IOException, InterruptedException {
assertThrows(factory.create(), fileData, expectedThrowable, false, false, false);
assertThrows(factory.create(), fileData, expectedThrowable, true, false, false);
assertThrows(factory.create(), fileData, expectedThrowable, false, true, false);
assertThrows(factory.create(), fileData, expectedThrowable, true, true, false);
assertThrows(factory.create(), fileData, expectedThrowable, false, false, true);
assertThrows(factory.create(), fileData, expectedThrowable, true, false, true);
assertThrows(factory.create(), fileData, expectedThrowable, false, true, true);
assertThrows(factory.create(), fileData, expectedThrowable, true, true, true);
}
/**
* Asserts {@code extractor} throws {@code expectedThrowable} while consuming {@code sampleFile}.
*
* @param extractor The {@link Extractor} to be tested.
* @param fileData Content of the input file.
* @param expectedThrowable Expected {@link Throwable} class.
* @param simulateIOErrors If true simulates IOErrors.
* @param simulateUnknownLength If true simulates unknown input length.
* @param simulatePartialReads If true simulates partial reads.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
*/
public static void assertThrows(Extractor extractor, byte[] fileData,
Class<? extends Throwable> expectedThrowable, boolean simulateIOErrors,
boolean simulateUnknownLength, boolean simulatePartialReads) throws IOException,
InterruptedException {
FakeExtractorInput input = new FakeExtractorInput.Builder().setData(fileData)
.setSimulateIOErrors(simulateIOErrors)
.setSimulateUnknownLength(simulateUnknownLength)
.setSimulatePartialReads(simulatePartialReads).build();
try {
consumeTestData(extractor, input, 0, true);
throw new AssertionError(expectedThrowable.getSimpleName() + " expected but not thrown");
} catch (Throwable throwable) {
if (expectedThrowable.equals(throwable.getClass())) {
return; // Pass!
}
throw throwable;
}
}
public static void recursiveDelete(File fileOrDirectory) {
if (fileOrDirectory.isDirectory()) {
for (File child : fileOrDirectory.listFiles()) {