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

Correctly handle read-only ByteBuf in ByteToMessageDecoder

Browse Source 
Motivation:

If a read-only ByteBuf is passed to the ByteToMessageDecoder.channelRead(...) method we need to make a copy of it once we try to merge buffers for cumulation. This usually is not the case but can for example happen if the local transport is used. This was the cause of the leak report we sometimes saw during the codec-http2 tests, as we are using the local transport and write a read-only buffer. This buffer will then be passed to the peer channel and fired through the pipeline and so end up as the cumulation buffer in the ByteToMessageDecoder. Once the next fragement is received we tried to merge these and failed with a ReadOnlyBufferException which then produced a leak.

Modifications:

Ensure we copy the buffer if its read-only.

Result:

No more exceptions and so leak when a read-only buffer is passed to ByteToMessageDecoder.channelRead(...)
This commit is contained in:
Norman Maurer 2017-04-18 20:51:11 +02:00
parent 4436d697a6
commit 38b054c65c
2 changed files with 15 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java
View File

@ -75,12 +75,12 @@ public abstract class ByteToMessageDecoder extends ChannelInboundHandlerAdapter
public static final Cumulator MERGE_CUMULATOR = new Cumulator() {
@Override
public ByteBuf cumulate(ByteBufAllocator alloc, ByteBuf cumulation, ByteBuf in) {
ByteBuf buffer;
final ByteBuf buffer;
if (cumulation.writerIndex() > cumulation.maxCapacity() - in.readableBytes()
|| cumulation.refCnt() > 1) {
|| cumulation.refCnt() > 1 || cumulation.isReadOnly()) {
// Expand cumulation (by replace it) when either there is not more room in the buffer
// or if the refCnt is greater then 1 which may happen when the user use slice().retain() or
// duplicate().retain().
// duplicate().retain() or if its read-only.
//
// See:
// - https://github.com/netty/netty/issues/2327

12
codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java
View File

@ -266,4 +266,16 @@ public class ByteToMessageDecoderTest {
expected.release();
}
}
@Test
public void testReadOnlyBuffer() {
EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() {
@Override
protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
}
});
assertFalse(channel.writeInbound(Unpooled.buffer(8).writeByte(1).asReadOnly()));
assertFalse(channel.writeInbound(Unpooled.wrappedBuffer(new byte[] { (byte) 2 })));
assertFalse(channel.finish());
}
}