From 9b08dbca00ffdcb37ec0be2100d868c7c2dd72b9 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 27 Jul 2018 01:56:09 +0800
Subject: [PATCH] Leak detection combined with composite buffers results in
 incorrectly handled writerIndex when calling ByteBufUtil.writeAscii/writeUtf8
 (#8153)

Motivation:

We need to add special handling for WrappedCompositeByteBuf as these also extend AbstractByteBuf, otherwise we will not correctly adjust / read the writerIndex during processing.

Modifications:

- Add instanceof checks for WrappedCompositeByteBuf as well.
- Add testcases

Result:

Fixes https://github.com/netty/netty/issues/8152.
---
 .../java/io/netty/buffer/ByteBufUtil.java     | 10 ++-
 .../java/io/netty/buffer/ByteBufUtilTest.java | 72 +++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
index 0d1b3a9b89..e7afab3672 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
@@ -498,7 +498,10 @@ public final class ByteBufUtil {
      */
     public static int reserveAndWriteUtf8(ByteBuf buf, CharSequence seq, int reserveBytes) {
         for (;;) {
-            if (buf instanceof AbstractByteBuf) {
+            if (buf instanceof WrappedCompositeByteBuf) {
+                // WrappedCompositeByteBuf is a sub-class of AbstractByteBuf so it needs special handling.
+                buf = buf.unwrap();
+            } else if (buf instanceof AbstractByteBuf) {
                 AbstractByteBuf byteBuf = (AbstractByteBuf) buf;
                 byteBuf.ensureWritable0(reserveBytes);
                 int written = writeUtf8(byteBuf, byteBuf.writerIndex, seq, seq.length());
@@ -665,7 +668,10 @@ public final class ByteBufUtil {
             buf.writeBytes(asciiString.array(), asciiString.arrayOffset(), len);
         } else {
             for (;;) {
-                if (buf instanceof AbstractByteBuf) {
+                if (buf instanceof WrappedCompositeByteBuf) {
+                    // WrappedCompositeByteBuf is a sub-class of AbstractByteBuf so it needs special handling.
+                    buf = buf.unwrap();
+                } else if (buf instanceof AbstractByteBuf) {
                     AbstractByteBuf byteBuf = (AbstractByteBuf) buf;
                     byteBuf.ensureWritable0(len);
                     int written = writeAscii(byteBuf, byteBuf.writerIndex, seq, len);
diff --git a/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java b/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java
index 38e36e20db..da9b344f40 100644
--- a/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java
+++ b/buffer/src/test/java/io/netty/buffer/ByteBufUtilTest.java
@@ -238,6 +238,42 @@ public class ByteBufUtilTest {
         buf2.unwrap().release();
     }
 
+    @Test
+    public void testWriteUsAsciiComposite() {
+        String usAscii = "NettyRocks";
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(usAscii.getBytes(CharsetUtil.US_ASCII));
+        ByteBuf buf2 = Unpooled.compositeBuffer().addComponent(
+                Unpooled.buffer(8)).addComponent(Unpooled.buffer(24));
+        // write some byte so we start writing with an offset.
+        buf2.writeByte(1);
+        ByteBufUtil.writeAscii(buf2, usAscii);
+
+        // Skip the previously written byte.
+        assertEquals(buf, buf2.skipBytes(1));
+
+        buf.release();
+        buf2.release();
+    }
+
+    @Test
+    public void testWriteUsAsciiCompositeWrapped() {
+        String usAscii = "NettyRocks";
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(usAscii.getBytes(CharsetUtil.US_ASCII));
+        ByteBuf buf2 = new WrappedCompositeByteBuf(Unpooled.compositeBuffer().addComponent(
+                Unpooled.buffer(8)).addComponent(Unpooled.buffer(24)));
+        // write some byte so we start writing with an offset.
+        buf2.writeByte(1);
+        ByteBufUtil.writeAscii(buf2, usAscii);
+
+        // Skip the previously written byte.
+        assertEquals(buf, buf2.skipBytes(1));
+
+        buf.release();
+        buf2.release();
+    }
+
     @Test
     public void testWriteUtf8() {
         String usAscii = "Some UTF-8 like äÄ∏ŒŒ";
@@ -252,6 +288,42 @@ public class ByteBufUtilTest {
         buf2.release();
     }
 
+    @Test
+    public void testWriteUtf8Composite() {
+        String utf8 = "Some UTF-8 like äÄ∏ŒŒ";
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(utf8.getBytes(CharsetUtil.UTF_8));
+        ByteBuf buf2 = Unpooled.compositeBuffer().addComponent(
+                Unpooled.buffer(8)).addComponent(Unpooled.buffer(24));
+        // write some byte so we start writing with an offset.
+        buf2.writeByte(1);
+        ByteBufUtil.writeUtf8(buf2, utf8);
+
+        // Skip the previously written byte.
+        assertEquals(buf, buf2.skipBytes(1));
+
+        buf.release();
+        buf2.release();
+    }
+
+    @Test
+    public void testWriteUtf8CompositeWrapped() {
+        String utf8 = "Some UTF-8 like äÄ∏ŒŒ";
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(utf8.getBytes(CharsetUtil.UTF_8));
+        ByteBuf buf2 = new WrappedCompositeByteBuf(Unpooled.compositeBuffer().addComponent(
+                Unpooled.buffer(8)).addComponent(Unpooled.buffer(24)));
+        // write some byte so we start writing with an offset.
+        buf2.writeByte(1);
+        ByteBufUtil.writeUtf8(buf2, utf8);
+
+        // Skip the previously written byte.
+        assertEquals(buf, buf2.skipBytes(1));
+
+        buf.release();
+        buf2.release();
+    }
+
     @Test
     public void testWriteUtf8Surrogates() {
         // leading surrogate + trailing surrogate