alhudz commented on code in PR #1703:
URL: https://github.com/apache/commons-lang/pull/1703#discussion_r3411758610
##########
src/main/java/org/apache/commons/lang3/RandomStringUtils.java:
##########
@@ -296,7 +296,9 @@ public static String random(int count, int start, int end,
final boolean letters
if (letters && digits && start <= ASCII_0 && end >= ASCII_z + 1) {
return random(count, 0, 0, false, false, ALPHANUMERICAL_CHARS,
random);
}
- if (digits && end <= ASCII_0 || letters && end <= ASCII_A) {
+ // Only reject when none of the requested categories is reachable;
otherwise a letters && digits
+ // request would throw on a range that holds one category but not
the other (e.g. [ASCII_0, ASCII_A)).
+ if ((!digits || end <= ASCII_0) && (!letters || end <= ASCII_A) &&
(digits || letters)) {
throw new IllegalArgumentException(
String.format("Parameter end (%,d) must be greater
than (%,d) for generating digits or greater than (%,d) for generating
letters.", end,
Review Comment:
Good catch. That collapse predates this change (the guard keyed off `end`
only before too), but it is a real sharp edge: `random(10, 'z' + 1, 0x7f, true,
true, null, rng)` clamps to `start == end`, so `gap == 0` and `nextBits(0)`
throws the unrelated `number of bits must be between 1 and 32`.
Pushed a fix: when the `letters && digits` clamp empties the range it now
throws the range-validation `IllegalArgumentException` (`No letters or digits
exist between start ... and end ...`), matching the single-category
reachability checks just below. Added
`testLettersAndDigitsOverEmptyAsciiRange`, which fails with the `number of
bits` message before the change and passes after.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
