Repository: netty/netty
Issue: NullPointerException while accessing a readable slice of CompositeByteBuf after discarding some bytes

### Expected behavior

After skipping and discarding some bytes using `skipBytes()` and `discardSomeReadBytes()` functions in a `CompositeByteBuf`, the readable bytes in the buffer should still be accessible.

### Actual behavior
Throws NullPointerException.

```
Exception in thread "main" java.lang.NullPointerException
	at io.netty.buffer.CompositeByteBuf.findIt(CompositeByteBuf.java:1604)
	at io.netty.buffer.CompositeByteBuf.findComponent0(CompositeByteBuf.java:1597)
	at io.netty.buffer.CompositeByteBuf._getByte(CompositeByteBuf.java:949)
	at io.netty.buffer.UnpooledSlicedByteBuf._getByte(UnpooledSlicedByteBuf.java:39)
	at io.netty.buffer.AbstractByteBuf.readByte(AbstractByteBuf.java:734)
...
```

### Steps to reproduce

1. Allocate a container using CompositeByteBuf
2. Fill it with a segment
3. Retain a slice of payload from the container for future access
4. Skip retained number of bytes in the container
5. Call `discardSomeReadBytes()` on the container
6. Try to access the sliced buffer from Step 3.

### Minimal yet complete reproducer code (or URL to code)
```
import io.netty.buffer.ByteBuf;
import io.netty.buffer.CompositeByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.ResourceLeakDetector;

public class BufBug {

    static {
        ResourceLeakDetector.setLevel(ResourceLeakDetector.Level.PARANOID);
    }
    private static final boolean EXPECTED = true;

    public static void main(String[] args) {
        // allocate a container
        final CompositeByteBuf composite = Unpooled.compositeBuffer();
        // fill it with a segment
        composite.addComponent(true, Unpooled.copyBoolean(EXPECTED));
        // take the whole payload (weirdly)
        final ByteBuf sliced = composite.retainedSlice();
        composite.skipBytes(sliced.readableBytes());
        composite.discardSomeReadBytes();
        // observe that "sliced" still claims to have readable bytes
        System.out.println("readerIndex = " + sliced.readerIndex()
                + ", writerIndex = " + sliced.writerIndex()
                + ", readableBytes = " + sliced.readableBytes());
        // this should work, but throws
        System.out.println("first byte = " + sliced.readBoolean());
        sliced.release();
        // cleanup
        composite.release();
    }
}
```

### Netty version
Affects 4.1.53.Final and 4.1.56.Final

### JVM version (e.g. `java -version`)
java.version: 11.0.2
java.vm.name: OpenJDK 64-Bit Server VM

### OS version (e.g. `uname -a`)
os.name: Mac OS X
os.arch: x86_64
os.version: 10.15.7

Issue comments:
@srinidhibashyam I don't think byte discarding is intended to be used in conjunction with derived buffers (slices/duplicates) since it moves things around in the underlying buffer without derived buffers' knowledge. There should probably be a check to disallow those operations if the buffer's ref count is > 1 (as it would be in your case), and I recall some discussion about that a very long time ago.

Depending on your use case, you could just use `copy()` instead of `retainedSlice()`. Or if the data is very large and you want to avoid copying bytes you could do:
```java
sliced = composite.alloc().compositeBuffer().addFlattenedComponents(true, composite)
```
after which you can call `discardReadBytes()` on `composite` without it affecting `sliced`'s content. However this only works if `composite` really is a `CompositeByteBuf`. 
The ref count check would guard against this issue when you use `retainedSlice`, but not if you just use `slice`, so I'm not sure it's worth it to add. I could also imagine some code that works today would be broken by stricter checks.

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
index 2b03b801cb1..6dc0a2cbf73 100644
--- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -1601,6 +1601,10 @@ private Component findIt(int offset) {
         for (int low = 0, high = componentCount; low <= high;) {
             int mid = low + high >>> 1;
             Component c = components[mid];
+            if (c == null) {
+                throw new IllegalStateException("No component found for offset. " +
+                        "Composite buffer layout might be outdated, e.g. from a discardReadBytes call.");
+            }
             if (offset >= c.endOffset) {
                 low = mid + 1;
             } else if (offset < c.offset) {


Test patch:
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
index 6991fabd539..4af2e2efbd4 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
@@ -1614,4 +1614,24 @@ public void testOverflowWhileAddingComponentsViaIterable() {
             compositeByteBuf.release();
         }
     }
+
+    @Test
+    public void sliceOfCompositeBufferMustThrowISEAfterDiscardBytes() {
+        CompositeByteBuf composite = compositeBuffer();
+        composite.addComponent(true, buffer(8).writeZero(8));
+
+        ByteBuf slice = composite.retainedSlice();
+        composite.skipBytes(slice.readableBytes());
+        composite.discardSomeReadBytes();
+
+        try {
+            slice.readByte();
+            fail("Expected readByte of discarded slice to throw.");
+        } catch (IllegalStateException ignore) {
+            // Good.
+        } finally {
+            slice.release();
+            composite.release();
+        }
+    }
 }