Simplify ByteBufferUtils#readBytes(), document side-effects, add te…

…sts (Graylog2#4538)

The `ByteBufferUtils#readBytes(ByteBuf, int, int)` method failed for non-array-backed byte buffers
whose reader index didn't start at index 0 and failed to mention side effects (e. g. consumption of
the given `ByteBuffer`) in the Javadocs.

Additionally, the "optimization" in `ByteBufferUtils#readBytes(ByteBuf, int, int)` was unnecessary
because the relevant `ByteBuffer` implementations are already using the backing array if available.

graylog2-server/src/main/java/org/graylog2/shared/utilities/ByteBufferUtils.java

@@ -21,24 +21,21 @@
 public abstract class ByteBufferUtils {
     /**
      * Read the given byte buffer into a byte array
+     *
+     * This will <em>consume</em> the given {@link ByteBuffer}.
      */
     public static byte[] readBytes(ByteBuffer buffer) {
-        return readBytes(buffer, 0, buffer.limit());
+        return readBytes(buffer, 0, buffer.remaining());
     }
 
     /**
      * Read a byte array from the given offset and size in the buffer
+     *
+     * This will <em>consume</em> the given {@link ByteBuffer}.
      */
     public static byte[] readBytes(ByteBuffer buffer, int offset, int size) {
         final byte[] dest = new byte[size];
-        if (buffer.hasArray()) {
-            System.arraycopy(buffer.array(), buffer.arrayOffset() + offset, dest, 0, size);
-        } else {
-            buffer.mark();
-            buffer.get(dest);
-            buffer.reset();
-        }
-
+        buffer.get(dest, offset, size);
         return dest;
     }
 }

graylog2-server/src/test/java/org/graylog2/shared/utilities/ByteBufferUtilsTest.java

@@ -0,0 +1,54 @@
+/**
+ * This file is part of Graylog.
+ *
+ * Graylog is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Graylog is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Graylog.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.graylog2.shared.utilities;
+
+import org.junit.Test;
+
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class ByteBufferUtilsTest {
+    @Test
+    public void readBytesFromArrayBackedByteBuffer() {
+        final byte[] bytes = "FOOBAR".getBytes(StandardCharsets.US_ASCII);
+        final ByteBuffer buffer1 = ByteBuffer.wrap(bytes);
+        final ByteBuffer buffer2 = ByteBuffer.wrap(bytes);
+        final byte[] readBytesComplete = ByteBufferUtils.readBytes(buffer1);
+        final byte[] readBytesPartial = ByteBufferUtils.readBytes(buffer2, 0, 3);
+
+        assertThat(readBytesComplete).isEqualTo(bytes);
+        assertThat(readBytesPartial).isEqualTo(Arrays.copyOf(bytes, 3));
+    }
+
+    @Test
+    public void readBytesFromNonArrayBackedByteBuffer() {
+        final byte[] bytes = "FOOBAR".getBytes(StandardCharsets.US_ASCII);
+        final ByteBuffer buffer1 = ByteBuffer.allocateDirect(1024);
+        buffer1.put(bytes).flip();
+        final ByteBuffer buffer2 = ByteBuffer.allocateDirect(1024);
+        buffer2.put(bytes).flip();
+
+        final byte[] readBytesComplete = ByteBufferUtils.readBytes(buffer1);
+        final byte[] readBytesPartial = ByteBufferUtils.readBytes(buffer2, 0, 3);
+
+        assertThat(readBytesComplete).isEqualTo(bytes);
+        assertThat(readBytesPartial).isEqualTo(Arrays.copyOf(bytes, 3));
+    }
+}