Commit 46598a46 by ibaker Committed by Ian Baker

Audit usages of ParsableByteArray#reset(int)

This method should be assumed to clear the data of the underlying array
(it will do this if the new limit > data.length).

This means it should only be called (directly) before writing into the
backing array.

It shouldn't be used as a shorthand for position=0, limit=x - those
should be two explicit method calls.

Most of these changes are no-ops, but they make the code more correct.

The TS SectionReader can't be easily changed to be 'safe', because it
relies on sectionData maintaining state between iterations of the while
loop. Instead I've added comments justifying the existing code.

PiperOrigin-RevId: 344515340
parent e66f0032
......@@ -68,8 +68,8 @@ public final class ParsableByteArray {
}
/**
* Resets the position to zero and the limit to the specified value. If the limit exceeds the
* capacity, {@code data} is replaced with a new array of sufficient size.
* Resets the position to zero and the limit to the specified value. This might replace or wipe
* the {@link #getData() underlying array}, potentially invalidating any local references.
*
* @param limit The limit to set.
*/
......
......@@ -177,7 +177,8 @@ public final class PgsDecoder extends SimpleSubtitleDecoder {
}
bitmapWidth = buffer.readUnsignedShort();
bitmapHeight = buffer.readUnsignedShort();
bitmapData.reset(totalLength - 4);
bitmapData.setPosition(0);
bitmapData.setLimit(totalLength - 4);
sectionLength -= 7;
}
......
......@@ -303,13 +303,11 @@ public final class FlacExtractor implements Extractor {
if (buffer.bytesLeft() < FlacConstants.MAX_FRAME_HEADER_SIZE) {
// The next frame header may not fit in the rest of the buffer, so put the trailing bytes at
// the start of the buffer, and reset the position and limit.
int bytesLeft = buffer.bytesLeft();
System.arraycopy(
buffer.getData(),
buffer.getPosition(),
buffer.getData(),
/* destPos= */ 0,
buffer.bytesLeft());
buffer.reset(buffer.bytesLeft());
buffer.getData(), buffer.getPosition(), buffer.getData(), /* destPos= */ 0, bytesLeft);
buffer.setPosition(0);
buffer.setLimit(bytesLeft);
}
return Extractor.RESULT_CONTINUE;
......
......@@ -1575,7 +1575,8 @@ public class MatroskaExtractor implements Extractor {
System.arraycopy(samplePrefix, 0, subtitleSample.getData(), 0, samplePrefix.length);
}
input.readFully(subtitleSample.getData(), samplePrefix.length, size);
subtitleSample.reset(sizeWithPrefix);
subtitleSample.setPosition(0);
subtitleSample.setLimit(sizeWithPrefix);
// Defer writing the data to the track output. We need to modify the sample data by setting
// the correct end timecode, which we might not have yet.
}
......
......@@ -104,9 +104,10 @@ import java.io.IOException;
*/
public boolean skipToNextPage(ExtractorInput input, long limit) throws IOException {
Assertions.checkArgument(input.getPosition() == input.getPeekPosition());
scratch.reset(/* limit= */ CAPTURE_PATTERN_SIZE);
while ((limit == C.POSITION_UNSET || input.getPosition() + CAPTURE_PATTERN_SIZE < limit)
&& peekSafely(input, scratch.getData(), 0, CAPTURE_PATTERN_SIZE, /* quiet= */ true)) {
scratch.reset(/* limit= */ CAPTURE_PATTERN_SIZE);
scratch.setPosition(0);
if (scratch.readUnsignedInt() == CAPTURE_PATTERN) {
input.resetPeekPosition();
return true;
......
......@@ -23,6 +23,7 @@ import com.google.android.exoplayer2.extractor.ExtractorOutput;
import com.google.android.exoplayer2.util.ParsableByteArray;
import com.google.android.exoplayer2.util.TimestampAdjuster;
import com.google.android.exoplayer2.util.Util;
import java.util.Arrays;
/**
* Reads section data packets and feeds the whole sections to a given {@link SectionPayloadReader}.
......@@ -91,10 +92,13 @@ public final class SectionReader implements TsPayloadReader {
}
}
int headerBytesToRead = min(data.bytesLeft(), SECTION_HEADER_LENGTH - bytesRead);
// sectionData is guaranteed to have enough space because it's initialized with a 32-element
// backing array and headerBytesToRead is at most 3.
data.readBytes(sectionData.getData(), bytesRead, headerBytesToRead);
bytesRead += headerBytesToRead;
if (bytesRead == SECTION_HEADER_LENGTH) {
sectionData.reset(SECTION_HEADER_LENGTH);
sectionData.setPosition(0);
sectionData.setLimit(SECTION_HEADER_LENGTH);
sectionData.skipBytes(1); // Skip table id (8).
int secondHeaderByte = sectionData.readUnsignedByte();
int thirdHeaderByte = sectionData.readUnsignedByte();
......@@ -104,13 +108,17 @@ public final class SectionReader implements TsPayloadReader {
if (sectionData.capacity() < totalSectionLength) {
// Ensure there is enough space to keep the whole section.
byte[] bytes = sectionData.getData();
sectionData.reset(min(MAX_SECTION_LENGTH, max(totalSectionLength, bytes.length * 2)));
System.arraycopy(bytes, 0, sectionData.getData(), 0, SECTION_HEADER_LENGTH);
int limit = min(MAX_SECTION_LENGTH, max(totalSectionLength, bytes.length * 2));
if (limit > bytes.length) {
bytes = Arrays.copyOf(sectionData.getData(), limit);
}
sectionData.reset(bytes, limit);
}
}
} else {
// Reading the body.
int bodyBytesToRead = min(data.bytesLeft(), totalSectionLength - bytesRead);
// sectionData has been sized large enough for totalSectionLength when reading the header.
data.readBytes(sectionData.getData(), bytesRead, bodyBytesToRead);
bytesRead += bodyBytesToRead;
if (bytesRead == totalSectionLength) {
......@@ -121,11 +129,12 @@ public final class SectionReader implements TsPayloadReader {
waitingForPayloadStart = true;
return;
}
sectionData.reset(totalSectionLength - 4); // Exclude the CRC_32 field.
sectionData.setLimit(totalSectionLength - 4); // Exclude the CRC_32 field.
} else {
// This is a private section with private defined syntax.
sectionData.reset(totalSectionLength);
sectionData.setLimit(totalSectionLength);
}
sectionData.setPosition(0);
reader.consume(sectionData);
bytesRead = 0;
}
......
......@@ -472,7 +472,8 @@ public final class WavExtractor implements Extractor {
}
}
int decodedDataSize = numOutputFramesToBytes(framesPerBlock * blockCount);
output.reset(decodedDataSize);
output.setPosition(0);
output.setLimit(decodedDataSize);
}
private void decodeBlockForChannel(
......
......@@ -473,12 +473,12 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
private long peekId3PrivTimestamp(ExtractorInput input) throws IOException {
input.resetPeekPosition();
try {
scratchId3Data.reset(Id3Decoder.ID3_HEADER_LENGTH);
input.peekFully(scratchId3Data.getData(), 0, Id3Decoder.ID3_HEADER_LENGTH);
} catch (EOFException e) {
// The input isn't long enough for there to be any ID3 data.
return C.TIME_UNSET;
}
scratchId3Data.reset(Id3Decoder.ID3_HEADER_LENGTH);
int id = scratchId3Data.readUnsignedInt24();
if (id != Id3Decoder.ID3_TAG) {
return C.TIME_UNSET;
......@@ -504,7 +504,8 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
if (PRIV_TIMESTAMP_FRAME_OWNER.equals(privFrame.owner)) {
System.arraycopy(
privFrame.privateData, 0, scratchId3Data.getData(), 0, 8 /* timestamp size */);
scratchId3Data.reset(8);
scratchId3Data.setPosition(0);
scratchId3Data.setLimit(8);
// The top 31 bits should be zeros, but explicitly zero them to wrap in the case that the
// streaming provider forgot. See: https://github.com/google/ExoPlayer/pull/3495.
return scratchId3Data.readLong() & 0x1FFFFFFFFL;
......
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