diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java index 3061c19329..7fe74de3c1 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java @@ -316,6 +316,11 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { // Call back the application and retrieve the number of bytes that have been // immediately processed. bytesToReturn = listener.onDataRead(ctx, streamId, data, padding, endOfStream); + + if (endOfStream) { + lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); + } + return bytesToReturn; } catch (Http2Exception e) { // If an exception happened during delivery, the listener may have returned part @@ -334,10 +339,6 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { } finally { // If appropriate, return the processed bytes to the flow controller. flowController.consumeBytes(stream, bytesToReturn); - - if (endOfStream) { - lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); - } } } @@ -411,16 +412,13 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { } stream.headersReceived(isInformational); - try { - verifyContentLength(stream, 0, endOfStream); - encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive); - listener.onHeadersRead(ctx, streamId, headers, streamDependency, - weight, exclusive, padding, endOfStream); - } finally { - // If the headers completes this stream, close it. - if (endOfStream) { - lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); - } + verifyContentLength(stream, 0, endOfStream); + encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive); + listener.onHeadersRead(ctx, streamId, headers, streamDependency, + weight, exclusive, padding, endOfStream); + // If the headers completes this stream, close it. + if (endOfStream) { + lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java index 42ab7031a1..3bf546f37a 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java @@ -472,7 +472,6 @@ public class DefaultHttp2ConnectionDecoderTest { } catch (RuntimeException cause) { verify(localFlow) .receiveFlowControlledFrame(eq(stream), eq(data), eq(padding), eq(true)); - verify(lifecycleManager).closeStreamRemote(eq(stream), eq(future)); verify(listener).onDataRead(eq(ctx), eq(STREAM_ID), eq(data), eq(padding), eq(true)); assertEquals(0, localFlow.unconsumedBytes(stream)); } finally { diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java index 211ef092cb..a0e06788e2 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java @@ -240,12 +240,64 @@ public abstract class Http2MultiplexTest { @Test public void headerContentLengthNotMatchValidationShouldPropagate() { + headerContentLengthNotMatchValidationShouldPropagate(false, false, false); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateWithEndStream() { + headerContentLengthNotMatchValidationShouldPropagate(false, true, false); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateCloseLocal() { + headerContentLengthNotMatchValidationShouldPropagate(true, false, false); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateWithEndStreamCloseLocal() { + headerContentLengthNotMatchValidationShouldPropagate(true, true, false); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateTrailers() { + headerContentLengthNotMatchValidationShouldPropagate(false, false, true); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateWithEndStreamTrailers() { + headerContentLengthNotMatchValidationShouldPropagate(false, true, true); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateCloseLocalTrailers() { + headerContentLengthNotMatchValidationShouldPropagate(true, false, true); + } + + @Test + public void headerContentLengthNotMatchValidationShouldPropagateWithEndStreamCloseLocalTrailers() { + headerContentLengthNotMatchValidationShouldPropagate(true, true, true); + } + + private void headerContentLengthNotMatchValidationShouldPropagate( + boolean closeLocal, boolean endStream, boolean trailer) { LastInboundHandler inboundHandler = new LastInboundHandler(); request.addLong(HttpHeaderNames.CONTENT_LENGTH, 1); Http2StreamChannel channel = newInboundStream(3, false, inboundHandler); assertTrue(channel.isActive()); - frameInboundWriter.writeInboundData(channel.stream().id(), bb("foo"), 0, false); + if (closeLocal) { + channel.writeAndFlush(new DefaultHttp2HeadersFrame(new DefaultHttp2Headers(), true)) + .syncUninterruptibly(); + assertEquals(Http2Stream.State.HALF_CLOSED_LOCAL, channel.stream().state()); + } else { + assertEquals(Http2Stream.State.OPEN, channel.stream().state()); + } + + if (trailer) { + frameInboundWriter.writeInboundHeaders(channel.stream().id(), new DefaultHttp2Headers(), 0, endStream); + } else { + frameInboundWriter.writeInboundData(channel.stream().id(), bb("foo"), 0, endStream); + } try { inboundHandler.checkException(); fail();