From 385dadcfbc9fe52c63acf3f97042a8774798fdce Mon Sep 17 00:00:00 2001
From: Nick Hill <nickhill@us.ibm.com>
Date: Mon, 27 May 2019 06:32:08 -0700
Subject: [PATCH] Fix redundant or missing checks and other inconsistencies in
 ByteBuf impls (#9119)

Motivation

There are a few minor inconsistencies / redundant operations in the
ByteBuf implementations which would be good to fix.

Modifications

- Unnecessary ByteBuffer.duplicate() performed in
CompositeByteBuf.nioBuffer(int,int)
- Add missing checkIndex(...) check to
ReadOnlyByteBufferBuf.nioBuffer(int,int)
- Remove duplicate bounds check in
ReadOnlyByteBufferBuf.getBytes(int,byte[],int,int)
- Omit redundant bounds check in
UnpooledHeapByteBuf.getBytes(int,ByteBuffer)

Result

More consistency and slightly less overhead
---
 .../main/java/io/netty/buffer/AbstractByteBuf.java    | 11 ++++++-----
 .../main/java/io/netty/buffer/CompositeByteBuf.java   |  2 +-
 .../java/io/netty/buffer/ReadOnlyByteBufferBuf.java   |  6 +-----
 .../java/io/netty/buffer/UnpooledHeapByteBuf.java     |  2 +-
 4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
index d3fd389dae..fe880c7efd 100644
--- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -1389,30 +1389,31 @@ public abstract class AbstractByteBuf extends ByteBuf {
         checkIndex0(index, fieldLength);
     }
 
-    private static void checkRangeBounds(final int index, final int fieldLength, final int capacity) {
+    private static void checkRangeBounds(final String indexName, final int index,
+            final int fieldLength, final int capacity) {
         if (isOutOfBounds(index, fieldLength, capacity)) {
             throw new IndexOutOfBoundsException(String.format(
-                    "index: %d, length: %d (expected: range(0, %d))", index, fieldLength, capacity));
+                    "%s: %d, length: %d (expected: range(0, %d))", indexName, index, fieldLength, capacity));
         }
     }
 
     final void checkIndex0(int index, int fieldLength) {
         if (checkBounds) {
-            checkRangeBounds(index, fieldLength, capacity());
+            checkRangeBounds("index", index, fieldLength, capacity());
         }
     }
 
     protected final void checkSrcIndex(int index, int length, int srcIndex, int srcCapacity) {
         checkIndex(index, length);
         if (checkBounds) {
-            checkRangeBounds(srcIndex, length, srcCapacity);
+            checkRangeBounds("srcIndex", srcIndex, length, srcCapacity);
         }
     }
 
     protected final void checkDstIndex(int index, int length, int dstIndex, int dstCapacity) {
         checkIndex(index, length);
         if (checkBounds) {
-            checkRangeBounds(dstIndex, length, dstCapacity);
+            checkRangeBounds("dstIndex", dstIndex, length, dstCapacity);
         }
     }
 
diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
index c157ab2489..33055ff160 100644
--- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -1630,7 +1630,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
         ByteBuffer[] buffers = nioBuffers(index, length);
 
         if (buffers.length == 1) {
-            return buffers[0].duplicate();
+            return buffers[0];
         }
 
         ByteBuffer merged = ByteBuffer.allocate(length).order(order());
diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java
index a8e0b2fb81..b5271d8d8e 100644
--- a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java
@@ -195,11 +195,6 @@ class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf {
     public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) {
         checkDstIndex(index, length, dstIndex, dst.length);
 
-        if (dstIndex < 0 || dstIndex > dst.length - length) {
-            throw new IndexOutOfBoundsException(String.format(
-                    "dstIndex: %d, length: %d (expected: range(0, %d))", dstIndex, length, dst.length));
-        }
-
         ByteBuffer tmpBuf = internalNioBuffer();
         tmpBuf.clear().position(index).limit(index + length);
         tmpBuf.get(dst, dstIndex, length);
@@ -449,6 +444,7 @@ class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public ByteBuffer nioBuffer(int index, int length) {
+        checkIndex(index, length);
         return (ByteBuffer) buffer.duplicate().position(index).limit(index + length);
     }
 
diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java
index f37ceb0559..0e1ff3488d 100644
--- a/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java
@@ -194,7 +194,7 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public ByteBuf getBytes(int index, ByteBuffer dst) {
-        checkIndex(index, dst.remaining());
+        ensureAccessible();
         dst.put(array, index, dst.remaining());
         return this;
     }