Skip to content

Conversation

@IcoreE
Copy link
Contributor

@IcoreE IcoreE commented Dec 9, 2025

# source code 
public static String random(int count, int start, int end, final boolean letters, final boolean numbers,
final char[] chars, final Random random) {

When a custom character array (chars != null) is supplied to RandomStringUtils.random(), the method does not strictly check that the start and end parameters fall within the valid bounds of the chars array.

As a result, if start or end exceeds chars.length, the method may generate a random index outside the array range, leading to an unexpected ArrayIndexOutOfBoundsException.

This fails the method contract and causes unpredictable runtime errors.

@Test
void testStartEndOutOfRangeWithChars() {
        char[] chars = {'a', 'b', 'c'};
        assertThrows(ArrayIndexOutOfBoundsException.class, () -> {
            RandomStringUtils.random(
                    5,
                    5,         // invalid: start > chars.length
                    10,             // invalid: end > chars.length
                    false,
                    false,
                    chars,
                    new Random()
            );
        });
}

Actual:
Throws ArrayIndexOutOfBoundsException

Expected:
Throw IllegalArgumentException indicating invalid start/end range when chars != null
my issue @garydgregory

…chars != null, causing potential IndexOutOfBoundsException
@theodoral22
Copy link

Hi maintainers,

I have prepared a fix to the problem of the method RandomStringUtils.random(), that:

  1. Adds proper validation to ensure start < chars.length and end <= chars.length.

  2. Throws an IllegalArgumentException when the bounds are invalid.

  3. Includes unit tests to cover these cases.

Would you be open to accepting a pull request with this fix?

Thank you for your time and for maintaining this excellent library!

Best regards,
Theodora Anastasia Lazaridou

@IcoreE IcoreE changed the title RandomStringUtils.random() does not strictly validate start/end when chars != null, causing potential IndexOutOfBoundsException 【LANG-1801】Fix RandomStringUtils.random() does not strictly validate start/end when chars != null, causing potential IndexOutOfBoundsException Dec 18, 2025
@garydgregory garydgregory changed the title 【LANG-1801】Fix RandomStringUtils.random() does not strictly validate start/end when chars != null, causing potential IndexOutOfBoundsException [LANG-1801] Fix RandomStringUtils.random() does not strictly validate start/end when chars != null, causing potential IndexOutOfBoundsException Dec 20, 2025
@IcoreE
Copy link
Contributor Author

IcoreE commented Dec 21, 2025

Hello @theodoral22
Thank you for your attention to this issue. I have submitted the code and unit tests to fix it. PR#1521

@IcoreE
Copy link
Contributor Author

IcoreE commented Dec 21, 2025

Hi @garydgregory
I hope everything is going well on your end! It’s been a little while since I submitted PR (#1521), and I just wanted to follow up in case you’ve had a chance to take a look. If you have any feedback or if there’s anything I can improve, I’d be happy to make the necessary changes. Thanks again for all the work you put into the project!

@garydgregory
Copy link
Member

Hello @IcoreE
There are higher priority issue I want to deal with.

assertIllegalArgumentException(() -> RandomStringUtils.random(8, 32, 48, false, true));
assertIllegalArgumentException(() -> RandomStringUtils.random(8, 32, 65, true, false));
assertIllegalArgumentException(() -> RandomStringUtils.random(1, Integer.MIN_VALUE, -10, false, false, null));
assertIllegalArgumentException(() -> RandomStringUtils.random(2, 5, 6, false, false, new char[] { 'a', 'b', 'c', 'd' }, new Random()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should cover checks for end and start both separately and together.
Also should check that the correct messages are generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants