[RNG-170] nextBytes(byte[], int, int) to be consistent with JDK range checks for index out of bounds empty bytes

• Type: Bug
• Status: Closed
• Priority: Trivial
• Resolution: Fixed
• Affects Version/s: 1.4
• Fix Version/s: 1.5
• Component/s: core
• Assignee: Unassigned
• Reporter: Alex Herbert
• Created: 15/Mar/22 22:02
• Updated: 10/Oct/22 11:54
• Resolved: 16/Mar/22 00:09

Description

The implementation of nextBytes is inconsistent:

UniformRandomProvider rng = RandomSource.KISS.create();
byte[] empty = {};
// OK
rng.nextBytes(empty);
// Throws IndexOutOfBoundsException
rng.nextBytes(empty, 0, empty.length);

This also throws:

byte[] bytes = new byte[10];
// Throws IndexOutOfBoundsException
rng.nextBytes(bytes, 0, -1);

The second exception is index out of bounds. This could be:

• illegal argument exception (negative length)
• index out of bounds
• ignore

The case for ignoring negative length is this loop:

 

for (int j = 0; j < len; j++) {
    bytes[start + j]++;
} 

Will not throw for a negative length irrespective of the start index.

 

The nextBytes(byte[], int, int) method is not in the JDK generators so there is no precedent for what to do for a negative length here. However System.arraycopy uses the start+length arguments. This test:

@ParameterizedTest
@CsvSource({
    // OK
    "10, 0, 10",
    "10, 5, 5",
    "10, 9, 1",
    "0, 0, 0",
    // Bad
    "10, 0, 11",
    "10, 10, 1",
    "10, 10, 2147483647",
    "10, 0, -1",
    "10, 5, -1",
})
void testNextBytesIndices(int size, int start, int len) {
    final SplitMix64 rng = new SplitMix64(123);
    final byte[] bytes = new byte[size];
    // Be consistent with System.arraycopy
    try {
        System.arraycopy(bytes, start, bytes, start, len);
    } catch (Exception ex) {
        // nextBytes should throw the same exception
        Assertions.assertThrows(ex.getClass(), () ->
            rng.nextBytes(bytes, start, len));
        return;
    }
    // OK
    rng.nextBytes(bytes, start, len);
}

Shows the exception raised should be ArrayIndexOutOfBoundsException (currently it is IndexOutOfBoundsException). It also shows that System.arraycopy will allows zero length for an empty array with start=0.

I suggest updating the method nextBytes(byte[], start, len) to throw consistent exceptions matching System.arraycopy. This is a minimal change to switch to ArrayIndexOutOfBoundException and to ignore zero length byte[] when start=len=0.