Fix CompositeBytesReference#slice to not throw AIOOBE with legal offsets. (#35955)
CompositeBytesReference#slice has two bugs: - One that makes it fail if the reference is empty and an empty slice is created, this is #35950 and is fixed by special-casing empty-slices. - One performance bug that makes it always create a composite slice when creating a slice that ends on a boundary, this is fixed by computing `limit` as the index of the sub reference that holds the last element rather than the next element after the slice. Closes #35950
This commit is contained in:
parent
54cf1f9d74
commit
fa3d365ee8
|
@ -22,6 +22,7 @@ package org.elasticsearch.common.bytes;
|
|||
import org.apache.lucene.util.BytesRef;
|
||||
import org.apache.lucene.util.BytesRefBuilder;
|
||||
import org.apache.lucene.util.BytesRefIterator;
|
||||
import org.apache.lucene.util.FutureObjects;
|
||||
import org.apache.lucene.util.RamUsageEstimator;
|
||||
|
||||
import java.io.IOException;
|
||||
|
@ -77,10 +78,16 @@ public final class CompositeBytesReference extends BytesReference {
|
|||
|
||||
@Override
|
||||
public BytesReference slice(int from, int length) {
|
||||
FutureObjects.checkFromIndexSize(from, length, this.length);
|
||||
|
||||
if (length == 0) {
|
||||
return BytesArray.EMPTY;
|
||||
}
|
||||
|
||||
// for slices we only need to find the start and the end reference
|
||||
// adjust them and pass on the references in between as they are fully contained
|
||||
final int to = from + length;
|
||||
final int limit = getOffsetIndex(from + length);
|
||||
final int limit = getOffsetIndex(to - 1);
|
||||
final int start = getOffsetIndex(from);
|
||||
final BytesReference[] inSlice = new BytesReference[1 + (limit - start)];
|
||||
for (int i = 0, j = start; i < inSlice.length; i++) {
|
||||
|
|
|
@ -23,6 +23,7 @@ import org.apache.lucene.util.BytesRefBuilder;
|
|||
import org.apache.lucene.util.BytesRefIterator;
|
||||
import org.elasticsearch.common.io.stream.BytesStreamOutput;
|
||||
import org.elasticsearch.common.io.stream.ReleasableBytesStreamOutput;
|
||||
import org.hamcrest.Matchers;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
|
@ -113,4 +114,18 @@ public class CompositeBytesReferenceTests extends AbstractBytesReferenceTestCase
|
|||
public void testSliceToBytesRef() throws IOException {
|
||||
// CompositeBytesReference shifts offsets
|
||||
}
|
||||
|
||||
public void testSliceIsNotCompositeIfMatchesSingleSubSlice() {
|
||||
CompositeBytesReference bytesRef = new CompositeBytesReference(
|
||||
new BytesArray(new byte[12]),
|
||||
new BytesArray(new byte[15]),
|
||||
new BytesArray(new byte[13]));
|
||||
|
||||
// Slices that cross boundaries are composite too
|
||||
assertThat(bytesRef.slice(5, 8), Matchers.instanceOf(CompositeBytesReference.class));
|
||||
|
||||
// But not slices that cover a single sub reference
|
||||
assertThat(bytesRef.slice(13, 10), Matchers.not(Matchers.instanceOf(CompositeBytesReference.class))); // strictly within sub
|
||||
assertThat(bytesRef.slice(12, 15), Matchers.not(Matchers.instanceOf(CompositeBytesReference.class))); // equal to sub
|
||||
}
|
||||
}
|
||||
|
|
|
@ -68,7 +68,7 @@ public abstract class AbstractBytesReferenceTestCase extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testSlice() throws IOException {
|
||||
int length = randomInt(PAGE_SIZE * 3);
|
||||
for (int length : new int[] {0, 1, randomIntBetween(2, PAGE_SIZE), randomIntBetween(PAGE_SIZE + 1, 3 * PAGE_SIZE)}) {
|
||||
BytesReference pbr = newBytesReference(length);
|
||||
int sliceOffset = randomIntBetween(0, length / 2);
|
||||
int sliceLength = Math.max(0, length - sliceOffset - 1);
|
||||
|
@ -84,6 +84,7 @@ public abstract class AbstractBytesReferenceTestCase extends ESTestCase {
|
|||
assertEquals(sliceLength, singlePageOrNull.length);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public void testStreamInput() throws IOException {
|
||||
int length = randomIntBetween(10, scaledRandomIntBetween(PAGE_SIZE * 2, PAGE_SIZE * 20));
|
||||
|
|
Loading…
Reference in New Issue