ppkarwasz commented on code in PR #1379:
URL: https://github.com/apache/commons-lang/pull/1379#discussion_r2086790446


##########
src/main/java/org/apache/commons/lang3/CachedRandomBits.java:
##########
@@ -52,6 +52,16 @@ final class CachedRandomBits {
      */
     private int bitIndex;
 
+    /**
+     * The maximum size of the cache.
+     *
+     * <p>
+     * This is dictated by the {@code if (bitIndex >> 3 >= cache.length)} in 
the {@link #nextBits(int)} method.
+     * Essentially {@code 63_913_202 << 3 } will overflow so this line will 
never evaluate to true.
+     * At some point the {@code bitIndex += generatedBitsInIteration} will 
overflow and cause an exception.
+     */
+    private static final int MAX_CACHE_SIZE = 63_913_200;

Review Comment:
   The value here should rather be `0x1FFF_FFFF` (i.e. `268_435_455`, but in 
hex it is easier to understand).
   
   `63_913_201` is the highest `x` such that `(21x + 3) / 5 + 10 ≤ 268 435 455`.



##########
src/main/java/org/apache/commons/lang3/RandomStringUtils.java:
##########
@@ -330,8 +329,8 @@ public static String random(int count, int start, int end, 
final boolean letters
         // Ideally the cache size depends on multiple factor, including the 
cost of generating x bytes
         // of randomness as well as the probability of rejection. It is 
however not easy to know
         // those values programmatically for the general case.
-        // To prevent overflow restrict the cache size to 60M entries (
-        final int cacheSize = Math.min ((count * gapBits + 3) / 5 + 10, 
MAX_CACHE_SIZE);
+        // For huge strings the padding required can cause an overflow
+        final int cacheSize = (count * gapBits + 3) / 5 + 10 > 0 ? (count * 
gapBits + 3) / 5 + 10 : Integer.MAX_VALUE;

Review Comment:
   ```suggestion
           final int cacheSize = count * gapBits + 3 > 0 ? (count * gapBits + 
3) / 5 + 10 : Integer.MAX_VALUE;
   ```
   
   Adding `10` can transform a negative number into a small positive one.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to