HTTP/2 Sending a GO_AWAY with an error code should close conneciton

Motivation:
The specification requires that sending a GO_AWAY frame with an error code results in closing the TCP connection https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-5.4.1.

Modifications:
- Close the connection after succesfully sending a GO_AWAY.

Result:
Fixes https://github.com/netty/netty/issues/3653
This commit is contained in:
Scott Mitchell 2015-04-22 15:18:42 -07:00
parent 891be30a28
commit f250dfedbe
3 changed files with 31 additions and 11 deletions

View File

@ -562,11 +562,17 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
future.addListener(new GenericFutureListener<ChannelFuture>() { future.addListener(new GenericFutureListener<ChannelFuture>() {
@Override @Override
public void operationComplete(ChannelFuture future) throws Exception { public void operationComplete(ChannelFuture future) throws Exception {
if (!future.isSuccess()) { if (future.isSuccess()) {
String msg = format("Sending GOAWAY failed: lastStreamId '%d', errorCode '%d', " + if (errorCode != NO_ERROR.code()) {
"debugData '%s'.", lastStreamId, errorCode, debugData); ctx.close();
logger.error(msg, future.cause()); }
ctx.channel().close(); } else {
if (logger.isErrorEnabled()) {
logger.error(
format("Sending GOAWAY failed: lastStreamId '%d', errorCode '%d', debugData '%s'.",
lastStreamId, errorCode, debugData), future.cause());
}
ctx.close();
} }
} }
}); });

View File

@ -66,11 +66,12 @@ public interface Http2LifecycleManager {
ChannelPromise promise); ChannelPromise promise);
/** /**
* Close the connection and prevent the peer from creating streams. After this call the peer * Prevents the peer from creating streams and close the connection if {@code errorCode} is not
* is not allowed to create any new streams and the local endpoint will be limited to creating streams with * {@link Http2Error#NO_ERROR}. After this call the peer is not allowed to create any new streams and the local
* {@code stream identifier <= lastStreamId}. This may result in sending a {@code GO_AWAY} frame (assuming we * endpoint will be limited to creating streams with {@code stream identifier <= lastStreamId}. This may result in
* have not already sent one with {@code Last-Stream-ID <= lastStreamId}), or may just return success if a * sending a {@code GO_AWAY} frame (assuming we have not already sent one with
* {@code GO_AWAY} has previously been sent. * {@code Last-Stream-ID <= lastStreamId}), or may just return success if a {@code GO_AWAY} has previously been
* sent.
* @param ctx The context used for communication and buffer allocation if necessary. * @param ctx The context used for communication and buffer allocation if necessary.
* @param lastStreamId The last stream that the local endpoint is claiming it will accept. * @param lastStreamId The last stream that the local endpoint is claiming it will accept.
* @param errorCode The rational as to why the connection is being closed. See {@link Http2Error}. * @param errorCode The rational as to why the connection is being closed. See {@link Http2Error}.

View File

@ -301,14 +301,27 @@ public class Http2ConnectionHandlerTest {
verify(ctx, times(1)).close(any(ChannelPromise.class)); verify(ctx, times(1)).close(any(ChannelPromise.class));
} }
@SuppressWarnings("unchecked")
@Test
public void canSendGoAwayFrame() throws Exception { public void canSendGoAwayFrame() throws Exception {
handler = newHandler();
ByteBuf data = mock(ByteBuf.class); ByteBuf data = mock(ByteBuf.class);
long errorCode = Http2Error.INTERNAL_ERROR.code(); long errorCode = Http2Error.INTERNAL_ERROR.code();
when(future.isDone()).thenReturn(true);
when(future.isSuccess()).thenReturn(true);
when(frameWriter.writeGoAway(eq(ctx), eq(STREAM_ID), eq(errorCode), eq(data), eq(promise))).thenReturn(future);
doAnswer(new Answer<Void>() {
@Override
public Void answer(InvocationOnMock invocation) throws Throwable {
invocation.getArgumentAt(0, GenericFutureListener.class).operationComplete(future);
return null;
}
}).when(future).addListener(any(GenericFutureListener.class));
handler = newHandler();
handler.goAway(ctx, STREAM_ID, errorCode, data, promise); handler.goAway(ctx, STREAM_ID, errorCode, data, promise);
verify(connection).goAwaySent(eq(STREAM_ID), eq(errorCode), eq(data)); verify(connection).goAwaySent(eq(STREAM_ID), eq(errorCode), eq(data));
verify(frameWriter).writeGoAway(eq(ctx), eq(STREAM_ID), eq(errorCode), eq(data), eq(promise)); verify(frameWriter).writeGoAway(eq(ctx), eq(STREAM_ID), eq(errorCode), eq(data), eq(promise));
verify(ctx).close();
} }
@Test @Test