Correctly handle client side http2 upgrades when Http2FrameCodec is used together with Http2MultiplexHandler (9495)

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java

@@ -47,13 +47,14 @@ public class Http2ClientUpgradeCodec implements HttpClientUpgradeHandler.Upgrade
     private final String handlerName;
     private final Http2ConnectionHandler connectionHandler;
     private final ChannelHandler upgradeToHandler;
+    private final ChannelHandler http2MultiplexHandler;
 
     public Http2ClientUpgradeCodec(Http2FrameCodec frameCodec, ChannelHandler upgradeToHandler) {
         this(null, frameCodec, upgradeToHandler);
     }
 
     public Http2ClientUpgradeCodec(String handlerName, Http2FrameCodec frameCodec, ChannelHandler upgradeToHandler) {
-        this(handlerName, (Http2ConnectionHandler) frameCodec, upgradeToHandler);
+        this(handlerName, (Http2ConnectionHandler) frameCodec, upgradeToHandler, null);
     }
 
     /**
@@ -66,6 +67,18 @@ public Http2ClientUpgradeCodec(Http2ConnectionHandler connectionHandler) {
         this((String) null, connectionHandler);
     }
 
+    /**
+     * Creates the codec using a default name for the connection handler when adding to the
+     * pipeline.
+     *
+     * @param connectionHandler the HTTP/2 connection handler
+     * @param http2MultiplexHandler the Http2 Multiplexer handler to work with Http2FrameCodec
+     */
+    public Http2ClientUpgradeCodec(Http2ConnectionHandler connectionHandler,
+        Http2MultiplexHandler http2MultiplexHandler) {
+        this((String) null, connectionHandler, http2MultiplexHandler);
+    }
+
     /**
      * Creates the codec providing an upgrade to the given handler for HTTP/2.
      *
@@ -74,37 +87,63 @@ public Http2ClientUpgradeCodec(Http2ConnectionHandler connectionHandler) {
      * @param connectionHandler the HTTP/2 connection handler
      */
     public Http2ClientUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler) {
-        this(handlerName, connectionHandler, connectionHandler);
+        this(handlerName, connectionHandler, connectionHandler, null);
+    }
+
+    /**
+     * Creates the codec providing an upgrade to the given handler for HTTP/2.
+     *
+     * @param handlerName the name of the HTTP/2 connection handler to be used in the pipeline,
+     *                    or {@code null} to auto-generate the name
+     * @param connectionHandler the HTTP/2 connection handler
+     */
+    public Http2ClientUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler,
+        Http2MultiplexHandler http2MultiplexHandler) {
+        this(handlerName, connectionHandler, connectionHandler, http2MultiplexHandler);
     }
 
     private Http2ClientUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler, ChannelHandler
-                                    upgradeToHandler) {
+        upgradeToHandler, Http2MultiplexHandler http2MultiplexHandler) {
         this.handlerName = handlerName;
         this.connectionHandler = checkNotNull(connectionHandler, "connectionHandler");
         this.upgradeToHandler = checkNotNull(upgradeToHandler, "upgradeToHandler");
+        this.http2MultiplexHandler = http2MultiplexHandler;
     }
 
     @Override
+
     public CharSequence protocol() {
         return HTTP_UPGRADE_PROTOCOL_NAME;
     }
 
     @Override
     public Collection<CharSequence> setUpgradeHeaders(ChannelHandlerContext ctx,
-            HttpRequest upgradeRequest) {
+        HttpRequest upgradeRequest) {
         CharSequence settingsValue = getSettingsHeaderValue(ctx);
         upgradeRequest.headers().set(HTTP_UPGRADE_SETTINGS_HEADER, settingsValue);
         return UPGRADE_HEADERS;
     }
 
     @Override
     public void upgradeTo(ChannelHandlerContext ctx, FullHttpResponse upgradeResponse)
-            throws Exception {
-        // Add the handler to the pipeline.
-        ctx.pipeline().addAfter(ctx.name(), handlerName, upgradeToHandler);
+        throws Exception {
+        try {
+            // Add the handler to the pipeline.
+            ctx.pipeline().addAfter(ctx.name(), handlerName, upgradeToHandler);
+
+            // Add the Http2 Multiplex handler as this handler handle events produced by the connectionHandler.
+            // See https://github.com/netty/netty/issues/9495
+            if (http2MultiplexHandler != null) {
+                final String name = ctx.pipeline().context(connectionHandler).name();
+                ctx.pipeline().addAfter(name, null, http2MultiplexHandler);
+            }
 
-        // Reserve local stream 1 for the response.
-        connectionHandler.onHttpClientUpgrade();
+            // Reserve local stream 1 for the response.
+            connectionHandler.onHttpClientUpgrade();
+        } catch (Http2Exception e) {
+            ctx.fireExceptionCaught(e);
+            ctx.close();
+        }
     }
 
     /**

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodecTest.java

@@ -33,33 +33,53 @@ public class Http2ClientUpgradeCodecTest {
 
     @Test
     public void testUpgradeToHttp2ConnectionHandler() throws Exception {
-        testUpgrade(new Http2ConnectionHandlerBuilder().server(false).frameListener(new Http2FrameAdapter()).build());
+        testUpgrade(new Http2ConnectionHandlerBuilder().server(false).frameListener(
+            new Http2FrameAdapter()).build(), null);
     }
 
     @Test
     public void testUpgradeToHttp2FrameCodec() throws Exception {
-        testUpgrade(Http2FrameCodecBuilder.forClient().build());
+        testUpgrade(Http2FrameCodecBuilder.forClient().build(), null);
     }
 
     @Test
     public void testUpgradeToHttp2MultiplexCodec() throws Exception {
         testUpgrade(Http2MultiplexCodecBuilder.forClient(new HttpInboundHandler())
-            .withUpgradeStreamHandler(new ChannelInboundHandlerAdapter()).build());
+            .withUpgradeStreamHandler(new ChannelInboundHandlerAdapter()).build(), null);
     }
 
-    private static void testUpgrade(Http2ConnectionHandler handler) throws Exception {
+    @Test
+    public void testUpgradeToHttp2FrameCodecWithMultiplexer() throws Exception {
+        testUpgrade(Http2FrameCodecBuilder.forClient().build(),
+            new Http2MultiplexHandler(new HttpInboundHandler(), new HttpInboundHandler()));
+    }
+
+    private static void testUpgrade(Http2ConnectionHandler handler, Http2MultiplexHandler multiplexer)
+            throws Exception {
         FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.OPTIONS, "*");
 
         EmbeddedChannel channel = new EmbeddedChannel(new ChannelInboundHandlerAdapter());
         ChannelHandlerContext ctx = channel.pipeline().firstContext();
-        Http2ClientUpgradeCodec codec = new Http2ClientUpgradeCodec("connectionHandler", handler);
+
+        Http2ClientUpgradeCodec codec;
+
+        if (multiplexer == null) {
+            codec = new Http2ClientUpgradeCodec("connectionHandler", handler);
+        } else {
+            codec = new Http2ClientUpgradeCodec("connectionHandler", handler, multiplexer);
+        }
+
         codec.setUpgradeHeaders(ctx, request);
         // Flush the channel to ensure we write out all buffered data
         channel.flush();
 
         codec.upgradeTo(ctx, null);
         assertNotNull(channel.pipeline().get("connectionHandler"));
 
+        if (multiplexer != null) {
+            assertNotNull(channel.pipeline().get(Http2MultiplexHandler.class));
+        }
+
         assertTrue(channel.finishAndReleaseAll());
     }