Loosen bounds check on CompositeByteBuf's maxNumComponents (#8621)

Motivation:

In versions of Netty prior to 4.1.31.Final, a CompositeByteBuf could be
created with any size (including potentially nonsensical negative
values). This behavior changed in e7737b993, which introduced a bounds
check to only allow for a component size greater than one. This broke
some existing use cases that attempted to create a byte buf with a
single component.

Modifications:

Lower the bounds check on numComponents to include the single component
case, but still throw an exception for anything less than one.

Add unit tests for the case of numComponents being less than, equal to,
and greater than this lower bound.

Result:

Return to the behavior of 4.1.30.Final, allowing one component, but
still include an explicit check against a lower bound.

Note that while creating a CompositeByteBuf with a single component is
in some ways a contradiction of the term "composite", this patch caters
for existing uses while excluding the clearly nonsensical case of asking
for a CompositeByteBuf with zero or fewer components.

Fixes #8613.
This commit is contained in:
Nick Travers 2018-12-04 23:42:23 -08:00 committed by Norman Maurer
parent b8a3394f9b
commit d0d30f1fbe
2 changed files with 35 additions and 2 deletions

View File

@ -64,9 +64,9 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
if (alloc == null) { if (alloc == null) {
throw new NullPointerException("alloc"); throw new NullPointerException("alloc");
} }
if (maxNumComponents < 2) { if (maxNumComponents < 1) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"maxNumComponents: " + maxNumComponents + " (expected: >= 2)"); "maxNumComponents: " + maxNumComponents + " (expected: >= 1)");
} }
this.alloc = alloc; this.alloc = alloc;
this.direct = direct; this.direct = direct;

View File

@ -52,6 +52,8 @@ import static org.junit.Assert.fail;
*/ */
public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
private static final ByteBufAllocator ALLOC = UnpooledByteBufAllocator.DEFAULT;
private final ByteOrder order; private final ByteOrder order;
protected AbstractCompositeByteBufTest(ByteOrder order) { protected AbstractCompositeByteBufTest(ByteOrder order) {
@ -1262,4 +1264,35 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
} }
} }
@Test
public void testComponentsLessThanLowerBound() {
try {
new CompositeByteBuf(ALLOC, true, 0);
fail();
} catch (IllegalArgumentException e) {
assertEquals("maxNumComponents: 0 (expected: >= 1)", e.getMessage());
}
}
@Test
public void testComponentsEqualToLowerBound() {
assertCompositeBufCreated(1);
}
@Test
public void testComponentsGreaterThanLowerBound() {
assertCompositeBufCreated(5);
}
/**
* Assert that a new {@linkplain CompositeByteBuf} was created successfully with the desired number of max
* components.
*/
private static void assertCompositeBufCreated(int expectedMaxComponents) {
CompositeByteBuf buf = new CompositeByteBuf(ALLOC, true, expectedMaxComponents);
assertEquals(expectedMaxComponents, buf.maxNumComponents());
assertTrue(buf.release());
}
} }