From 8320a45c156f87afcdb545218f49d561b42843f9 Mon Sep 17 00:00:00 2001
From: chhsiao90 <chhsiao90@gmail.com>
Date: Fri, 7 Jul 2017 20:51:16 +0800
Subject: [PATCH] Configures HTTP2 pipeline with more proper way

Motivation:

When we use pipeline.replace and we still had ongoing inbound, then
there will be some problem that inbound message would go to wrong
handlers. So we add handler first, and remove self after add, so that
the next handler will be the correct one.

Modifications:

Uses remove after addAfter instead of replace.

Result:

Fixed #6881
---
 .../CleartextHttp2ServerUpgradeHandler.java   |  7 ++-
 ...leartextHttp2ServerUpgradeHandlerTest.java | 53 +++++++++++++++++--
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandler.java
index cca1911b9e59..c70b343920b6 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandler.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandler.java
@@ -89,8 +89,11 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
                 // following network traffic
                 ctx.pipeline()
                    .remove(httpServerCodec)
-                   .remove(httpServerUpgradeHandler)
-                   .replace(this, null, http2ServerHandler);
+                   .remove(httpServerUpgradeHandler);
+
+                ctx.pipeline().addAfter(ctx.name(), null, http2ServerHandler);
+                ctx.pipeline().remove(this);
+
                 ctx.fireUserEventTriggered(PriorKnowledgeUpgradeEvent.INSTANCE);
             }
         }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java
index e4bd0be2d657..1b1fde8b7a9c 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java
@@ -17,8 +17,10 @@
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
+import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.ChannelInitializer;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.handler.codec.http.DefaultHttpHeaders;
 import io.netty.handler.codec.http.HttpMethod;
@@ -35,7 +37,6 @@
 import io.netty.util.CharsetUtil;
 import io.netty.util.ReferenceCountUtil;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.util.ArrayList;
@@ -56,8 +57,7 @@ public class CleartextHttp2ServerUpgradeHandlerTest {
 
     private List<Object> userEvents;
 
-    @Before
-    public void setUp() {
+    private void setUpServerChannel() {
         frameListener = mock(Http2FrameListener.class);
 
         http2ConnectionHandler = new Http2ConnectionHandlerBuilder().frameListener(frameListener).build();
@@ -91,6 +91,8 @@ public void tearDown() throws Exception {
 
     @Test
     public void priorKnowledge() throws Exception {
+        setUpServerChannel();
+
         channel.writeInbound(Http2CodecUtil.connectionPrefaceBuf());
 
         ByteBuf settingsFrame = settingsFrameBuf();
@@ -109,6 +111,8 @@ public void priorKnowledge() throws Exception {
 
     @Test
     public void upgrade() throws Exception {
+        setUpServerChannel();
+
         String upgradeString = "GET / HTTP/1.1\r\n" +
                                "Host: example.com\r\n" +
                                "Connection: Upgrade, HTTP2-Settings\r\n" +
@@ -138,6 +142,8 @@ public void upgrade() throws Exception {
 
     @Test
     public void priorKnowledgeInFragments() throws Exception {
+        setUpServerChannel();
+
         ByteBuf connectionPreface = Http2CodecUtil.connectionPrefaceBuf();
         assertFalse(channel.writeInbound(connectionPreface.readBytes(5), connectionPreface));
 
@@ -156,6 +162,8 @@ public void priorKnowledgeInFragments() throws Exception {
 
     @Test
     public void downgrade() throws Exception {
+        setUpServerChannel();
+
         String requestString = "GET / HTTP/1.1\r\n" +
                          "Host: example.com\r\n\r\n";
         ByteBuf inbound = Unpooled.buffer().writeBytes(requestString.getBytes(CharsetUtil.US_ASCII));
@@ -175,6 +183,45 @@ public void downgrade() throws Exception {
         assertNull(channel.readInbound());
     }
 
+    @Test
+    public void usedHttp2Codec() throws Exception {
+        final Http2Codec http2Codec = new Http2CodecBuilder(true, new ChannelInitializer<Channel>() {
+            @Override
+            protected void initChannel(Channel ch) throws Exception {
+            }
+        }).build();
+        UpgradeCodecFactory upgradeCodecFactory = new UpgradeCodecFactory() {
+            @Override
+            public UpgradeCodec newUpgradeCodec(CharSequence protocol) {
+                return new Http2ServerUpgradeCodec(http2Codec);
+            }
+        };
+        http2ConnectionHandler = http2Codec.frameCodec().connectionHandler();
+
+        userEvents = new ArrayList<Object>();
+
+        HttpServerCodec httpServerCodec = new HttpServerCodec();
+        HttpServerUpgradeHandler upgradeHandler = new HttpServerUpgradeHandler(httpServerCodec, upgradeCodecFactory);
+
+        CleartextHttp2ServerUpgradeHandler handler = new CleartextHttp2ServerUpgradeHandler(
+                httpServerCodec, upgradeHandler, http2Codec);
+        channel = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
+            @Override
+            public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
+                userEvents.add(evt);
+            }
+        });
+
+        assertFalse(channel.writeInbound(Http2CodecUtil.connectionPrefaceBuf()));
+
+        ByteBuf settingsFrame = settingsFrameBuf();
+
+        assertFalse(channel.writeInbound(settingsFrame));
+
+        assertEquals(1, userEvents.size());
+        assertTrue(userEvents.get(0) instanceof PriorKnowledgeUpgradeEvent);
+    }
+
     private static ByteBuf settingsFrameBuf() {
         ByteBuf settingsFrame = Unpooled.buffer();
         settingsFrame.writeMedium(12); // Payload length