webm_extractor: Re-sync to next level 1 element on invalid data

Try re-sync'ing to the next level 1 element when invalid data is found. This
corrects the behavior for test case 4 in the mkv test suite.

Partially Fixes Issue #631

library/src/androidTest/java/com/google/android/exoplayer/extractor/webm/DefaultEbmlReaderTest.java

@@ -196,6 +196,11 @@ public class DefaultEbmlReaderTest extends TestCase {
     }
 
     @Override
+    public boolean isLevel1Element(int id) {
+      return false;
+    }
+
+    @Override
     public void startMasterElement(int id, long contentPosition, long contentSize) {
       events.add(formatEvent(id, "start contentPosition=" + contentPosition
           + " contentSize=" + contentSize));

library/src/androidTest/java/com/google/android/exoplayer/extractor/webm/VarintReaderTest.java

@@ -95,17 +95,26 @@ public class VarintReaderTest extends TestCase {
     int bytesRead = input.read(new byte[1], 0, 1);
     assertEquals(1, bytesRead);
     // End of input allowed.
-    long result = reader.readUnsignedVarint(input, true, false);
-    assertEquals(-1, result);
+    long result = reader.readUnsignedVarint(input, true, false, 8);
+    assertEquals(C.RESULT_END_OF_INPUT, result);
     // End of input not allowed.
     try {
-      reader.readUnsignedVarint(input, false, false);
+      reader.readUnsignedVarint(input, false, false, 8);
       fail();
     } catch (EOFException e) {
       // Expected.
     }
   }
 
+  public void testReadVarintExceedsMaximumAllowedLength() throws IOException, InterruptedException {
+    VarintReader reader = new VarintReader();
+    DataSource dataSource = buildDataSource(DATA_8_BYTE_0);
+    dataSource.open(new DataSpec(Uri.parse(TEST_URI)));
+    ExtractorInput input = new DefaultExtractorInput(dataSource, 0, C.LENGTH_UNBOUNDED);
+    long result = reader.readUnsignedVarint(input, false, true, 4);
+    assertEquals(C.RESULT_MAX_LENGTH_EXCEEDED, result);
+  }
+
   public void testReadVarint() throws IOException, InterruptedException {
     VarintReader reader = new VarintReader();
     testReadVarint(reader, true, DATA_1_BYTE_0, 1, 0);
@@ -183,7 +192,7 @@ public class VarintReaderTest extends TestCase {
     DataSource dataSource = buildDataSource(data);
     dataSource.open(new DataSpec(Uri.parse(TEST_URI)));
     ExtractorInput input = new DefaultExtractorInput(dataSource, 0, C.LENGTH_UNBOUNDED);
-    long result = reader.readUnsignedVarint(input, false, removeMask);
+    long result = reader.readUnsignedVarint(input, false, removeMask, 8);
     assertEquals(expectedLength, input.getPosition());
     assertEquals(expectedValue, result);
   }
@@ -198,7 +207,7 @@ public class VarintReaderTest extends TestCase {
       dataSource.open(new DataSpec(Uri.parse(TEST_URI), position, C.LENGTH_UNBOUNDED, null));
       input = new DefaultExtractorInput(dataSource, position, C.LENGTH_UNBOUNDED);
       try {
-        result = reader.readUnsignedVarint(input, false, removeMask);
+        result = reader.readUnsignedVarint(input, false, removeMask, 8);
         position = input.getPosition();
       } catch (IOException e) {
         // Expected. We'll try again from the position that the input was advanced to.

library/src/main/java/com/google/android/exoplayer/C.java

@@ -90,6 +90,11 @@ public final class C {
    */
   public static final int RESULT_END_OF_INPUT = -1;
 
+  /**
+   * A return value for methods where the length of parsed data exceeds the maximum length allowed.
+   */
+  public static final int RESULT_MAX_LENGTH_EXCEEDED = -2;
+
   private C() {}
 
 }

library/src/main/java/com/google/android/exoplayer/extractor/webm/DefaultEbmlReader.java

@@ -19,6 +19,7 @@ import com.google.android.exoplayer.C;
 import com.google.android.exoplayer.extractor.ExtractorInput;
 import com.google.android.exoplayer.util.Assertions;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.util.Stack;
@@ -32,6 +33,9 @@ import java.util.Stack;
   private static final int ELEMENT_STATE_READ_CONTENT_SIZE = 1;
   private static final int ELEMENT_STATE_READ_CONTENT = 2;
 
+  private static final int MAX_ID_BYTES = 4;
+  private static final int MAX_LENGTH_BYTES = 8;
+
   private static final int MAX_INTEGER_ELEMENT_SIZE_BYTES = 8;
   private static final int VALID_FLOAT32_ELEMENT_SIZE_BYTES = 4;
   private static final int VALID_FLOAT64_ELEMENT_SIZE_BYTES = 8;
@@ -68,8 +72,11 @@ import java.util.Stack;
       }
 
       if (elementState == ELEMENT_STATE_READ_ID) {
-        long result = varintReader.readUnsignedVarint(input, true, false);
-        if (result == -1) {
+        long result = varintReader.readUnsignedVarint(input, true, false, MAX_ID_BYTES);
+        if (result == C.RESULT_MAX_LENGTH_EXCEEDED) {
+          result = maybeResyncToNextLevel1Element(input);
+        }
+        if (result == C.RESULT_END_OF_INPUT) {
           return false;
         }
         // Element IDs are at most 4 bytes, so we can cast to integers.
@@ -78,7 +85,7 @@ import java.util.Stack;
       }
 
       if (elementState == ELEMENT_STATE_READ_CONTENT_SIZE) {
-        elementContentSize = varintReader.readUnsignedVarint(input, false, true);
+        elementContentSize = varintReader.readUnsignedVarint(input, false, true, MAX_LENGTH_BYTES);
         elementState = ELEMENT_STATE_READ_CONTENT;
       }
 
@@ -128,6 +135,35 @@ import java.util.Stack;
   }
 
   /**
+   * Does a byte by byte search to try and find the next level 1 element. This method is called if
+   * some invalid data is encountered in the parser.
+   *
+   * @param input The {@link ExtractorInput} from which data has to be read.
+   * @return id of the next level 1 element that has been found.
+   * @throws EOFException If the end of input was encountered when searching for the next level 1
+   *     element.
+   * @throws IOException If an error occurs reading from the input.
+   * @throws InterruptedException If the thread is interrupted.
+   */
+  private long maybeResyncToNextLevel1Element(ExtractorInput input) throws EOFException,
+      IOException, InterruptedException {
+    while (true) {
+      input.resetPeekPosition();
+      input.peekFully(scratch, 0, MAX_ID_BYTES);
+      int varintLength = VarintReader.parseUnsignedVarintLength(scratch[0]);
+      if (varintLength != -1 && varintLength <= MAX_ID_BYTES) {
+        int potentialId = (int) VarintReader.assembleVarint(scratch, varintLength, false);
+        if (output.isLevel1Element(potentialId)) {
+          input.skipFully(varintLength);
+          input.resetPeekPosition();
+          return potentialId;
+        }
+      }
+      input.skipFully(1);
+    }
+  }
+
+  /**
    * Reads and returns an integer of length {@code byteLength} from the {@link ExtractorInput}.
    *
    * @param input The {@link ExtractorInput} from which to read.

library/src/main/java/com/google/android/exoplayer/extractor/webm/EbmlReaderOutput.java

@@ -37,6 +37,14 @@ import java.io.IOException;
   int getElementType(int id);
 
   /**
+   * Checks if the given id is that of a level 1 element.
+   *
+   * @param id The element ID.
+   * @return True the given id is that of a level 1 element. false otherwise.
+   */
+  boolean isLevel1Element(int id);
+
+  /**
    * Called when the start of a master element is encountered.
    * <p>
    * Following events should be considered as taking place within this element until a matching call

library/src/main/java/com/google/android/exoplayer/extractor/webm/VarintReader.java

 package com.google.android.exoplayer.extractor.webm;
 
+import com.google.android.exoplayer.C;
 import com.google.android.exoplayer.extractor.ExtractorInput;
 
 import java.io.EOFException;
@@ -19,8 +20,8 @@ import java.io.IOException;
    *
    * <p>{@code 0x80} is a one-byte integer, {@code 0x40} is two bytes, and so on up to eight bytes.
    */
-  private static final int[] VARINT_LENGTH_MASKS = new int[] {
-    0x80, 0x40, 0x20, 0x10, 0x08, 0x04, 0x02, 0x01
+  private static final long[] VARINT_LENGTH_MASKS = new long[] {
+    0x80L, 0x40L, 0x20L, 0x10L, 0x08L, 0x04L, 0x02L, 0x01L
   };
 
   private final byte[] scratch;
@@ -53,48 +54,41 @@ import java.io.IOException;
    *
    * @param input The {@link ExtractorInput} from which the integer should be read.
    * @param allowEndOfInput True if encountering the end of the input having read no data is
-   *     allowed, and should result in {@code -1} being returned. False if it should be
-   *     considered an error, causing an {@link EOFException} to be thrown.
-   * @param removeLengthMask Removes the variable-length integer length mask from the value
-   * @return The read value, or -1 if {@code allowEndOfStream} is true and the end of the input was
-   *     encountered.
+   *     allowed, and should result in {@link C#RESULT_END_OF_INPUT} being returned. False if it
+   *     should be considered an error, causing an {@link EOFException} to be thrown.
+   * @param removeLengthMask Removes the variable-length integer length mask from the value.
+   * @param maximumAllowedLength Maximum allowed length of the variable integer to be read.
+   * @return The read value, or {@link C#RESULT_END_OF_INPUT} if {@code allowEndOfStream} is true
+   *     and the end of the input was encountered, or {@link C#RESULT_MAX_LENGTH_EXCEEDED} if the
+   *     length of the varint exceeded maximumAllowedLength.
    * @throws IOException If an error occurs reading from the input.
    * @throws InterruptedException If the thread is interrupted.
    */
   public long readUnsignedVarint(ExtractorInput input, boolean allowEndOfInput,
-      boolean removeLengthMask) throws IOException, InterruptedException {
+      boolean removeLengthMask, int maximumAllowedLength) throws IOException, InterruptedException {
     if (state == STATE_BEGIN_READING) {
       // Read the first byte to establish the length.
       if (!input.readFully(scratch, 0, 1, allowEndOfInput)) {
-        return -1;
+        return C.RESULT_END_OF_INPUT;
       }
       int firstByte = scratch[0] & 0xFF;
-      length = -1;
-      for (int i = 0; i < VARINT_LENGTH_MASKS.length; i++) {
-        if ((VARINT_LENGTH_MASKS[i] & firstByte) != 0) {
-          length = i + 1;
-          break;
-        }
-      }
+      length = parseUnsignedVarintLength(firstByte);
       if (length == -1) {
         throw new IllegalStateException("No valid varint length mask found");
       }
       state = STATE_READ_CONTENTS;
     }
 
+    if (length > maximumAllowedLength) {
+      state = STATE_BEGIN_READING;
+      return C.RESULT_MAX_LENGTH_EXCEEDED;
+    }
+
     // Read the remaining bytes.
     input.readFully(scratch, 1, length - 1);
 
-    // Parse the value.
-    if (removeLengthMask) {
-      scratch[0] &= ~VARINT_LENGTH_MASKS[length - 1];
-    }
-    long varint = 0;
-    for (int i = 0; i < length; i++) {
-      varint = (varint << 8) | (scratch[i] & 0xFF);
-    }
     state = STATE_BEGIN_READING;
-    return varint;
+    return assembleVarint(scratch, length, removeLengthMask);
   }
 
   /**
@@ -104,4 +98,41 @@ import java.io.IOException;
     return length;
   }
 
+  /**
+   * Parses and the length of the varint given the first byte.
+   *
+   * @param firstByte First byte of the varint.
+   * @return Length of the varint beginning with the given byte if it was valid, -1 otherwise.
+   */
+  public static int parseUnsignedVarintLength(int firstByte) {
+    int varIntLength = -1;
+    for (int i = 0; i < VARINT_LENGTH_MASKS.length; i++) {
+      if ((VARINT_LENGTH_MASKS[i] & firstByte) != 0) {
+        varIntLength = i + 1;
+        break;
+      }
+    }
+    return varIntLength;
+  }
+
+  /**
+   * Assemble a varint from the given byte array.
+   *
+   * @param varintBytes Bytes that make up the varint.
+   * @param varintLength Length of the varint to assemble.
+   * @param removeLengthMask Removes the variable-length integer length mask from the value.
+   * @return Parsed and assembled varint.
+   */
+  public static long assembleVarint(byte[] varintBytes, int varintLength,
+      boolean removeLengthMask) {
+    long varint = varintBytes[0] & 0xFFL;
+    if (removeLengthMask) {
+      varint &= ~VARINT_LENGTH_MASKS[varintLength - 1];
+    }
+    for (int i = 1; i < varintLength; i++) {
+      varint = (varint << 8) | (varintBytes[i] & 0xFFL);
+    }
+    return varint;
+  }
+
 }

library/src/main/java/com/google/android/exoplayer/extractor/webm/WebmExtractor.java

@@ -349,6 +349,10 @@ public final class WebmExtractor implements Extractor {
     }
   }
 
+  /* package */ boolean isLevel1Element(int id) {
+    return id == ID_SEGMENT_INFO || id == ID_CLUSTER || id == ID_CUES || id == ID_TRACKS;
+  }
+
   /* package */ void startMasterElement(int id, long contentPosition, long contentSize)
       throws ParserException {
     switch (id) {
@@ -641,7 +645,7 @@ public final class WebmExtractor implements Extractor {
         // differ only in the way flags are specified.
 
         if (blockState == BLOCK_STATE_START) {
-          blockTrackNumber = (int) varintReader.readUnsignedVarint(input, false, true);
+          blockTrackNumber = (int) varintReader.readUnsignedVarint(input, false, true, 8);
           blockTrackNumberLength = varintReader.getLastLength();
           blockDurationUs = UNKNOWN;
           blockState = BLOCK_STATE_HEADER;
@@ -1074,6 +1078,11 @@ public final class WebmExtractor implements Extractor {
     }
 
     @Override
+    public boolean isLevel1Element(int id) {
+      return WebmExtractor.this.isLevel1Element(id);
+    }
+
+    @Override
     public void startMasterElement(int id, long contentPosition, long contentSize)
         throws ParserException {
       WebmExtractor.this.startMasterElement(id, contentPosition, contentSize);