[
https://issues.apache.org/jira/browse/RNG-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17507260#comment-17507260
]
Alex Herbert commented on RNG-170:
----------------------------------
Java 9 introduced:
{code:java}
Objects.checkFromIndexSize(int fromIndex, int size, int length);
{code}
See [Objects.checkFromIndexSize JDK 11
javadoc|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Objects.html#checkFromIndexSize(int,int,int)]
The behaviour of this method is the same as System.arraycopy but throws
IndexOutOfBoundException, not ArrayIndexOutOfBoundException.
It will allow:
{noformat}
size start len
0 0 0
10 10 0{noformat}
The latter is an edge case that also previously threw for nextBytes. So this is
two behaviour changes introduced by updating to be consistent with the JDK
range checks, both when the length is 0.
> nextBytes(byte[], int, int) to be consistent with JDK range checks for index
> out of bounds empty bytes
> ------------------------------------------------------------------------------------------------------
>
> Key: RNG-170
> URL: https://issues.apache.org/jira/browse/RNG-170
> Project: Commons RNG
> Issue Type: Bug
> Components: core
> Affects Versions: 1.4
> Reporter: Alex Herbert
> Priority: Trivial
> Fix For: 1.5
>
>
> The implementation of nextBytes is inconsistent:
> {code:java}
> UniformRandomProvider rng = RandomSource.KISS.create();
> byte[] empty = {};
> // OK
> rng.nextBytes(empty);
> // Throws IndexOutOfBoundsException
> rng.nextBytes(empty, 0, empty.length);
> {code}
> This also throws:
> {code:java}
> byte[] bytes = new byte[10];
> // Throws IndexOutOfBoundsException
> rng.nextBytes(bytes, 0, -1);
> {code}
> 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:
>
> {code:java}
> for (int j = 0; j < len; j++) {
> bytes[start + j]++;
> } {code}
> 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:
> {code:java}
> @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);
> }
> {code}
> 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.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
