This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 313d877d5 Two fixes in RandomStringUtils.random(int, int, int,
boolean, boolean, (#1638)
313d877d5 is described below
commit 313d877d57abefdbb1ad0c42781bf719b83d5a35
Author: Gary Gregory <[email protected]>
AuthorDate: Thu May 7 08:08:51 2026 -0400
Two fixes in RandomStringUtils.random(int, int, int, boolean, boolean,
(#1638)
char[], Random)
- A custom chars array throws IllegalArgumentException because
validation loops treat the loop index as a char code point instead of an
index into the chars array.
- random() hangs when the specified [start, end) range contains ONLY
rejected code points (UNASSIGNED, PRIVATE_USE, SURROGATE). The loop
increments count and retries indefinitely.
---
.../apache/commons/lang3/RandomStringUtils.java | 77 ++++++++++++++++++----
.../commons/lang3/RandomStringUtilsTest.java | 45 +++++++++++++
2 files changed, 108 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
b/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
index 4d91b8418..4795fb0d6 100644
--- a/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
+++ b/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
@@ -294,8 +294,8 @@ public static String random(int count, int start, int end,
final boolean letters
}
if (digits && end <= ASCII_0 || letters && end <= ASCII_A) {
throw new IllegalArgumentException(
- String.format("Parameter end (%,d) must be greater
than (%,d) for generating digits or greater than (%,d) for generating letters.",
- end, ASCII_0, ASCII_A));
+ String.format("Parameter end (%,d) must be greater
than (%,d) for generating digits or greater than (%,d) for generating
letters.", end,
+ ASCII_0, ASCII_A));
}
// Optimize start and end when filtering by letters and/or numbers:
// The range provided may be too large since we filter anyway
afterward.
@@ -318,25 +318,50 @@ public static String random(int count, int start, int
end, final boolean letters
end = Math.min(ASCII_z + 1, end);
}
}
- if (letters && !digits) {
- for (int i = start; i < end; i++) {
- if (Character.isLetter(i)) {
- break;
+ if (chars == null) {
+ // start/end are code points: validate using
Character.isLetter/isDigit on the
+ // code-point range rather than on the loop index.
+ if (letters && !digits) {
+ boolean ok = false;
+ for (int i = start; i < end; i++) {
+ if (Character.isLetter(i)) {
+ ok = true;
+ break;
+ }
}
- if (i == end - 1) {
+ if (!ok) {
throw new IllegalArgumentException(String.format("No
letters exist between start %,d and end %,d.", start, end));
}
}
- }
- if (!letters && digits) {
- for (int i = start; i < end; i++) {
- if (Character.isDigit(i)) {
- break;
+ if (!letters && digits) {
+ boolean ok = false;
+ for (int i = start; i < end; i++) {
+ if (Character.isDigit(i)) {
+ ok = true;
+ break;
+ }
}
- if (i == end - 1) {
+ if (!ok) {
throw new IllegalArgumentException(String.format("No
digits exist between start %,d and end %,d.", start, end));
}
}
+ } else if (letters || digits) {
+ // chars != null. start/end are indices into chars[]; validate the
actual
+ // chars contain at least one element matching some requested
letter/digit
+ // category to avoid an infinite generation loop when the array
lacks every
+ // requested category.
+ boolean hasMatch = false;
+ for (int i = start; i < end; i++) {
+ final char c = chars[i];
+ if (letters && Character.isLetter(c) || digits &&
Character.isDigit(c)) {
+ hasMatch = true;
+ break;
+ }
+ }
+ if (!hasMatch) {
+ throw new IllegalArgumentException(String.format("No %s%s%s
exist in chars[%,d..%,d).", letters ? "letters" : "",
+ letters && digits ? " or " : "", digits ? "digits" :
"", start, end));
+ }
}
final StringBuilder builder = new StringBuilder(count);
final int gap = end - start;
@@ -352,16 +377,27 @@ public static String random(int count, int start, int
end, final boolean letters
// 3. Divide by 5 to convert to bytes (normally this would be by 8,
dividing by 5 allows for about 60% extra space)
// 4. Add base padding (10) to handle small counts efficiently
// 5. Ensure we don't exceed Integer.MAX_VALUE / 5 + 10 to provide a
good balance between overflow prevention and
- // making the cache extremely large
+ // making the cache extremely large
final long desiredCacheSize = ((long) count * gapBits +
CACHE_PADDING_BITS) / BITS_TO_BYTES_DIVISOR + BASE_CACHE_SIZE_PADDING;
final int cacheSize = (int) Math.min(desiredCacheSize,
Integer.MAX_VALUE / BITS_TO_BYTES_DIVISOR + BASE_CACHE_SIZE_PADDING);
final CachedRandomBits arb = new CachedRandomBits(cacheSize, random);
+ // Bound rejection retries so a range that rejects every sample
+ // (for example, entirely UNASSIGNED/PRIVATE_USE/SURROGATE) raises an
+ // IllegalArgumentException instead of looping indefinitely. Cap is
+ // (end - start) * 10 with a small floor so tiny gaps still get a
+ // reasonable budget. The counter resets on every accepted code point.
+ final int maxRejections = Math.max(64, gap * 10);
+ int rejections = 0;
while (count-- != 0) {
// Generate a random value between start (included) and end
(excluded)
final int randomValue = arb.nextBits(gapBits) + start;
// Rejection sampling if value too large
if (randomValue >= end) {
count++;
+ if (++rejections > maxRejections) {
+ throw new IllegalArgumentException(
+ String.format("No acceptable code points found in
range [%,d, %,d) within %,d attempts.", start, end, maxRejections));
+ }
continue;
}
final int codePoint;
@@ -372,6 +408,10 @@ public static String random(int count, int start, int end,
final boolean letters
case Character.PRIVATE_USE:
case Character.SURROGATE:
count++;
+ if (++rejections > maxRejections) {
+ throw new IllegalArgumentException(
+ String.format("No acceptable code points found
in range [%,d, %,d) within %,d attempts.", start, end, maxRejections));
+ }
continue;
}
} else {
@@ -380,6 +420,10 @@ public static String random(int count, int start, int end,
final boolean letters
final int numberOfChars = Character.charCount(codePoint);
if (count == 0 && numberOfChars > 1) {
count++;
+ if (++rejections > maxRejections) {
+ throw new IllegalArgumentException(
+ String.format("No acceptable code points found in
range [%,d, %,d) within %,d attempts.", start, end, maxRejections));
+ }
continue;
}
if (letters && Character.isLetter(codePoint) || digits &&
Character.isDigit(codePoint) || !letters && !digits) {
@@ -387,8 +431,13 @@ public static String random(int count, int start, int end,
final boolean letters
if (numberOfChars == 2) {
count--;
}
+ rejections = 0;
} else {
count++;
+ if (++rejections > maxRejections) {
+ throw new IllegalArgumentException(
+ String.format("No acceptable code points found in
range [%,d, %,d) within %,d attempts.", start, end, maxRejections));
+ }
}
}
return builder.toString();
diff --git a/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
b/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
index e5622c94e..7ccd9e48b 100644
--- a/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
@@ -17,15 +17,18 @@
package org.apache.commons.lang3;
import static
org.apache.commons.lang3.LangAssertions.assertIllegalArgumentException;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
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.assertTimeout;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
+import java.time.Duration;
import java.util.Random;
import java.util.stream.Stream;
@@ -93,6 +96,30 @@ void testConstructor() {
assertNotNull(new RandomStringUtils());
}
+ /**
+ * A custom chars array throws IllegalArgumentException because validation
loops treat the loop index as a char code point instead of an index into the
+ * chars array.
+ * <p>
+ * Pre-patch: random(5, 0, 0, true, false, new char[]{'a','b','c'}, rng)
enters the "letters && !digits" loop, iterates i from 0 to chars.length, but
checks
+ * Character.isLetter(i) where i=0,1,2 are control characters, so it
throws IAE "No letters exist between start 0 and end 3".
+ * </p>
+ *
+ * <p>
+ * Post-patch: validation skips index-based char check when chars array is
provided, or correctly checks chars[i] instead of i.
+ * </p>
+ */
+ @Test
+ public void testCustomLetterCharsArrayDoesNotThrowIAE() {
+ final char[] letters = { 'a', 'b', 'c' };
+ assertDoesNotThrow(() -> {
+ final String result = RandomStringUtils.random(5, 0, 0, true,
false, letters, new Random(42));
+ assertEquals(5, result.length());
+ for (final char c : result.toCharArray()) {
+ assertTrue(c == 'a' || c == 'b' || c == 'c', () -> "Expected
char from {a,b,c} but got: " + c);
+ }
+ }, "RandomStringUtils.random() threw IAE for valid letter chars array
- pre-patch behavior");
+ }
+
@Test
void testExceptionsRandom() {
assertIllegalArgumentException(() -> RandomStringUtils.random(-1));
@@ -380,6 +407,24 @@ void testNonASCIINumbers(final RandomStringUtils rsu) {
assertTrue(found, "no non-ASCII number generated");
}
+ /**
+ * random() hangs when the specified [start, end) range contains ONLY
rejected code points (UNASSIGNED, PRIVATE_USE, SURROGATE). The loop increments
count
+ * and retries indefinitely.
+ * <p>
+ * The private-use area U+E000..U+F8FF (0xE000..0xF900) contains only
PRIVATE_USE code points, so random(1, 0xE000, 0xF900, false, false, null, rng)
hangs
+ * forever pre-patch.
+ * </p>
+ * <ul>
+ * <li>Pre-patch: hangs indefinitely.</li>
+ * <li>Post-patch: throws IllegalArgumentException quickly.</li>
+ * </ul>
+ */
+ @Test
+ public void testOnlyRejectedCodePoints() {
+ assertTimeout(Duration.ofSeconds(2),
+ () -> assertThrows(IllegalArgumentException.class, () ->
RandomStringUtils.random(1, 0xE000, 0xF900, false, false, null, new
Random(42))));
+ }
+
/**
* Make sure boundary alpha characters are generated by randomAlphabetic
This test will fail randomly with probability = 4 * (51/52)**1000 ~ 1.58E-8
*/
