Skip to content

Commit

Permalink
Fix seeking of empty chunkers.
Browse files Browse the repository at this point in the history
Empty chunkers are strange in that they emit one empty chunk and then end. This change makes them properly resetable. Previously, reset() on a empty chunker could result in a NullPointerException.

Also, it's important to call close() on the underlying data stream even if it's empty.

Closes #17797.

PiperOrigin-RevId: 517119206
Change-Id: Iff7908d6cd0633aa2a355ea89f8e647a9fefffcd
  • Loading branch information
benjaminp authored and copybara-github committed Mar 16, 2023
1 parent 3b475b3 commit cfef67d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ public void reset() throws IOException {
public void seek(long toOffset) throws IOException {
// For compressed stream, we need to reinitialize the stream since the offset refers to the
// uncompressed form.
if (initialized && toOffset >= offset && !compressed) {
if (initialized && size > 0 && toOffset >= offset && !compressed) {
ByteStreams.skipFully(data, toOffset - offset);
offset = toOffset;
} else {
reset();
initialize(toOffset);
}
if (data.finished()) {
if (size > 0 && data.finished()) {
close();
}
}
Expand Down Expand Up @@ -216,7 +216,7 @@ public Chunk next() throws IOException {
maybeInitialize();

if (size == 0) {
data = null;
close();
return emptyChunk;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,17 @@ public void nextShouldThrowIfNoMoreData() throws IOException {

@Test
public void emptyData() throws Exception {
byte[] data = new byte[0];
Chunker chunker = Chunker.builder().setInput(data).build();
var inp =
new ByteArrayInputStream(new byte[0]) {
private boolean closed;

@Override
public void close() throws IOException {
closed = true;
super.close();
}
};
Chunker chunker = Chunker.builder().setInput(0, inp).build();

assertThat(chunker.hasNext()).isTrue();

Expand All @@ -96,6 +105,7 @@ public void emptyData() throws Exception {
assertThat(next.getOffset()).isEqualTo(0);

assertThat(chunker.hasNext()).isFalse();
assertThat(inp.closed).isTrue();

assertThrows(NoSuchElementException.class, () -> chunker.next());
}
Expand Down Expand Up @@ -193,6 +203,21 @@ public void seekForwards() throws IOException {
assertThat(chunker.hasNext()).isFalse();
}

@Test
public void seekEmptyData() throws IOException {
var chunker = Chunker.builder().setInput(new byte[0]).build();
for (var i = 0; i < 2; i++) {
chunker.seek(0);
var next = chunker.next();
assertThat(next).isNotNull();
assertThat(next.getData()).isEmpty();
assertThat(next.getOffset()).isEqualTo(0);

assertThat(chunker.hasNext()).isFalse();
assertThrows(NoSuchElementException.class, chunker::next);
}
}

@Test
public void testSingleChunkCompressed() throws IOException {
byte[] data = {72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33};
Expand Down

0 comments on commit cfef67d

Please sign in to comment.