fabrice102 opened a new pull request, #1273:
URL: https://github.com/apache/commons-lang/pull/1273

   An optimization introduced in commit 
f382d61a03778ccf838c6c051bd8692e4834dec2 incorrectly assumed that letters and 
numbers/digits are ASCII.
   
   For example, `RandomStringUtils.random(1, 0x4e00, 0x4e01, true, false)` 
would take too long to terminate and would not output the correct answer, that 
is the string with a single character `0x4e00`. The issue is that `end` would 
be replaced by `'z' + 1 = 123` (the latest ASCII letter + 1) there: 
https://github.com/apache/commons-lang/blob/f382d61a03778ccf838c6c051bd8692e4834dec2/src/main/java/org/apache/commons/lang3/RandomStringUtils.java#L266-L270
    Thus, we would have `end < start` and `gap = end - start < 0`.
   
   This PR fixes this issue by restricting the optimization to cases where only 
ASCII characters are requested, that is `end < 0x7f`. This PR also slightly 
clarifies the code, using constant variables instead of '0', '9', 'A', and 'z'.
   
   The PR also includes two regression tests.
   
   The Github diff does not show it very clearly, but all the PR does is
   
   1. moving the optimization code inside a bug `if (chars == null && end <= 
0x7f)`
   2. using constant variables instead of '0', '9', 'A', and 'z'
   3. moving the  test `(numbers && end <= zeroDigitAscii || letters && end <= 
firstLetterAscii)` inside the big `if (chars == null && end <= 0x7f)` and 
before the problematic optimization: this does not change the semantics and is 
just done for clarity of the resulting code.


-- 
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]

Reply via email to