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

Do not send GOAWAY frame before connection preface (#11107)

Browse Source 
Motivation

A GOAWAY frame (or any other HTTP/2 frame) should not be sent before the
connection preface. Clients that immediately close the channel may
currently attempt to send a GOAWAY frame before the connection preface,
resulting in servers receiving a seemingly-corrupt connection preface.

Modifications

* Ensure that the preface has been sent before attempting to
automatically send a GOAWAY frame as part of channel shutdown logic
* Add unit test that only passes with new behavior

Result

Fixes https://github.com/netty/netty/issues/11026

Co-authored-by: Bennett Lynch <Bennett-Lynch@users.noreply.github.com>
This commit is contained in:
Bennett Lynch 2021-03-23 01:04:46 -07:00 committed by Norman Maurer
parent 56703b93ba
commit 7753431e48
2 changed files with 13 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -458,8 +458,8 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
return;
}
promise = promise.unvoid();
// Avoid NotYetConnectedException
if (!ctx.channel().isActive()) {
// Avoid NotYetConnectedException and avoid sending before connection preface
if (!ctx.channel().isActive() || !prefaceSent()) {
ctx.close(promise);
return;
}

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

@ -73,6 +73,7 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
/**
@ -691,6 +692,16 @@ public class Http2ConnectionHandlerTest {
any(ChannelPromise.class));
}
@Test
public void clientChannelClosedDoesNotSendGoAwayBeforePreface() throws Exception {
when(connection.isServer()).thenReturn(false);
when(channel.isActive()).thenReturn(false);
handler = newHandler();
when(channel.isActive()).thenReturn(true);
handler.close(ctx, promise);
verifyZeroInteractions(frameWriter);
}
@Test
public void writeRstStreamForUnknownStreamUsingVoidPromise() throws Exception {
writeRstStreamUsingVoidPromise(NON_EXISTANT_STREAM_ID);