Fix ByteBufUtil.getBytes() incorrectly sharing the array in some cases (#10529)

Motivation:

If ByteBufUtil.getBytes() is called with copy=false, it does not
correctly check that the underlying array can be shared in some cases.

In particular:

* It does not check that the arrayOffset() is zero. This causes it to
  incorrectly return the underlying array if the other conditions are
  met. The returned array will be longer than requested, with additional
  unwanted bytes at its start.

* It assumes that the capacity() of the ByteBuf is equal to the backing
  array length. This is not true for some types of ByteBuf, such as
  PooledHeapByteBuf. This causes it to incorrectly return the underlying
  array if the other conditions are met. The returned array will be
  longer than requested, with additional unwanted bytes at its end.

Modifications:

This commit fixes the two bugs by:

* Checking that the arrayOffset() is zero before returning the
  underlying array.

* Comparing the requested length to the underlying array's length,
  rather than the ByteBuf's capacity, before returning the underlying
  array.

This commit also adds a series of test cases for ByteBufUtil.getBytes().

Result:

ByteBufUtil.getBytes() now correctly checks whether the underlying array
can be shared or not.

The test cases will ensure the bug is not reintroduced in the future.

buffer/src/main/java/io/netty/buffer/ByteBufUtil.java

@@ -905,17 +905,18 @@ public final class ByteBufUtil {
         }
 
         if (buf.hasArray()) {
-            if (copy || start != 0 || length != capacity) {
-                int baseOffset = buf.arrayOffset() + start;
-                return Arrays.copyOfRange(buf.array(), baseOffset, baseOffset + length);
+            int baseOffset = buf.arrayOffset() + start;
+            byte[] bytes = buf.array();
+            if (copy || baseOffset != 0 || length != bytes.length) {
+                return Arrays.copyOfRange(bytes, baseOffset, baseOffset + length);
             } else {
-                return buf.array();
+                return bytes;
             }
         }
 
-        byte[] v = PlatformDependent.allocateUninitializedArray(length);
-        buf.getBytes(start, v);
-        return v;
+        byte[] bytes = PlatformDependent.allocateUninitializedArray(length);
+        buf.getBytes(start, bytes);
+        return bytes;
     }
 
     /**

buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java

@@ -29,11 +29,16 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static io.netty.buffer.Unpooled.unreleasableBuffer;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeThat;
+import static org.junit.Assume.assumeTrue;
 
 public class ByteBufUtilTest {
     @Test
@@ -797,4 +802,84 @@ public class ByteBufUtilTest {
             buffer.release();
         }
     }
+
+    @Test
+    public void testGetBytesHeap() {
+        final ByteBuf buf = Unpooled.buffer(4);
+        try {
+            assumeTrue(buf.hasArray());
+            checkGetBytes(buf);
+        } finally {
+            buf.release();
+        }
+    }
+
+    @Test
+    public void testGetBytesDirect() {
+        final ByteBuf buf = Unpooled.directBuffer(4);
+        try {
+            assumeFalse(buf.hasArray());
+            checkGetBytes(buf);
+        } finally {
+            buf.release();
+        }
+    }
+
+    @Test
+    public void testGetBytesHeapWithNonZeroArrayOffset() {
+        final ByteBuf buf = Unpooled.buffer(5);
+        try {
+            buf.setByte(0, 0x05);
+
+            final ByteBuf slice = buf.slice(1, 4);
+            slice.writerIndex(0);
+
+            assumeTrue(slice.hasArray());
+            assumeThat(slice.arrayOffset(), is(1));
+            assumeThat(slice.array().length, is(buf.capacity()));
+
+            checkGetBytes(slice);
+        } finally {
+            buf.release();
+        }
+    }
+
+    @Test
+    public void testGetBytesHeapWithArrayLengthGreaterThanCapacity() {
+        final ByteBuf buf = Unpooled.buffer(5);
+        try {
+            buf.setByte(4, 0x05);
+
+            final ByteBuf slice = buf.slice(0, 4);
+            slice.writerIndex(0);
+
+            assumeTrue(slice.hasArray());
+            assumeThat(slice.arrayOffset(), is(0));
+            assumeThat(slice.array().length, greaterThan(slice.capacity()));
+
+            checkGetBytes(slice);
+        } finally {
+            buf.release();
+        }
+    }
+
+    private static void checkGetBytes(final ByteBuf buf) {
+        buf.writeInt(0x01020304);
+
+        byte[] expected = { 0x01, 0x02, 0x03, 0x04 };
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf));
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 0, buf.readableBytes(), false));
+
+        expected = new byte[] { 0x01, 0x02, 0x03 };
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 0, 3));
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 0, 3, false));
+
+        expected = new byte[] { 0x02, 0x03, 0x04 };
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 1, 3));
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 1, 3, false));
+
+        expected = new byte[] { 0x02, 0x03 };
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 1, 2));
+        assertArrayEquals(expected, ByteBufUtil.getBytes(buf, 1, 2, false));
+    }
 }