Add a new handler to close on inbound http/2 protocol errors (#1701)

* Initial commit

* Unit test

* Http2OrHttpHanlder unit test

* Rename metric

* Add missing license

* Remove unneeded go-away logic as it's already handled by netty

* Rename to try and make it clear that the new handler isn't doing all that much

zuul-core/src/main/java/com/netflix/netty/common/channel/config/CommonChannelConfigKeys.java

@@ -77,4 +77,6 @@ public class CommonChannelConfigKeys {
             new ChannelConfigKey<>("http2AllowGracefulDelayed", true);
     public static final ChannelConfigKey<Boolean> http2SwallowUnknownExceptionsOnConnClose =
             new ChannelConfigKey<>("http2SwallowUnknownExceptionsOnConnClose", false);
+    public static final ChannelConfigKey<Boolean> http2CatchConnectionErrors =
+            new ChannelConfigKey<>("http2CatchConnectionErrors", true);
 }

zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2ConnectionErrorHandler.java

@@ -0,0 +1,47 @@
+/**
+ * Copyright 2023 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.netflix.zuul.netty.server.http2;
+
+import com.netflix.zuul.netty.SpectatorUtils;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.handler.codec.http2.Http2Exception;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Logs and tracks connection errors. The actual
+ * sending of the go-away and closing the connection is handled by netty in {@link io.netty.handler.codec.http2.Http2ConnectionHandler}
+ * onConnectionError
+ *
+ * See also, {@link com.netflix.netty.common.channel.config.CommonChannelConfigKeys#http2CatchConnectionErrors}
+ * @author Justin Guerra
+ * @since 11/14/23
+ */
+public class Http2ConnectionErrorHandler extends ChannelInboundHandlerAdapter {
+
+    private static final Logger LOG = LoggerFactory.getLogger(Http2ConnectionErrorHandler.class);
+
+    @Override
+    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+        if(cause instanceof Http2Exception http2Exception) {
+            LOG.debug("Received Http/2 connection error", cause);
+            SpectatorUtils.newCounter("server.connection.http2.connection.exception", http2Exception.error().name()).increment();
+        } else {
+            ctx.fireExceptionCaught(cause);
+        }
+    }
+}

zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2OrHttpHandler.java

@@ -54,6 +54,7 @@ public class Http2OrHttpHandler extends ApplicationProtocolNegotiationHandler {
     private final int initialWindowSize;
     private final long maxHeaderTableSize;
     private final long maxHeaderListSize;
+    private final boolean catchConnectionErrors;
     private final Consumer<ChannelPipeline> addHttpHandlerFn;
 
     public Http2OrHttpHandler(
@@ -66,6 +67,7 @@ public Http2OrHttpHandler(
         this.initialWindowSize = channelConfig.get(CommonChannelConfigKeys.initialWindowSize);
         this.maxHeaderTableSize = channelConfig.get(CommonChannelConfigKeys.maxHttp2HeaderTableSize);
         this.maxHeaderListSize = channelConfig.get(CommonChannelConfigKeys.maxHttp2HeaderListSize);
+        this.catchConnectionErrors = channelConfig.get(CommonChannelConfigKeys.http2CatchConnectionErrors);
         this.addHttpHandlerFn = addHttpHandlerFn;
     }
 
@@ -107,8 +109,11 @@ private void configureHttp2(ChannelPipeline pipeline) {
         Http2MultiplexHandler multiplexHandler = new Http2MultiplexHandler(http2StreamHandler);
 
         // The frame codec MUST be in the pipeline.
-        pipeline.addBefore("codec_placeholder", /* name= */ null, frameCodec);
+        pipeline.addBefore("codec_placeholder", null, frameCodec);
         pipeline.replace("codec_placeholder", BaseZuulChannelInitializer.HTTP_CODEC_HANDLER_NAME, multiplexHandler);
+        if (catchConnectionErrors) {
+            pipeline.addLast(new Http2ConnectionErrorHandler());
+        }
     }
 
     private void configureHttp1(ChannelPipeline pipeline) {

zuul-core/src/test/java/com/netflix/zuul/netty/server/http2/Http2ConnectionErrorHandlerTest.java

@@ -0,0 +1,57 @@
+/**
+ * Copyright 2023 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.netflix.zuul.netty.server.http2;
+
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.embedded.EmbeddedChannel;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * @author Justin Guerra
+ * @since 11/15/23
+ */
+class Http2ConnectionErrorHandlerTest {
+
+    private EmbeddedChannel channel;
+    private ExceptionCapturingHandler exceptionCapturingHandler;
+
+    @BeforeEach
+    void setup() {
+        exceptionCapturingHandler = new ExceptionCapturingHandler();
+        channel = new EmbeddedChannel(new Http2ConnectionErrorHandler(), exceptionCapturingHandler);
+    }
+
+    @Test
+    public void nonHttp2ExceptionsPassedUpPipeline() {
+        RuntimeException exception = new RuntimeException();
+        channel.pipeline().fireExceptionCaught(exception);
+        assertEquals(exception, exceptionCapturingHandler.caught);
+    }
+
+    private static class ExceptionCapturingHandler extends ChannelInboundHandlerAdapter {
+
+        private Throwable caught;
+
+        @Override
+        public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+            this.caught = cause;
+        }
+    }
+}

zuul-core/src/test/java/com/netflix/zuul/netty/server/http2/Http2OrHttpHandlerTest.java

@@ -24,6 +24,7 @@
 import com.netflix.netty.common.metrics.Http2MetricsChannelHandlers;
 import com.netflix.spectator.api.NoopRegistry;
 import com.netflix.zuul.netty.server.BaseZuulChannelInitializer;
+import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.handler.codec.http2.Http2FrameCodec;
 import io.netty.handler.codec.http2.Http2MultiplexHandler;
@@ -32,6 +33,8 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 /**
  * @author Argha C
@@ -66,10 +69,42 @@ void swapInHttp2HandlerBasedOnALPN() throws Exception {
 
         http2OrHttpHandler.configurePipeline(channel.pipeline().lastContext(), ApplicationProtocolNames.HTTP_2);
 
-        assertThat(channel.pipeline().get(Http2FrameCodec.class.getSimpleName() + "#0"))
-                .isInstanceOf(Http2FrameCodec.class);
+        assertThat(channel.pipeline().get(Http2FrameCodec.class)).isInstanceOf(Http2FrameCodec.class);
         assertThat(channel.pipeline().get(BaseZuulChannelInitializer.HTTP_CODEC_HANDLER_NAME))
                 .isInstanceOf(Http2MultiplexHandler.class);
         assertEquals("HTTP/2", channel.attr(Http2OrHttpHandler.PROTOCOL_NAME).get());
     }
+
+    @Test
+    void protocolCloseHandlerAddedByDefault() throws Exception {
+        EmbeddedChannel channel = new EmbeddedChannel();
+        ChannelConfig channelConfig = new ChannelConfig();
+        channelConfig.add(new ChannelConfigValue<>(CommonChannelConfigKeys.maxHttp2HeaderListSize, 32768));
+
+        Http2OrHttpHandler http2OrHttpHandler =
+                new Http2OrHttpHandler(new ChannelInboundHandlerAdapter(), channelConfig, cp -> {});
+
+        channel.pipeline().addLast("codec_placeholder", new DummyChannelHandler());
+        channel.pipeline().addLast(Http2OrHttpHandler.class.getSimpleName(), http2OrHttpHandler);
+
+        http2OrHttpHandler.configurePipeline(channel.pipeline().lastContext(), ApplicationProtocolNames.HTTP_2);
+        assertNotNull(channel.pipeline().context(Http2ConnectionErrorHandler.class));
+    }
+
+    @Test
+    void skipProtocolCloseHandler() throws Exception {
+        EmbeddedChannel channel = new EmbeddedChannel();
+        ChannelConfig channelConfig = new ChannelConfig();
+        channelConfig.add(new ChannelConfigValue<>(CommonChannelConfigKeys.http2CatchConnectionErrors, false));
+        channelConfig.add(new ChannelConfigValue<>(CommonChannelConfigKeys.maxHttp2HeaderListSize, 32768));
+
+        Http2OrHttpHandler http2OrHttpHandler =
+                new Http2OrHttpHandler(new ChannelInboundHandlerAdapter(), channelConfig, cp -> {});
+
+        channel.pipeline().addLast("codec_placeholder", new DummyChannelHandler());
+        channel.pipeline().addLast(Http2OrHttpHandler.class.getSimpleName(), http2OrHttpHandler);
+
+        http2OrHttpHandler.configurePipeline(channel.pipeline().lastContext(), ApplicationProtocolNames.HTTP_2);
+        assertNull(channel.pipeline().context(Http2ConnectionErrorHandler.class));
+    }
 }