From d0d30f1fbe2117cd0a7b2ad103cd3ff596a2d7f9 Mon Sep 17 00:00:00 2001 From: Nick Travers Date: Tue, 4 Dec 2018 23:42:23 -0800 Subject: [PATCH] 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. --- .../io/netty/buffer/CompositeByteBuf.java | 4 +-- .../buffer/AbstractCompositeByteBufTest.java | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java index c9212d5c6e..ce3c9f7f41 100644 --- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java @@ -64,9 +64,9 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements if (alloc == null) { throw new NullPointerException("alloc"); } - if (maxNumComponents < 2) { + if (maxNumComponents < 1) { throw new IllegalArgumentException( - "maxNumComponents: " + maxNumComponents + " (expected: >= 2)"); + "maxNumComponents: " + maxNumComponents + " (expected: >= 1)"); } this.alloc = alloc; this.direct = direct; diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java index 8e0f148cdf..c51a991ec3 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java @@ -52,6 +52,8 @@ import static org.junit.Assert.fail; */ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { + private static final ByteBufAllocator ALLOC = UnpooledByteBufAllocator.DEFAULT; + private final 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()); + } + }