Fix NPE in ByteToMessageDecoder if the user removes the handler while channelInputClosed(...) is processing the buffer. (#10817)

Motivation:

We need to carefully check for null before we pass the cumulation buffer into decodeLast as callDecode(...) may have removed the codec already and so set cumulation to null.

Modifications:

- Check for null and if we see null use Unpooled.EMPTY_BUFFEr
- Only call decodeLast(...) if callDecode(...) didnt remove the handler yet.

Result:

Fixes https://github.com/netty/netty/issues/10802
This commit is contained in:
Norman Maurer 2020-11-24 14:08:32 +01:00 committed by GitHub
parent 02cd85181a
commit 9cfe3bf5e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 1 deletions

View File

@ -402,7 +402,14 @@ public abstract class ByteToMessageDecoder extends ChannelInboundHandlerAdapter
void channelInputClosed(ChannelHandlerContext ctx, List<Object> out) throws Exception {
if (cumulation != null) {
callDecode(ctx, cumulation, out);
decodeLast(ctx, cumulation, out);
// If callDecode(...) removed the handle from the pipeline we should not call decodeLast(...) as this would
// be unexpected.
if (!ctx.isRemoved()) {
// Use Unpooled.EMPTY_BUFFER if cumulation become null after calling callDecode(...).
// See https://github.com/netty/netty/issues/10802.
ByteBuf buffer = cumulation == null ? Unpooled.EMPTY_BUFFER : cumulation;
decodeLast(ctx, buffer, out);
}
} else {
decodeLast(ctx, Unpooled.EMPTY_BUFFER, out);
}

View File

@ -26,12 +26,14 @@ import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelOutboundHandlerAdapter;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.channel.socket.ChannelInputShutdownEvent;
import io.netty.util.internal.PlatformDependent;
import org.junit.Test;
import java.util.List;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.atomic.AtomicBoolean;
import static io.netty.buffer.Unpooled.wrappedBuffer;
import static org.junit.Assert.assertEquals;
@ -510,4 +512,30 @@ public class ByteToMessageDecoderTest {
assertTrue(buffer5.release());
assertFalse(channel.finish());
}
@Test
public void testDecodeLast() {
final AtomicBoolean removeHandler = new AtomicBoolean();
EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() {
@Override
protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) {
if (removeHandler.get()) {
ctx.pipeline().remove(this);
}
}
});
byte[] bytes = new byte[1024];
PlatformDependent.threadLocalRandom().nextBytes(bytes);
assertFalse(channel.writeInbound(Unpooled.copiedBuffer(bytes)));
assertNull(channel.readInbound());
removeHandler.set(true);
// This should trigger channelInputClosed(...)
channel.pipeline().fireUserEventTriggered(ChannelInputShutdownEvent.INSTANCE);
assertTrue(channel.finish());
assertBuffer(Unpooled.wrappedBuffer(bytes), (ByteBuf) channel.readInbound());
assertNull(channel.readInbound());
}
}