Skip to content

Commit

Permalink
Prevent NPE on backwards seek on Chunker
Browse files Browse the repository at this point in the history
Ensure that a reset() is not immediately followed by a data member
access with a call to maybeInitialize(). Practically, this can occur if
an offset has progressed locally, but the remote indicates a non-0
committed size, and would always have thrown an NPE on a requisite seek,
proven with the accompanied test case.

Closes #10566.

PiperOrigin-RevId: 289615198
  • Loading branch information
George Gensure authored and copybara-github committed Jan 14, 2020
1 parent 872c60f commit 9a823d9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
10 changes: 3 additions & 7 deletions src/main/java/com/google/devtools/build/lib/remote/Chunker.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,14 @@ public void reset() throws IOException {
/**
* Seek to an offset, if necessary resetting or initializing
*
* <p>Closes any open resources (file handles, ...).
* <p>May close open resources in order to seek to an earlier offset.
*/
public void seek(long toOffset) throws IOException {
maybeInitialize();
if (toOffset < offset) {
reset();
if (toOffset != 0) {
data.skip(toOffset);
}
} else if (offset != toOffset) {
data.skip(toOffset - offset);
}
maybeInitialize();
ByteStreams.skipFully(data, toOffset - offset);
offset = toOffset;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,20 @@ public void seekAfterReset() throws IOException {
assertThat(next.getData()).hasSize(8);
}

@Test
public void seekBackwards() throws IOException {
byte[] data = new byte[10];
Chunker chunker = Chunker.builder().setInput(data).setChunkSize(10).build();

chunker.seek(4);
chunker.seek(2);

Chunk next = chunker.next();
assertThat(next).isNotNull();
assertThat(next.getOffset()).isEqualTo(2);
assertThat(next.getData()).hasSize(8);
}

private void assertNextEquals(Chunker chunker, byte... data) throws IOException {
assertThat(chunker.hasNext()).isTrue();
ByteString next = chunker.next().getData();
Expand Down

0 comments on commit 9a823d9

Please sign in to comment.