[#4386] ByteToMessage.decodeLast(...) should not call decode(...) if buffer is empty.

Motivation:

If the input buffer is empty we should not have decodeLast(...) call decode(...) as the user may not expect this.

Modifications:

- Not call decode(...) in decodeLast(...) if the input buffer is empty.
- Add testcases.

Result:

decodeLast(...) will not call decode(...) if input buffer is empty.

codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java

@@ -398,7 +398,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
 
     @Override
     protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-        decode(ctx, in, out);
+        super.decodeLast(ctx, in, out);
 
         // Handle the last unfinished message.
         if (message != null) {

codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java

@@ -150,7 +150,11 @@ public abstract class ByteToMessageCodec<I> extends ChannelDuplexHandler {
      * @see ByteToMessageDecoder#decodeLast(ChannelHandlerContext, ByteBuf, List)
      */
     protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-        decode(ctx, in, out);
+        if (in.isReadable()) {
+            // Only call decode() if there is something left in the buffer to decode.
+            // See https://github.com/netty/netty/issues/4386
+            decode(ctx, in, out);
+        }
     }
 
     private final class Encoder extends MessageToByteEncoder<I> {

codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java

@@ -439,7 +439,11 @@ public abstract class ByteToMessageDecoder extends ChannelInboundHandlerAdapter
      * override this for some special cleanup operation.
      */
     protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-        decode(ctx, in, out);
+        if (in.isReadable()) {
+            // Only call decode() if there is something left in the buffer to decode.
+            // See https://github.com/netty/netty/issues/4386
+            decode(ctx, in, out);
+        }
     }
 
     static ByteBuf expandCumulation(ByteBufAllocator alloc, ByteBuf cumulation, int readable) {

codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java

@@ -21,6 +21,7 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.util.ReferenceCountUtil;
+import io.netty.util.internal.ThreadLocalRandom;
 import org.junit.Test;
 
 import java.util.List;
@@ -137,10 +138,15 @@ public class ByteToMessageDecoderTest {
         EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() {
             @Override
             protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-                in.skipBytes(in.readableBytes());
-                if (!ctx.channel().isActive()) {
-                    out.add("data");
-                }
+                int readable = in.readableBytes();
+                assertTrue(readable > 0);
+                in.skipBytes(readable);
+            }
+
+            @Override
+            protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+                assertFalse(in.isReadable());
+                out.add("data");
             }
         }, new ChannelInboundHandlerAdapter() {
             @Override
@@ -199,4 +205,66 @@ public class ByteToMessageDecoderTest {
         buf.release();
         b.release();
     }
+
+    @Test
+    public void testDecodeLastEmptyBuffer() {
+        EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() {
+            @Override
+            protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+                int readable = in.readableBytes();
+                assertTrue(readable > 0);
+                out.add(in.readBytes(readable));
+            }
+        });
+        byte[] bytes = new byte[1024];
+        ThreadLocalRandom.current().nextBytes(bytes);
+
+        assertTrue(channel.writeInbound(Unpooled.wrappedBuffer(bytes)));
+        assertBuffer(Unpooled.wrappedBuffer(bytes), (ByteBuf) channel.readInbound());
+        assertNull(channel.readInbound());
+        assertFalse(channel.finish());
+        assertNull(channel.readInbound());
+    }
+
+    @Test
+    public void testDecodeLastNonEmptyBuffer() {
+        EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() {
+            private boolean decodeLast;
+
+            @Override
+            protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+                int readable = in.readableBytes();
+                assertTrue(readable > 0);
+                if (!decodeLast && readable == 1) {
+                    return;
+                }
+                out.add(in.readBytes(decodeLast ? readable : readable - 1));
+            }
+
+            @Override
+            protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+                assertFalse(decodeLast);
+                decodeLast = true;
+                super.decodeLast(ctx, in, out);
+            }
+        });
+        byte[] bytes = new byte[1024];
+        ThreadLocalRandom.current().nextBytes(bytes);
+
+        assertTrue(channel.writeInbound(Unpooled.wrappedBuffer(bytes)));
+        assertBuffer(Unpooled.wrappedBuffer(bytes, 0, bytes.length - 1), (ByteBuf) channel.readInbound());
+        assertNull(channel.readInbound());
+        assertTrue(channel.finish());
+        assertBuffer(Unpooled.wrappedBuffer(bytes, bytes.length - 1, 1), (ByteBuf) channel.readInbound());
+        assertNull(channel.readInbound());
+    }
+
+    private static void assertBuffer(ByteBuf expected, ByteBuf buffer) {
+        try {
+            assertEquals(expected, buffer);
+        } finally {
+            buffer.release();
+            expected.release();
+        }
+    }
 }

codec/src/test/java/io/netty/handler/codec/ReplayingDecoderTest.java

@@ -188,10 +188,16 @@ public class ReplayingDecoderTest {
 
             @Override
             protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-                in.skipBytes(in.readableBytes());
-                if (!ctx.channel().isActive()) {
-                    out.add("data");
-                }
+                int readable = in.readableBytes();
+                assertTrue(readable > 0);
+                in.skipBytes(readable);
+                out.add("data");
+            }
+
+            @Override
+            protected void decodeLast(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+                assertFalse(in.isReadable());
+                out.add("data");
             }
         }, new ChannelInboundHandlerAdapter() {
             @Override