From 83c4aa6ad880445856551de1f7d4aeb40ee06df4 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Thu, 18 Feb 2016 14:02:17 -0800
Subject: [PATCH] HTTP/2 Writes GO_AWAY on channelInactive

Motivation:
Http2ConnectionHandler inherits from ByteToMessageDecoder. ByteToMessageDecoder.channelInactive will attempt to decode any remaining data by calling the abstract decode method. If the Http2ConnectionHandler is in server mode, and no data has been exchanged yet, it will try to treat this data as an invalid connection preface and write a GO_AWAY. This is noisy in the logs and creates an illusion that there is a protocol violation when there has not been.

Modifications:
- If the channel is inactive the connection preface decode should not be executed.

Result:
Log files don't include misleading error messages related to connection preface errors.
---
 .../handler/codec/http2/Http2ConnectionHandler.java  |  6 +++---
 .../codec/http2/Http2ConnectionHandlerTest.java      | 12 ++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
index 5c975af55a..18edf2e2fb 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
@@ -207,7 +207,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
         @Override
         public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
             try {
-                if (readClientPrefaceString(in) && verifyFirstFrameIsSettings(in)) {
+                if (ctx.channel().isActive() && readClientPrefaceString(in) && verifyFirstFrameIsSettings(in)) {
                     // After the preface is read, it is time to hand over control to the post initialized decoder.
                     byteDecoder = new FrameDecoder();
                     byteDecoder.decode(ctx, in, out);
@@ -735,8 +735,8 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
                     ctx.close();
                 }
             } else {
-                if (logger.isErrorEnabled()) {
-                    logger.error("{} Sending GOAWAY failed: lastStreamId '{}', errorCode '{}', " +
+                if (logger.isWarnEnabled()) {
+                    logger.warn("{} Sending GOAWAY failed: lastStreamId '{}', errorCode '{}', " +
                                  "debugData '{}'. Forcing shutdown of the connection.",
                                  ctx.channel(), lastStreamId, errorCode, debugData.toString(UTF_8), future.cause());
                 }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
index ed9d1c06ea..65b8670c77 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
@@ -436,6 +436,18 @@ public class Http2ConnectionHandlerTest {
         verify(ctx, times(1)).flush();
     }
 
+    @Test
+    public void channelClosedDoesNotThrowPrefaceException() throws Exception {
+        when(connection.isServer()).thenReturn(true);
+        handler = newHandler();
+        when(channel.isActive()).thenReturn(false);
+        handler.channelInactive(ctx);
+        verify(frameWriter, never()).writeGoAway(any(ChannelHandlerContext.class), anyInt(), anyLong(),
+                                                 any(ByteBuf.class), any(ChannelPromise.class));
+        verify(frameWriter, never()).writeRstStream(any(ChannelHandlerContext.class), anyInt(), anyLong(),
+                                                    any(ChannelPromise.class));
+    }
+
     private static ByteBuf dummyData() {
         return Unpooled.buffer().writeBytes("abcdefgh".getBytes(UTF_8));
     }