findNextPositivePowerOfTwo out of bounds

Motivation:
Some usages of findNextPositivePowerOfTwo assume that bounds checking is taken care of by this method. However bounds checking is not taken care of by findNextPositivePowerOfTwo and instead assert statements are used to imply the caller has checked the bounds. This can lead to unexpected non power of 2 return values if the caller is not careful and thus invalidate any logic which depends upon a power of 2.

Modifications:
- Add a safeFindNextPositivePowerOfTwo method which will do runtime bounds checks and always return a power of 2

Result:
Fixes https://github.com/netty/netty/issues/5601

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

@@ -369,7 +369,7 @@ final class PoolThreadCache {
         private int allocations;
 
         MemoryRegionCache(int size, SizeClass sizeClass) {
-            this.size = MathUtil.findNextPositivePowerOfTwo(size);
+            this.size = MathUtil.safeFindNextPositivePowerOfTwo(size);
             queue = PlatformDependent.newFixedMpscQueue(this.size);
             this.sizeClass = sizeClass;
         }

common/src/main/java/io/netty/util/Recycler.java

@@ -27,7 +27,7 @@ import java.util.Map;
 import java.util.WeakHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
+import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;
 import static java.lang.Math.max;
 import static java.lang.Math.min;
 
@@ -71,14 +71,13 @@ public abstract class Recycler<T> {
                 SystemPropertyUtil.getInt("io.netty.recycler.maxSharedCapacityFactor",
                         2));
 
-        LINK_CAPACITY = findNextPositivePowerOfTwo(
+        LINK_CAPACITY = safeFindNextPositivePowerOfTwo(
                 max(SystemPropertyUtil.getInt("io.netty.recycler.linkCapacity", 16), 16));
 
         // By default we allow one push to a Recycler for each 8th try on handles that were never recycled before.
         // This should help to slowly increase the capacity of the recycler while not be too sensitive to allocation
         // bursts.
-        RATIO = min(findNextPositivePowerOfTwo(
-                max(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8), 2)), 0x40000000);
+        RATIO = safeFindNextPositivePowerOfTwo(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8));
 
         if (logger.isDebugEnabled()) {
             if (DEFAULT_MAX_CAPACITY == 0) {
@@ -121,10 +120,7 @@ public abstract class Recycler<T> {
     }
 
     protected Recycler(int maxCapacity, int maxSharedCapacityFactor, int ratio) {
-        if (ratio > 0x40000000) {
-            throw new IllegalArgumentException(ratio + ": " + ratio + " (expected: < 0x40000000)");
-        }
-        ratioMask = findNextPositivePowerOfTwo(ratio) - 1;
+        ratioMask = safeFindNextPositivePowerOfTwo(ratio) - 1;
         if (maxCapacity <= 0) {
             this.maxCapacity = 0;
             this.maxSharedCapacityFactor = 1;

common/src/main/java/io/netty/util/ResourceLeakDetector.java

@@ -193,15 +193,12 @@ public class ResourceLeakDetector<T> {
         if (resourceType == null) {
             throw new NullPointerException("resourceType");
         }
-        if (samplingInterval <= 0) {
-            throw new IllegalArgumentException("samplingInterval: " + samplingInterval + " (expected: 1+)");
-        }
         if (maxActive <= 0) {
             throw new IllegalArgumentException("maxActive: " + maxActive + " (expected: 1+)");
         }
 
         this.resourceType = resourceType;
-        this.samplingInterval = MathUtil.findNextPositivePowerOfTwo(samplingInterval);
+        this.samplingInterval = MathUtil.safeFindNextPositivePowerOfTwo(samplingInterval);
         // samplingInterval is a power of two so we calculate a mask that we can use to
         // check if we need to do any leak detection or not.
         mask = this.samplingInterval - 1;

common/src/main/java/io/netty/util/internal/MathUtil.java

@@ -25,7 +25,7 @@ public final class MathUtil {
     /**
      * Fast method of finding the next power of 2 greater than or equal to the supplied value.
      *
-     * If the value is {@code <= 0} then 1 will be returned.
+     * <p>If the value is {@code <= 0} then 1 will be returned.
      * This method is not suitable for {@link Integer#MIN_VALUE} or numbers greater than 2^30.
      *
      * @param value from which to search for next power of 2
@@ -36,6 +36,22 @@ public final class MathUtil {
         return 1 << (32 - Integer.numberOfLeadingZeros(value - 1));
     }
 
+    /**
+     * Fast method of finding the next power of 2 greater than or equal to the supplied value.
+     * <p>This method will do runtime bounds checking and call {@link #findNextPositivePowerOfTwo(int)} if within a
+     * valid range.
+     * @param value from which to search for next power of 2
+     * @return The next power of 2 or the value itself if it is a power of 2.
+     * <p>Special cases for return values are as follows:
+     * <ul>
+     *     <li>{@code <= 0} -> 1</li>
+     *     <li>{@code >= 2^30} -> 2^30</li>
+     * </ul>
+     */
+    public static int safeFindNextPositivePowerOfTwo(final int value) {
+        return value <= 0 ? 1 : value >= 0x40000000 ? 0x40000000 : findNextPositivePowerOfTwo(value);
+    }
+
     /**
      * Determine if the requested {@code index} and {@code length} will fit within {@code capacity}.
      * @param index The starting index.

common/src/main/templates/io/netty/util/collection/KObjectHashMap.template

@@ -15,7 +15,7 @@
 
 package io.netty.util.collection;
 
-import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
+import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;
 
 import java.util.AbstractCollection;
 import java.util.AbstractSet;
@@ -77,9 +77,6 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
     }
 
     public @K@ObjectHashMap(int initialCapacity, float loadFactor) {
-        if (initialCapacity < 1) {
-            throw new IllegalArgumentException("initialCapacity must be >= 1");
-        }
         if (loadFactor <= 0.0f || loadFactor > 1.0f) {
             // Cannot exceed 1 because we can never store more than capacity elements;
             // using a bigger loadFactor would trigger rehashing before the desired load is reached.
@@ -89,7 +86,7 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
         this.loadFactor = loadFactor;
 
         // Adjust the initial capacity if necessary.
-        int capacity = findNextPositivePowerOfTwo(initialCapacity);
+        int capacity = safeFindNextPositivePowerOfTwo(initialCapacity);
         mask = capacity - 1;
 
         // Allocate the arrays.