andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

HTTP/2 Writes GO_AWAY on channelInactive

Browse Source 
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.
This commit is contained in:
Scott Mitchell 2016-02-18 14:02:17 -08:00
parent 23f7fc67a4
commit 83c4aa6ad8
2 changed files with 15 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
View File

@ -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());
}

12
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
View File

@ -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));
}