Skip to content
Permalink
Browse files
RNG-170: Ensure nextBytes is consistent with JDK range checks
Updated to match behaviour of System.arraycopy and JDK 9
Objects.checkFromIndexSize. This now allows:

nextBytes(new byte[0], 0, 0)
nextBytes(new byte[10], 10, 0)

Previously these would throw an exception.
  • Loading branch information
aherbert committed Mar 16, 2022
1 parent 3ed42d7 commit c3c802117da90523ecc0dd1507d895ac9c3f5f54
Showing 8 changed files with 224 additions and 7 deletions.
@@ -152,9 +152,7 @@ public void nextBytes(byte[] bytes) {
public void nextBytes(byte[] bytes,
int start,
int len) {
checkIndex(0, bytes.length - 1, start);
checkIndex(0, bytes.length - start, len);

checkFromIndexSize(start, len, bytes.length);
nextBytesFill(this, bytes, start, len);
}

@@ -206,4 +204,41 @@ static void nextBytesFill(RandomIntSource source,
}
}
}

/**
* Checks if the sub-range from fromIndex (inclusive) to fromIndex + size (exclusive) is
* within the bounds of range from 0 (inclusive) to length (exclusive).
*
* <p>This function provides the functionality of
* {@code java.utils.Objects.checkFromIndexSize} introduced in JDK 9. The
* <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Objects.html#checkFromIndexSize(int,int,int)">Objects</a>
* javadoc has been reproduced for reference.
*
* <p>The sub-range is defined to be out of bounds if any of the following inequalities
* is true:
* <ul>
* <li>{@code fromIndex < 0}
* <li>{@code size < 0}
* <li>{@code fromIndex + size > length}, taking into account integer overflow
* <li>{@code length < 0}, which is implied from the former inequalities
* </ul>
*
* @param fromIndex the lower-bound (inclusive) of the sub-interval
* @param size the size of the sub-range
* @param length the upper-bound (exclusive) of the range
* @return the fromIndex
* @throws IndexOutOfBoundsException if the sub-range is out of bounds
*/
private static int checkFromIndexSize(int fromIndex, int size, int length) {
// check for any negatives,
// or overflow safe length check given the values are all positive
// remaining = length - fromIndex
if ((fromIndex | size | length) < 0 || size > length - fromIndex) {
throw new IndexOutOfBoundsException(
// Note: %<d is 'relative indexing' to re-use the last argument
String.format("Range [%d, %<d + %d) out of bounds for length %d",
fromIndex, size, length));
}
return fromIndex;
}
}
@@ -184,9 +184,7 @@ public void nextBytes(byte[] bytes) {
public void nextBytes(byte[] bytes,
int start,
int len) {
checkIndex(0, bytes.length - 1, start);
checkIndex(0, bytes.length - start, len);

checkFromIndexSize(start, len, bytes.length);
nextBytesFill(this, bytes, start, len);
}

@@ -242,4 +240,41 @@ static void nextBytesFill(RandomLongSource source,
}
}
}

/**
* Checks if the sub-range from fromIndex (inclusive) to fromIndex + size (exclusive) is
* within the bounds of range from 0 (inclusive) to length (exclusive).
*
* <p>This function provides the functionality of
* {@code java.utils.Objects.checkFromIndexSize} introduced in JDK 9. The
* <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Objects.html#checkFromIndexSize(int,int,int)">Objects</a>
* javadoc has been reproduced for reference.
*
* <p>The sub-range is defined to be out of bounds if any of the following inequalities
* is true:
* <ul>
* <li>{@code fromIndex < 0}
* <li>{@code size < 0}
* <li>{@code fromIndex + size > length}, taking into account integer overflow
* <li>{@code length < 0}, which is implied from the former inequalities
* </ul>
*
* @param fromIndex the lower-bound (inclusive) of the sub-interval
* @param size the size of the sub-range
* @param length the upper-bound (exclusive) of the range
* @return the fromIndex
* @throws IndexOutOfBoundsException if the sub-range is out of bounds
*/
private static int checkFromIndexSize(int fromIndex, int size, int length) {
// check for any negatives,
// or overflow safe length check given the values are all positive
// remaining = length - fromIndex
if ((fromIndex | size | length) < 0 || size > length - fromIndex) {
throw new IndexOutOfBoundsException(
// Note: %<d is 'relative indexing' to re-use the last argument
String.format("Range [%d, %<d + %d) out of bounds for length %d",
fromIndex, size, length));
}
return fromIndex;
}
}
@@ -82,6 +82,51 @@ void testFillStateLong() {
}
}

/**
* Test the checkIndex method. This is not used by any RNG implementations
* as it has been superseded by the equivalent of JDK 9 Objects.checkFromIndexSize.
*/
@Test
void testCheckIndex() {
final BaseProvider rng = new BaseProvider() {
@Override
public void nextBytes(byte[] bytes) { /* noop */ }
@Override
public void nextBytes(byte[] bytes, int start, int len) { /* noop */ }
@Override
public int nextInt() {
return 0;
}
@Override
public long nextLong() {
return 0;
}
@Override
public boolean nextBoolean() {
return false;
}
@Override
public float nextFloat() {
return 0;
}
@Override
public double nextDouble() {
return 0;
}
};
// Arguments are (min, max, index)
// index must be in [min, max]
rng.checkIndex(-10, 5, 0);
rng.checkIndex(-10, 5, -10);
rng.checkIndex(-10, 5, 5);
rng.checkIndex(Integer.MIN_VALUE, Integer.MAX_VALUE, Integer.MIN_VALUE);
rng.checkIndex(Integer.MIN_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE);
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> rng.checkIndex(-10, 5, -11));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> rng.checkIndex(-10, 5, 6));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> rng.checkIndex(-10, 5, Integer.MIN_VALUE));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> rng.checkIndex(-10, 5, Integer.MAX_VALUE));
}

/**
* Dummy class for checking the behaviorof the IntProvider. Tests:
* <ul>
@@ -56,14 +56,31 @@ void testPreconditionNextLong(UniformRandomProvider generator) {
@ParameterizedTest
@MethodSource("getList")
void testPreconditionNextBytes(UniformRandomProvider generator) {
Assertions.assertThrows(NullPointerException.class, () -> generator.nextBytes(null, 0, 0));
final int size = 10;
final int num = 1;
final byte[] buf = new byte[size];
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, -1, num));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, size, 0));
// Edge-case allowed by JDK range checks (see RNG-170)
generator.nextBytes(buf, size, 0);
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, size, 1));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, size, -1));
final int offset = 2;
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, offset, size - offset + 1));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, offset, -1));
// offset + length overflows
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(buf, offset, Integer.MAX_VALUE));
// Should be OK
generator.nextBytes(buf, 0, size);
generator.nextBytes(buf, 0, size - offset);
generator.nextBytes(buf, offset, size - offset);
// Should be consistent with no length
final byte[] empty = {};
generator.nextBytes(empty);
generator.nextBytes(empty, 0, 0);
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(empty, 0, 1));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(empty, 0, -1));
Assertions.assertThrows(IndexOutOfBoundsException.class, () -> generator.nextBytes(empty, -1, 0));
}

// Uniformity tests
@@ -18,6 +18,8 @@

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

/**
* The tests the caching of calls to {@link IntProvider#nextInt()} are used as
@@ -75,4 +77,37 @@ void testNextBoolean() {
}
}
}

@ParameterizedTest
@CsvSource({
// OK
"10, 0, 10",
"10, 5, 5",
"10, 9, 1",
// Allowed edge cases
"0, 0, 0",
"10, 10, 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 FlipIntProvider rng = new FlipIntProvider(999);
final byte[] bytes = new byte[size];
// Be consistent with System.arraycopy
try {
System.arraycopy(bytes, start, bytes, start, len);
} catch (IndexOutOfBoundsException ex) {
// nextBytes should throw under the same conditions.
// Note: This is not ArrayIndexOutOfBoundException to be
// future compatible with Objects.checkFromIndexSize.
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
rng.nextBytes(bytes, start, len));
return;
}
rng.nextBytes(bytes, start, len);
}
}
@@ -18,6 +18,8 @@

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

/**
* The tests the caching of calls to {@link LongProvider#nextLong()} are used as
@@ -116,4 +118,37 @@ void testNextBoolean() {
}
}
}

@ParameterizedTest
@CsvSource({
// OK
"10, 0, 10",
"10, 5, 5",
"10, 9, 1",
// Allowed edge cases
"0, 0, 0",
"10, 10, 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 FixedLongProvider rng = new FixedLongProvider(999);
final byte[] bytes = new byte[size];
// Be consistent with System.arraycopy
try {
System.arraycopy(bytes, start, bytes, start, len);
} catch (IndexOutOfBoundsException ex) {
// nextBytes should throw under the same conditions.
// Note: This is not ArrayIndexOutOfBoundException to be
// future compatible with Objects.checkFromIndexSize.
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
rng.nextBytes(bytes, start, len));
return;
}
rng.nextBytes(bytes, start, len);
}
}
@@ -81,6 +81,11 @@ re-run tests that fail, and pass the build if they succeed
within the allotted number of reruns (the test will be marked
as 'flaky' in the report).
">
<action dev="aherbert" type="fix" issue="170">
Update implementations of "UniformRandomProvider.nextBytes" with a range
[start, start + length) to be consistent with the exception conditions of the
JDK array range checks.
</action>
<action dev="aherbert" type="add" issue="167">
New "TSampler" class to sample from Student's t-distribution.
</action>
@@ -107,4 +107,14 @@
<BugPattern name="FE_FLOATING_POINT_EQUALITY"/>
</Match>

<!-- False positives for range checks. The return value matches the argument. -->
<Match>
<Or>
<Class name="org.apache.commons.rng.core.source32.IntProvider"/>
<Class name="org.apache.commons.rng.core.source64.LongProvider"/>
</Or>
<Method name="nextBytes"/>
<BugPattern name="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
</Match>

</FindBugsFilter>

0 comments on commit c3c8021

Please sign in to comment.