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

Correctly handle WrappedCompositeByteBufs in addFlattenedComponents() (#10247)

Browse Source 
Motivation

An NPE was reported in #10245, caused by a regression introduced in
#8939. This in particular affects ByteToMessageDecoders that use the
COMPOSITE_CUMULATOR.

Modification

- Unwrap WrappedCompositeByteBufs passed to
CompositeByteBuf#addFlattenedComponents(...) method before accessing
internal components field
- Extend unit test to cover this case and ensure more of the
CompositeByteBuf tests are also run on the wrapped variant

Results

Fixes #10245
This commit is contained in:
Nick Hill 2020-05-05 04:56:40 -07:00 committed by GitHub
parent cfcd7a4fde
commit c3db0391af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 54 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
View File

@ -473,7 +473,12 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
consolidateIfNeeded();
return this;
}
final CompositeByteBuf from = (CompositeByteBuf) buffer;
final CompositeByteBuf from;
if (buffer instanceof WrappedCompositeByteBuf) {
from = (CompositeByteBuf) buffer.unwrap();
} else {
from = (CompositeByteBuf) buffer;
}
from.checkIndex(ridx, widx - ridx);
final Component[] fromComponents = from.components;
final int compCountBefore = componentCount;

58
buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
View File

@ -24,7 +24,6 @@ import org.junit.Test;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
@ -89,7 +88,20 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
buffers.add(EMPTY_BUFFER);
}
ByteBuf buffer = wrappedBuffer(Integer.MAX_VALUE, buffers.toArray(new ByteBuf[0])).order(order);
ByteBuf buffer;
// Ensure that we are really testing a CompositeByteBuf
switch (buffers.size()) {
case 0:
buffer = compositeBuffer(Integer.MAX_VALUE);
break;
case 1:
buffer = compositeBuffer(Integer.MAX_VALUE).addComponent(buffers.get(0));
break;
default:
buffer = wrappedBuffer(Integer.MAX_VALUE, buffers.toArray(new ByteBuf[0]));
break;
}
buffer = buffer.order(order);
// Truncate to the requested capacity.
buffer.capacity(length);
@ -101,6 +113,10 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
return buffer;
}
protected CompositeByteBuf newCompositeBuffer() {
return compositeBuffer();
}
// Composite buffer does not waste bandwidth on discardReadBytes, but
// the test will fail in strict mode.
@Override
@ -1124,8 +1140,17 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
@Test
public void testAddFlattenedComponents() {
testAddFlattenedComponents(false);
}
@Test
public void testAddFlattenedComponentsWithWrappedComposite() {
testAddFlattenedComponents(true);
}
private void testAddFlattenedComponents(boolean addWrapped) {
ByteBuf b1 = Unpooled.wrappedBuffer(new byte[] { 1, 2, 3 });
CompositeByteBuf newComposite = Unpooled.compositeBuffer()
CompositeByteBuf newComposite = newCompositeBuffer()
.addComponent(true, b1)
.addFlattenedComponents(true, b1.retain())
.addFlattenedComponents(true, Unpooled.EMPTY_BUFFER);
@ -1148,7 +1173,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
ByteBuf s4 = s2.retainedSlice(0, 2);
buffer.release();
ByteBuf compositeToAdd = Unpooled.compositeBuffer()
CompositeByteBuf compositeToAdd = compositeBuffer()
.addComponent(s1)
.addComponent(Unpooled.EMPTY_BUFFER)
.addComponents(s2, s3, s4);
@ -1161,6 +1186,9 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
ByteBuf compositeCopy = compositeToAdd.copy();
if (addWrapped) {
compositeToAdd = new WrappedCompositeByteBuf(compositeToAdd);
}
newComposite.addFlattenedComponents(true, compositeToAdd);
// verify that added range matches
@ -1189,7 +1217,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
@Test
public void testIterator() {
CompositeByteBuf cbuf = compositeBuffer();
CompositeByteBuf cbuf = newCompositeBuffer();
cbuf.addComponent(EMPTY_BUFFER);
cbuf.addComponent(EMPTY_BUFFER);
@ -1211,7 +1239,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
@Test
public void testEmptyIterator() {
CompositeByteBuf cbuf = compositeBuffer();
CompositeByteBuf cbuf = newCompositeBuffer();
Iterator<ByteBuf> it = cbuf.iterator();
assertFalse(it.hasNext());
@ -1227,7 +1255,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
@Test(expected = ConcurrentModificationException.class)
public void testIteratorConcurrentModificationAdd() {
CompositeByteBuf cbuf = compositeBuffer();
CompositeByteBuf cbuf = newCompositeBuffer();
cbuf.addComponent(EMPTY_BUFFER);
Iterator<ByteBuf> it = cbuf.iterator();
@ -1243,7 +1271,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
@Test(expected = ConcurrentModificationException.class)
public void testIteratorConcurrentModificationRemove() {
CompositeByteBuf cbuf = compositeBuffer();
CompositeByteBuf cbuf = newCompositeBuffer();
cbuf.addComponent(EMPTY_BUFFER);
Iterator<ByteBuf> it = cbuf.iterator();
@ -1302,7 +1330,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
ByteBuf s3 = s2.readRetainedSlice(2); // 4
ByteBuf s4 = s3.readRetainedSlice(2); // 5
ByteBuf composite = Unpooled.compositeBuffer()
ByteBuf composite = newCompositeBuffer()
.addComponent(s1)
.addComponents(s2, s3, s4)
.order(ByteOrder.LITTLE_ENDIAN);
@ -1327,7 +1355,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
ByteBuf b2 = Unpooled.buffer(2).writeShort(2);
// composite takes ownership of s1 and s2
ByteBuf composite = Unpooled.compositeBuffer()
ByteBuf composite = newCompositeBuffer()
.addComponents(b1, b2);
assertEquals(4, composite.capacity());
@ -1357,7 +1385,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
ByteBuf b2 = b1.retainedSlice(b1.readerIndex(), 2);
// composite takes ownership of b1 and b2
ByteBuf composite = Unpooled.compositeBuffer()
ByteBuf composite = newCompositeBuffer()
.addComponents(b1, b2);
assertEquals(4, composite.capacity());
@ -1413,12 +1441,12 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
testDecompose(310, 0, 0);
}
private static void testDecompose(int offset, int length, int expectedListSize) {
private void testDecompose(int offset, int length, int expectedListSize) {
byte[] bytes = new byte[1024];
PlatformDependent.threadLocalRandom().nextBytes(bytes);
ByteBuf buf = wrappedBuffer(bytes);
CompositeByteBuf composite = compositeBuffer();
CompositeByteBuf composite = newCompositeBuffer();
composite.addComponents(true,
buf.retainedSlice(100, 200),
buf.retainedSlice(300, 400),
@ -1479,8 +1507,8 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
testDiscardCorrectlyUpdatesLastAccessed(false);
}
private static void testDiscardCorrectlyUpdatesLastAccessed(boolean discardSome) {
CompositeByteBuf cbuf = compositeBuffer();
private void testDiscardCorrectlyUpdatesLastAccessed(boolean discardSome) {
CompositeByteBuf cbuf = newCompositeBuffer();
List<ByteBuf> buffers = new ArrayList<ByteBuf>(4);
for (int i = 0; i < 4; i++) {
ByteBuf buf = buffer().writeInt(i);

5
buffer/src/test/java/io/netty/buffer/WrappedCompositeByteBufTest.java
View File

@ -25,4 +25,9 @@ public class WrappedCompositeByteBufTest extends BigEndianCompositeByteBufTest {
protected WrappedCompositeByteBuf wrap(CompositeByteBuf buffer) {
return new WrappedCompositeByteBuf(buffer);
}
@Override
protected CompositeByteBuf newCompositeBuffer() {
return wrap(super.newCompositeBuffer());
}
}