Repository: netty/netty
Issue: IndexOutOfBoundsException from CompositeByteBuf.forEachByteDesc
Labels: defect

After updating from 4.1.4.Final to 4.1.5.Final I have failing tests because of the following:

``` java
java.lang.IndexOutOfBoundsException: index: 21, length: 1 (expected: range(0, 21))
    at io.netty.buffer.AbstractByteBuf.checkIndex0(AbstractByteBuf.java:1359)
    at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1354)
    at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1349)
    at io.netty.buffer.CompositeByteBuf.findComponent(CompositeByteBuf.java:1407)
    at io.netty.buffer.CompositeByteBuf._getByte(CompositeByteBuf.java:751)
    at io.netty.buffer.AbstractByteBuf.forEachByteDesc0(AbstractByteBuf.java:1297)
    at io.netty.buffer.AbstractByteBuf.forEachByteDesc(AbstractByteBuf.java:1277)
```

This seems to be specific to CompositeByteBuf:

``` java

// Doesn't throw
Unpooled.copiedBuffer("test", StandardCharsets.UTF_8)
    .forEachByteDesc(ByteProcessor.FIND_CR);

// Throws (forEachByteDesc)
Unpooled.compositeBuffer()
    .addComponent(true, Unpooled.copiedBuffer("test", StandardCharsets.UTF_8))
    .forEachByteDesc(ByteProcessor.FIND_CR);

// Doesn't throw (forEachByte)
Unpooled.compositeBuffer()
    .addComponent(true, Unpooled.copiedBuffer("test", StandardCharsets.UTF_8))
    .forEachByte(ByteProcessor.FIND_CR);
```


Issue comments:
Will have a look

> Am 30.08.2016 um 18:22 schrieb Nick Pordash notifications@github.com:
> 
> After updating from 4.1.4.Final to 4.1.5.Final I have failing tests because of the following:
> 
> java.lang.IndexOutOfBoundsException: index: 21, length: 1 (expected: range(0, 21))
>     at io.netty.buffer.AbstractByteBuf.checkIndex0(AbstractByteBuf.java:1359)
>     at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1354)
>     at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1349)
>     at io.netty.buffer.CompositeByteBuf.findComponent(CompositeByteBuf.java:1407)
>     at io.netty.buffer.CompositeByteBuf._getByte(CompositeByteBuf.java:751)
>     at io.netty.buffer.AbstractByteBuf.forEachByteDesc0(AbstractByteBuf.java:1297)
>     at io.netty.buffer.AbstractByteBuf.forEachByteDesc(AbstractByteBuf.java:1277)
> This seems to be specific to CompositeByteBuf:
> 
> // Doesn't throw
> Unpooled.copiedBuffer("test", StandardCharsets.UTF_8)
>     .forEachByteDesc(ByteProcessor.FIND_CR);
> 
> // Throws (forEachByteDesc)
> Unpooled.compositeBuffer()
>     .addComponent(true, Unpooled.copiedBuffer("test", StandardCharsets.UTF_8))
>     .forEachByteDesc(ByteProcessor.FIND_CR);
> 
> // Doesn't throw (forEachByte)
> Unpooled.compositeBuffer()
>     .addComponent(true, Unpooled.copiedBuffer("test", StandardCharsets.UTF_8))
>     .forEachByte(ByteProcessor.FIND_CR);
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.

@npordash I think I have an idea... stay tuned

thanks for the repro @npordash 

See https://github.com/netty/netty/pull/5783

Fixed by https://github.com/netty/netty/pull/5783


Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
index 3b9d2b1137f..5c8c837b024 100644
--- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -1274,7 +1274,7 @@ private int forEachByteAsc0(int start, int end, ByteProcessor processor) throws
     public int forEachByteDesc(ByteProcessor processor) {
         ensureAccessible();
         try {
-            return forEachByteDesc0(writerIndex, readerIndex, processor);
+            return forEachByteDesc0(writerIndex - 1, readerIndex, processor);
         } catch (Exception e) {
             PlatformDependent.throwException(e);
             return -1;


Test patch:
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
index f4e6b90c799..53ec4b11560 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
@@ -3203,6 +3203,52 @@ public void testReadBytes() {
         assertEquals(0, buffer2.refCnt());
     }
 
+    @Test
+    public void testForEachByteDesc2() {
+        byte[] expected = {1, 2, 3, 4};
+        ByteBuf buf = newBuffer(expected.length);
+        try {
+            buf.writeBytes(expected);
+            final byte[] bytes = new byte[expected.length];
+            int i = buf.forEachByteDesc(new ByteProcessor() {
+                private int index = bytes.length - 1;
+
+                @Override
+                public boolean process(byte value) throws Exception {
+                    bytes[index--] = value;
+                    return true;
+                }
+            });
+            assertEquals(-1, i);
+            assertArrayEquals(expected, bytes);
+        } finally {
+            buf.release();
+        }
+    }
+
+    @Test
+    public void testForEachByte2() {
+        byte[] expected = {1, 2, 3, 4};
+        ByteBuf buf = newBuffer(expected.length);
+        try {
+            buf.writeBytes(expected);
+            final byte[] bytes = new byte[expected.length];
+            int i = buf.forEachByte(new ByteProcessor() {
+                private int index;
+
+                @Override
+                public boolean process(byte value) throws Exception {
+                    bytes[index++] = value;
+                    return true;
+                }
+            });
+            assertEquals(-1, i);
+            assertArrayEquals(expected, bytes);
+        } finally {
+            buf.release();
+        }
+    }
+
     private static void assertTrueAndRelease(ByteBuf buf, boolean actual) {
         try {
             assertTrue(actual);
diff --git a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java
index 1ab56d49043..f126138233a 100644
--- a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java
@@ -126,6 +126,18 @@ public void testReadBytes() {
         // ignore for SlicedByteBuf
     }
 
+    @Test
+    @Override
+    public void testForEachByteDesc2() {
+        // Ignore for SlicedByteBuf
+    }
+
+    @Test
+    @Override
+    public void testForEachByte2() {
+        // Ignore for SlicedByteBuf
+    }
+
     @Test(expected = UnsupportedOperationException.class)
     @Override
     public void testDuplicateCapacityChange() {
diff --git a/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java
index 645074b3842..81480afb19c 100644
--- a/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java
@@ -123,4 +123,16 @@ public void testRetainedDuplicateCapacityChange() {
     public void testLittleEndianWithExpand() {
         super.testLittleEndianWithExpand();
     }
+
+    @Test
+    @Override
+    public void testForEachByteDesc2() {
+        // Ignore
+    }
+
+    @Test
+    @Override
+    public void testForEachByte2() {
+        // Ignore
+    }
 }