diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 8c1ad6746..fa59134ca 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -48,6 +48,7 @@ The type attribute can be add,update,fix,remove. Remove redundant argument from substring call. Enable Dependabot #587. + Correct implementation of RandomUtils.nextLong(long, long) Refine test output for FastDateParserTest diff --git a/src/main/java/org/apache/commons/lang3/RandomUtils.java b/src/main/java/org/apache/commons/lang3/RandomUtils.java index 571cece99..b1c7f0fb1 100644 --- a/src/main/java/org/apache/commons/lang3/RandomUtils.java +++ b/src/main/java/org/apache/commons/lang3/RandomUtils.java @@ -145,7 +145,7 @@ public class RandomUtils { return startInclusive; } - return (long) nextDouble(startInclusive, endExclusive); + return startInclusive + nextLong(endExclusive - startInclusive); } /** @@ -156,7 +156,27 @@ public class RandomUtils { * @since 3.5 */ public static long nextLong() { - return nextLong(0, Long.MAX_VALUE); + return nextLong(Long.MAX_VALUE); + } + + /** + * Generates a {@code long} value between 0 (inclusive) and the specified + * value (exclusive). + * + * @param n Bound on the random number to be returned. Must be positive. + * @return a random {@code long} value between 0 (inclusive) and {@code n} + * (exclusive). + */ + private static long nextLong(long n) { + // Extracted from o.a.c.rng.core.BaseProvider.nextLong(long) + long bits; + long val; + do { + bits = RANDOM.nextLong() >>> 1; + val = bits % n; + } while (bits - val + (n - 1) < 0); + + return val; } /** diff --git a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java index b05c71906..0c8d60992 100644 --- a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java @@ -19,6 +19,7 @@ package org.apache.commons.lang3; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -262,4 +263,28 @@ public class RandomUtilsTest { final double result = RandomUtils.nextDouble(0, Double.MAX_VALUE); assertTrue(result >= 0 && result <= Double.MAX_VALUE); } + + /** + * Test a large value for long. A previous implementation using + * {@link RandomUtils#nextDouble(double, double)} could generate a value equal + * to the upper limit. + * + *
+     * return (long) nextDouble(startInclusive, endExclusive);
+     * 
+ * + *

See LANG-1592.

+ */ + @Test + public void testLargeValueRangeLong() { + final long startInclusive = 12900000000001L; + final long endExclusive = 12900000000016L; + // Note: The method using 'return (long) nextDouble(startInclusive, endExclusive)' + // takes thousands of calls to generate an error. This size loop fails most + // of the time with the previous method. + final int n = (int) (endExclusive - startInclusive) * 1000; + for (int i = 0; i < n; i++) { + assertNotEquals(endExclusive, RandomUtils.nextLong(startInclusive, endExclusive)); + } + } }