From 5f60f22b6aed5031fbdef31a8479a2142ee0b706 Mon Sep 17 00:00:00 2001
From: Jihun Cho <jihuncho@google.com>
Date: Wed, 13 Feb 2019 11:48:59 -0800
Subject: [PATCH] netty: prevent IndexOutOfBoundsException when no handler is
 passed to BufferingHandler

---
 .../io/grpc/netty/ProtocolNegotiators.java    |  2 +-
 .../grpc/netty/ProtocolNegotiatorsTest.java   | 26 +++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java b/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java
index c028b6b54c..2d47eb6e2c 100644
--- a/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java
+++ b/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java
@@ -465,7 +465,7 @@ final class ProtocolNegotiators {
        * This check is necessary as a channel may be registered with different event loops during it
        * lifetime and we only want to configure it once.
        */
-      if (handlers != null) {
+      if (handlers != null && handlers.length > 0) {
         for (ChannelHandler handler : handlers) {
           ctx.pipeline().addBefore(ctx.name(), null, handler);
         }
diff --git a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java
index 6beda2a39b..df1c052d42 100644
--- a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java
+++ b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java
@@ -32,6 +32,7 @@ import io.grpc.Grpc;
 import io.grpc.SecurityLevel;
 import io.grpc.internal.GrpcAttributes;
 import io.grpc.internal.testing.TestUtils;
+import io.grpc.netty.ProtocolNegotiators.AbstractBufferingHandler;
 import io.grpc.netty.ProtocolNegotiators.HostPort;
 import io.grpc.netty.ProtocolNegotiators.ServerTlsHandler;
 import io.grpc.netty.ProtocolNegotiators.TlsNegotiator;
@@ -584,6 +585,24 @@ public class ProtocolNegotiatorsTest {
     elg.shutdownGracefully();
   }
 
+  @Test(expected = Test.None.class /* no exception expected */)
+  @SuppressWarnings("TestExceptionChecker")
+  public void bufferingHandler_shouldNotThrowForEmptyHandler() throws Exception {
+    LocalAddress addr = new LocalAddress("local");
+    ChannelFuture unused = new Bootstrap()
+        .channel(LocalChannel.class)
+        .handler(new BufferingHandlerWithoutHandlers())
+        .group(group)
+        .register().sync();
+    ChannelFuture sf = new ServerBootstrap()
+        .channel(LocalServerChannel.class)
+        .childHandler(new ChannelHandlerAdapter() {})
+        .group(group)
+        .bind(addr);
+    // sync will trigger client's NoHandlerBufferingHandler which should not throw
+    sf.sync();
+  }
+
   private static class FakeGrpcHttp2ConnectionHandler extends GrpcHttp2ConnectionHandler {
 
     static GrpcHttp2ConnectionHandler noopHandler() {
@@ -629,4 +648,11 @@ public class ProtocolNegotiatorsTest {
   private static ByteBuf bb(String s, Channel c) {
     return ByteBufUtil.writeUtf8(c.alloc(), s);
   }
+
+  private static class BufferingHandlerWithoutHandlers extends AbstractBufferingHandler {
+
+    public BufferingHandlerWithoutHandlers(ChannelHandler... handlers) {
+      super(handlers);
+    }
+  }
 }