Browse Source

Fix BytesRefArray on append empty BytesRef (#91364)

Appending an empty BytesRef to BytesRefArray can fail with NPE or IOE 
when the current page is fully used. This PR skips writing the empty
input to the backing bytes array.
Nhat Nguyen 3 years ago
parent
commit
c4771edf32

+ 5 - 0
docs/changelog/91364.yaml

@@ -0,0 +1,5 @@
+pr: 91364
+summary: Fix `BytesRefArray` on append empty `BytesRef`
+area: Infra/Core
+type: bug
+issues: []

+ 4 - 2
server/src/main/java/org/elasticsearch/common/util/BytesRefArray.java

@@ -87,11 +87,13 @@ public class BytesRefArray implements Accountable, Releasable, Writeable {
     public void append(BytesRef key) {
         assert startOffsets != null : "using BytesRefArray after ownership taken";
         final long startOffset = startOffsets.get(size);
-        bytes = bigArrays.grow(bytes, startOffset + key.length);
-        bytes.set(startOffset, key.bytes, key.offset, key.length);
         startOffsets = bigArrays.grow(startOffsets, size + 2);
         startOffsets.set(size + 1, startOffset + key.length);
         ++size;
+        if (key.length > 0) {
+            bytes = bigArrays.grow(bytes, startOffset + key.length);
+            bytes.set(startOffset, key.bytes, key.offset, key.length);
+        }
     }
 
     /**

+ 44 - 0
server/src/test/java/org/elasticsearch/common/util/BytesRefArrayTests.java

@@ -17,6 +17,8 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class BytesRefArrayTests extends ESTestCase {
 
     public static BytesRefArray randomArray() {
@@ -70,6 +72,48 @@ public class BytesRefArrayTests extends ESTestCase {
         newOwnerOfArray.close();
     }
 
+    public void testLookup() throws IOException {
+        int size = randomIntBetween(0, 16 * 1024);
+        BytesRefArray array = new BytesRefArray(randomIntBetween(0, size), mockBigArrays());
+        try {
+            BytesRef[] values = new BytesRef[size];
+            for (int i = 0; i < size; i++) {
+                BytesRef bytesRef = new BytesRef(randomByteArrayOfLength(between(1, 20)));
+                if (bytesRef.length > 0 && randomBoolean()) {
+                    bytesRef.offset = randomIntBetween(0, bytesRef.length - 1);
+                    bytesRef.length = randomIntBetween(0, bytesRef.length - bytesRef.offset);
+                }
+                values[i] = bytesRef;
+                if (randomBoolean()) {
+                    bytesRef = BytesRef.deepCopyOf(bytesRef);
+                }
+                array.append(bytesRef);
+            }
+            int copies = randomIntBetween(0, 3);
+            for (int i = 0; i < copies; i++) {
+                BytesRefArray inArray = array;
+                array = copyInstance(
+                    inArray,
+                    writableRegistry(),
+                    (out, value) -> value.writeTo(out),
+                    in -> new BytesRefArray(in, mockBigArrays()),
+                    Version.CURRENT
+                );
+                assertEquality(inArray, array);
+                inArray.close();
+            }
+            assertThat(array.size(), equalTo((long) size));
+            BytesRef bytes = new BytesRef();
+            for (int i = 0; i < size; i++) {
+                int pos = randomIntBetween(0, size - 1);
+                bytes = array.get(pos, bytes);
+                assertThat(bytes, equalTo(values[pos]));
+            }
+        } finally {
+            array.close();
+        }
+    }
+
     private static BigArrays mockBigArrays() {
         return new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
     }