PooledByteBuf.capacity(...) not enforces maxCapacity()

Motivation:

PooledByteBuf.capacity(...) miss to enforce maxCapacity() and so its possible to increase the capacity of the buffer even if it will be bigger then maxCapacity().

Modifications:

- Correctly enforce maxCapacity()
- Add unit tests for capacity(...) calls.

Result:

Correctly enforce maxCapacity().

buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java

@@ -1383,6 +1383,13 @@ public abstract class AbstractByteBuf extends ByteBuf {
         checkReadableBytes0(minimumReadableBytes);
     }
 
+    protected final void checkNewCapacity(int newCapacity) {
+        ensureAccessible();
+        if (newCapacity < 0 || newCapacity > maxCapacity()) {
+            throw new IllegalArgumentException("newCapacity: " + newCapacity + " (expected: 0-" + maxCapacity() + ')');
+        }
+    }
+
     private void checkReadableBytes0(int minimumReadableBytes) {
         ensureAccessible();
         if (readerIndex > writerIndex - minimumReadableBytes) {

buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java

@@ -642,10 +642,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
 
     @Override
     public CompositeByteBuf capacity(int newCapacity) {
-        ensureAccessible();
-        if (newCapacity < 0 || newCapacity > maxCapacity()) {
-            throw new IllegalArgumentException("newCapacity: " + newCapacity);
-        }
+        checkNewCapacity(newCapacity);
 
         int oldCapacity = capacity();
         if (newCapacity > oldCapacity) {

buffer/src/main/java/io/netty/buffer/PooledByteBuf.java

@@ -86,7 +86,7 @@ abstract class PooledByteBuf<T> extends AbstractReferenceCountedByteBuf {
 
     @Override
     public final ByteBuf capacity(int newCapacity) {
-        ensureAccessible();
+        checkNewCapacity(newCapacity);
 
         // If the request capacity does not require reallocation, just update the length of the memory.
         if (chunk.unpooled) {

buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java

@@ -140,10 +140,7 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public ByteBuf capacity(int newCapacity) {
-        ensureAccessible();
-        if (newCapacity < 0 || newCapacity > maxCapacity()) {
-            throw new IllegalArgumentException("newCapacity: " + newCapacity);
-        }
+        checkNewCapacity(newCapacity);
 
         int readerIndex = readerIndex();
         int writerIndex = writerIndex();

buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java

@@ -105,10 +105,7 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public ByteBuf capacity(int newCapacity) {
-        ensureAccessible();
-        if (newCapacity < 0 || newCapacity > maxCapacity()) {
-            throw new IllegalArgumentException("newCapacity: " + newCapacity);
-        }
+        checkNewCapacity(newCapacity);
 
         int oldCapacity = array.length;
         if (newCapacity > oldCapacity) {

buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java

@@ -147,10 +147,7 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf
 
     @Override
     public ByteBuf capacity(int newCapacity) {
-        ensureAccessible();
-        if (newCapacity < 0 || newCapacity > maxCapacity()) {
-            throw new IllegalArgumentException("newCapacity: " + newCapacity);
-        }
+        checkNewCapacity(newCapacity);
 
         int readerIndex = readerIndex();
         int writerIndex = writerIndex();

buffer/src/main/java/io/netty/buffer/UnpooledUnsafeNoCleanerDirectByteBuf.java

@@ -37,10 +37,7 @@ final class UnpooledUnsafeNoCleanerDirectByteBuf extends UnpooledUnsafeDirectByt
 
     @Override
     public ByteBuf capacity(int newCapacity) {
-        ensureAccessible();
-        if (newCapacity < 0 || newCapacity > maxCapacity()) {
-            throw new IllegalArgumentException("newCapacity: " + newCapacity);
-        }
+        checkNewCapacity(newCapacity);
 
         int readerIndex = readerIndex();
         int writerIndex = writerIndex();

buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java

@@ -78,7 +78,11 @@ public abstract class AbstractByteBufTest {
     private Random random;
     private ByteBuf buffer;
 
-    protected abstract ByteBuf newBuffer(int capacity);
+    protected final ByteBuf newBuffer(int capacity) {
+        return newBuffer(capacity, Integer.MAX_VALUE);
+    }
+
+    protected abstract ByteBuf newBuffer(int capacity, int maxCapacity);
 
     protected boolean discardReadBytesDoesNotMoveWritableBytes() {
         return true;
@@ -4022,4 +4026,56 @@ public abstract class AbstractByteBufTest {
             return true;
         }
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCapacityEnforceMaxCapacity() {
+        ByteBuf buffer = newBuffer(3, 13);
+        assertEquals(13, buffer.maxCapacity());
+        assertEquals(3, buffer.capacity());
+        try {
+            buffer.capacity(14);
+        } finally {
+            buffer.release();
+        }
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCapacityNegative() {
+        ByteBuf buffer = newBuffer(3, 13);
+        assertEquals(13, buffer.maxCapacity());
+        assertEquals(3, buffer.capacity());
+        try {
+            buffer.capacity(-1);
+        } finally {
+            buffer.release();
+        }
+    }
+
+    @Test
+    public void testCapacityDecrease() {
+        ByteBuf buffer = newBuffer(3, 13);
+        assertEquals(13, buffer.maxCapacity());
+        assertEquals(3, buffer.capacity());
+        try {
+            buffer.capacity(2);
+            assertEquals(2, buffer.capacity());
+            assertEquals(13, buffer.maxCapacity());
+        } finally {
+            buffer.release();
+        }
+    }
+
+    @Test
+    public void testCapacityIncrease() {
+        ByteBuf buffer = newBuffer(3, 13);
+        assertEquals(13, buffer.maxCapacity());
+        assertEquals(3, buffer.capacity());
+        try {
+            buffer.capacity(4);
+            assertEquals(4, buffer.capacity());
+            assertEquals(13, buffer.maxCapacity());
+        } finally {
+            buffer.release();
+        }
+    }
 }

buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java

@@ -16,6 +16,7 @@
 package io.netty.buffer;
 
 import io.netty.util.ReferenceCountUtil;
+import org.junit.Assume;
 import org.junit.Test;
 
 import java.nio.ByteBuffer;
@@ -60,7 +61,9 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        Assume.assumeTrue(maxCapacity == Integer.MAX_VALUE);
+
         List<ByteBuf> buffers = new ArrayList<ByteBuf>();
         for (int i = 0; i < length + 45; i += 45) {
             buffers.add(EMPTY_BUFFER);

buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java

@@ -19,11 +19,11 @@ import static org.junit.Assert.*;
 
 public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest {
 
-    protected abstract ByteBuf alloc(int length);
+    protected abstract ByteBuf alloc(int length, int maxCapacity);
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf buffer = alloc(length);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf buffer = alloc(length, maxCapacity);
 
         // Testing if the writerIndex and readerIndex are correct when allocate and also after we reset the mark.
         assertEquals(0, buffer.writerIndex());

buffer/src/test/java/io/netty/buffer/BigEndianDirectByteBufTest.java

@@ -25,14 +25,14 @@ import java.nio.ByteOrder;
 public class BigEndianDirectByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf buffer = newDirectBuffer(length);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf buffer = newDirectBuffer(length, maxCapacity);
         assertSame(ByteOrder.BIG_ENDIAN, buffer.order());
         assertEquals(0, buffer.writerIndex());
         return buffer;
     }
 
-    protected ByteBuf newDirectBuffer(int length) {
-        return new UnpooledDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newDirectBuffer(int length, int maxCapacity) {
+        return new UnpooledDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/BigEndianHeapByteBufTest.java

@@ -25,8 +25,8 @@ import static org.junit.Assert.*;
 public class BigEndianHeapByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf buffer = Unpooled.buffer(length);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf buffer = Unpooled.buffer(length, maxCapacity);
         assertEquals(0, buffer.writerIndex());
         return buffer;
     }

buffer/src/test/java/io/netty/buffer/BigEndianUnsafeDirectByteBufTest.java

@@ -30,7 +30,7 @@ public class BigEndianUnsafeDirectByteBufTest extends BigEndianDirectByteBufTest
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        return new UnpooledUnsafeDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        return new UnpooledUnsafeDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/BigEndianUnsafeNoCleanerDirectByteBufTest.java

@@ -31,7 +31,7 @@ public class BigEndianUnsafeNoCleanerDirectByteBufTest extends BigEndianDirectBy
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        return new UnpooledUnsafeNoCleanerDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        return new UnpooledUnsafeNoCleanerDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/DuplicatedByteBufTest.java

@@ -25,8 +25,8 @@ import static org.junit.Assert.assertEquals;
 public class DuplicatedByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf wrapped = Unpooled.buffer(length);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf wrapped = Unpooled.buffer(length, maxCapacity);
         ByteBuf buffer = new DuplicatedByteBuf(wrapped);
         assertEquals(wrapped.writerIndex(), buffer.writerIndex());
         assertEquals(wrapped.readerIndex(), buffer.readerIndex());

buffer/src/test/java/io/netty/buffer/LittleEndianDirectByteBufTest.java

@@ -25,14 +25,14 @@ import java.nio.ByteOrder;
 public class LittleEndianDirectByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf buffer = newDirectBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf buffer = newDirectBuffer(length, maxCapacity).order(ByteOrder.LITTLE_ENDIAN);
         assertSame(ByteOrder.LITTLE_ENDIAN, buffer.order());
         assertEquals(0, buffer.writerIndex());
         return buffer;
     }
 
-    protected ByteBuf newDirectBuffer(int length) {
-        return new UnpooledDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newDirectBuffer(int length, int maxCapacity) {
+        return new UnpooledDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/LittleEndianHeapByteBufTest.java

@@ -25,8 +25,8 @@ import java.nio.ByteOrder;
 public class LittleEndianHeapByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf buffer = Unpooled.buffer(length).order(ByteOrder.LITTLE_ENDIAN);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf buffer = Unpooled.buffer(length, maxCapacity).order(ByteOrder.LITTLE_ENDIAN);
         assertEquals(0, buffer.writerIndex());
         return buffer;
     }

buffer/src/test/java/io/netty/buffer/LittleEndianUnsafeDirectByteBufTest.java

@@ -29,7 +29,7 @@ public class LittleEndianUnsafeDirectByteBufTest extends LittleEndianDirectByteB
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        return new UnpooledUnsafeDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        return new UnpooledUnsafeDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/LittleEndianUnsafeNoCleanerDirectByteBufTest.java

@@ -30,7 +30,7 @@ public class LittleEndianUnsafeNoCleanerDirectByteBufTest extends LittleEndianDi
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        return new UnpooledUnsafeNoCleanerDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, Integer.MAX_VALUE);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        return new UnpooledUnsafeNoCleanerDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/PooledBigEndianDirectByteBufTest.java

@@ -25,8 +25,8 @@ import static org.junit.Assert.*;
 public class PooledBigEndianDirectByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf alloc(int length) {
-        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length);
+    protected ByteBuf alloc(int length, int maxCapacity) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length, maxCapacity);
         assertSame(ByteOrder.BIG_ENDIAN, buffer.order());
         return buffer;
     }

buffer/src/test/java/io/netty/buffer/PooledBigEndianHeapByteBufTest.java

@@ -21,7 +21,7 @@ package io.netty.buffer;
 public class PooledBigEndianHeapByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf alloc(int length) {
-        return PooledByteBufAllocator.DEFAULT.heapBuffer(length);
+    protected ByteBuf alloc(int length, int maxCapacity) {
+        return PooledByteBufAllocator.DEFAULT.heapBuffer(length, maxCapacity);
     }
 }

buffer/src/test/java/io/netty/buffer/PooledLittleEndianDirectByteBufTest.java

@@ -25,8 +25,9 @@ import static org.junit.Assert.*;
 public class PooledLittleEndianDirectByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf alloc(int length) {
-        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
+    protected ByteBuf alloc(int length, int maxCapacity) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length, maxCapacity)
+                .order(ByteOrder.LITTLE_ENDIAN);
         assertSame(ByteOrder.LITTLE_ENDIAN, buffer.order());
         return buffer;
     }

buffer/src/test/java/io/netty/buffer/PooledLittleEndianHeapByteBufTest.java

@@ -25,8 +25,8 @@ import static org.junit.Assert.*;
 public class PooledLittleEndianHeapByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf alloc(int length) {
-        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.heapBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
+    protected ByteBuf alloc(int length, int maxCapacity) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.heapBuffer(length, maxCapacity).order(ByteOrder.LITTLE_ENDIAN);
         assertSame(ByteOrder.LITTLE_ENDIAN, buffer.order());
         return buffer;
     }

buffer/src/test/java/io/netty/buffer/RetainedDuplicatedByteBufTest.java

@@ -20,8 +20,8 @@ import static org.junit.Assert.assertEquals;
 
 public class RetainedDuplicatedByteBufTest extends DuplicatedByteBufTest {
     @Override
-    protected ByteBuf newBuffer(int length) {
-        ByteBuf wrapped = Unpooled.buffer(length);
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        ByteBuf wrapped = Unpooled.buffer(length, maxCapacity);
         ByteBuf buffer = wrapped.retainedDuplicate();
         wrapped.release();
 

buffer/src/test/java/io/netty/buffer/RetainedSlicedByteBufTest.java

@@ -17,12 +17,14 @@
 package io.netty.buffer;
 
 import io.netty.util.internal.ThreadLocalRandom;
+import org.junit.Assume;
 
 import static org.junit.Assert.assertEquals;
 
 public class RetainedSlicedByteBufTest extends SlicedByteBufTest {
     @Override
-    protected ByteBuf newBuffer(int length) {
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        Assume.assumeTrue(maxCapacity == Integer.MAX_VALUE);
         ByteBuf wrapped = Unpooled.wrappedBuffer(new byte[length * 2]);
         ByteBuf buffer = wrapped.retainedSlice(length > 1 ? ThreadLocalRandom.current().nextInt(length - 1) + 1 : 0,
                                                length);

buffer/src/test/java/io/netty/buffer/SimpleLeakAwareByteBufTest.java

@@ -31,8 +31,8 @@ public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
     private final Queue<NoopResourceLeakTracker<ByteBuf>> trackers = new ArrayDeque<NoopResourceLeakTracker<ByteBuf>>();
 
     @Override
-    protected final ByteBuf newBuffer(int capacity) {
-        return wrap(super.newBuffer(capacity));
+    protected final ByteBuf newBuffer(int capacity, int maxCapacity) {
+        return wrap(super.newBuffer(capacity, maxCapacity));
     }
 
     private ByteBuf wrap(ByteBuf buffer) {

buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java

@@ -16,6 +16,7 @@
 package io.netty.buffer;
 
 import io.netty.util.internal.ThreadLocalRandom;
+import org.junit.Assume;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -30,7 +31,8 @@ import static org.junit.Assert.*;
 public class SlicedByteBufTest extends AbstractByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        Assume.assumeTrue(maxCapacity == Integer.MAX_VALUE);
         ByteBuf buffer = Unpooled.wrappedBuffer(
                 new byte[length * 2], length > 1 ? ThreadLocalRandom.current().nextInt(length - 1) + 1 : 0, length);
         assertEquals(0, buffer.readerIndex());

buffer/src/test/java/io/netty/buffer/WrappedCompositeByteBufTest.java

@@ -18,8 +18,8 @@ package io.netty.buffer;
 public class WrappedCompositeByteBufTest extends BigEndianCompositeByteBufTest {
 
     @Override
-    protected final ByteBuf newBuffer(int length) {
-        return wrap((CompositeByteBuf) super.newBuffer(length));
+    protected final ByteBuf newBuffer(int length, int maxCapacity) {
+        return wrap((CompositeByteBuf) super.newBuffer(length, maxCapacity));
     }
 
     protected WrappedCompositeByteBuf wrap(CompositeByteBuf buffer) {

buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java

@@ -32,7 +32,9 @@ public class WrappedUnpooledUnsafeByteBufTest extends BigEndianUnsafeDirectByteB
     }
 
     @Override
-    protected ByteBuf newBuffer(int length) {
+    protected ByteBuf newBuffer(int length, int maxCapacity) {
+        Assume.assumeTrue(maxCapacity == Integer.MAX_VALUE);
+
         return new WrappedUnpooledUnsafeDirectByteBuf(UnpooledByteBufAllocator.DEFAULT,
                 PlatformDependent.allocateMemory(length), length, true);
     }