From dd1785ba66dca72a82ec5e5d2e63498f1f591db0 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Wed, 17 Jul 2019 12:12:19 -0600 Subject: [PATCH] Fix an NPE in AbstractHttp2StreamChannel (#9379) Motivation: If a read triggers a AbstractHttp2StreamChannel to close we can get an NPE in the read loop. Modifications: Make sure that the inboundBuffer isn't null before attempting to continue the loop. Result: No NPE. Fixes #9337 --- .../http2/AbstractHttp2StreamChannel.java | 2 +- .../codec/http2/Http2MultiplexTest.java | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java index 68ed5c990a..32825db919 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java @@ -783,7 +783,7 @@ abstract class AbstractHttp2StreamChannel extends DefaultAttributeMap implements do { flowControlledBytes += doRead0((Http2Frame) message, allocHandle); } while ((readEOS || (continueReading = allocHandle.continueReading())) && - (message = inboundBuffer.poll()) != null); + inboundBuffer != null && (message = inboundBuffer.poll()) != null); if (continueReading && isParentReadInProgress() && !readEOS) { // Currently the parent and child channel are on the same EventLoop thread. If the parent is 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 a93c03734e..1abaf00938 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 @@ -303,6 +303,42 @@ public abstract class Http2MultiplexTest { verifyFramesMultiplexedToCorrectChannel(childChannel, inboundHandler, 2); } + @Test + public void channelReadShouldRespectAutoReadAndNotProduceNPE() throws Exception { + LastInboundHandler inboundHandler = new LastInboundHandler(); + Http2StreamChannel childChannel = newInboundStream(3, false, inboundHandler); + assertTrue(childChannel.config().isAutoRead()); + Http2HeadersFrame headersFrame = inboundHandler.readInbound(); + assertNotNull(headersFrame); + + childChannel.config().setAutoRead(false); + childChannel.pipeline().addFirst(new ChannelInboundHandlerAdapter() { + private int count; + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + ctx.fireChannelRead(msg); + // Close channel after 2 reads so there is still something in the inboundBuffer when the close happens. + if (++count == 2) { + ctx.close(); + } + } + }); + frameInboundWriter.writeInboundData(childChannel.stream().id(), bb("hello world"), 0, false); + Http2DataFrame dataFrame0 = inboundHandler.readInbound(); + assertNotNull(dataFrame0); + release(dataFrame0); + + frameInboundWriter.writeInboundData(childChannel.stream().id(), bb("foo"), 0, false); + frameInboundWriter.writeInboundData(childChannel.stream().id(), bb("bar"), 0, false); + frameInboundWriter.writeInboundData(childChannel.stream().id(), bb("bar"), 0, false); + + assertNull(inboundHandler.readInbound()); + + childChannel.config().setAutoRead(true); + verifyFramesMultiplexedToCorrectChannel(childChannel, inboundHandler, 3); + inboundHandler.checkException(); + } + @Test public void readInChannelReadWithoutAutoRead() { useReadWithoutAutoRead(false);