From 379ac890f4dbec15d19714711f85455a12112c3f Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 1 Sep 2017 12:17:58 +0200 Subject: [PATCH] Fix reference count issue when using Http2FrameCodec / Http2MultiplexCodec with HttpServerUpgradeHandler Motivation: When using Http2FrameCodec / Http2MultiplexCodec with HttpServerUpgradeHandler reference count exception will be triggered. Modifications: - Correctly retain before calling InboundHttpToHttp2Adapter.handle - Add unit test Result: Fixes [#7172]. --- .../handler/codec/http2/Http2FrameCodec.java | 2 +- .../codec/http2/Http2FrameCodecTest.java | 31 +++++++++++++++++++ .../org.mockito.plugins.MockMaker | 1 + 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 codec-http2/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java index 6f4350dcd5..b1bcdd213a 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java @@ -248,7 +248,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler { upgrade.upgradeRequest().headers().setInt( HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), HTTP_UPGRADE_STREAM_ID); InboundHttpToHttp2Adapter.handle( - ctx, connection(), decoder().frameListener(), upgrade.upgradeRequest()); + ctx, connection(), decoder().frameListener(), upgrade.upgradeRequest().retain()); } finally { upgrade.release(); } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java index d10bec643c..c871cf803b 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java @@ -24,9 +24,13 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPromise; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.UnsupportedMessageTypeException; +import io.netty.handler.codec.http.DefaultFullHttpRequest; +import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpScheme; +import io.netty.handler.codec.http.HttpServerUpgradeHandler; +import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http2.Http2Exception.StreamException; import io.netty.handler.codec.http2.Http2Stream.State; import io.netty.handler.logging.LogLevel; @@ -40,6 +44,8 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import java.net.InetSocketAddress; import java.util.HashSet; @@ -641,6 +647,31 @@ public class Http2FrameCodecTest { assertTrue(listenerExecuted.get()); } + @Test + public void upgradeEventNoRefCntError() throws Http2Exception { + frameListener.onHeadersRead(http2HandlerCtx, Http2CodecUtil.HTTP_UPGRADE_STREAM_ID, request, 31, false); + + final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"); + final HttpServerUpgradeHandler.UpgradeEvent upgradeEvent = mock(HttpServerUpgradeHandler.UpgradeEvent.class); + when(upgradeEvent.retain()).thenAnswer(new Answer() { + @Override + public HttpServerUpgradeHandler.UpgradeEvent answer(InvocationOnMock invocationOnMock) throws Throwable { + request.retain(); + return upgradeEvent; + } + }); + when(upgradeEvent.release()).thenAnswer(new Answer() { + @Override + public Boolean answer(InvocationOnMock invocationOnMock) throws Throwable { + return request.release(); + } + }); + + when(upgradeEvent.upgradeRequest()).thenReturn(request); + channel.pipeline().fireUserEventTriggered(upgradeEvent); + assertEquals(1, request.refCnt()); + } + private static ChannelPromise anyChannelPromise() { return any(ChannelPromise.class); } diff --git a/codec-http2/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/codec-http2/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 0000000000..ca6ee9cea8 --- /dev/null +++ b/codec-http2/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline \ No newline at end of file