Align int and long in RandomValuePropertySource

This commit aligns int and long so that a random number is generated
by delegating to ints/longs in the JDK's Random API. In the case of a
single bound value, it needs to be greater than 0 because 0 is used as
the lower bound.

Fixes gh-26628

Author: mbhave

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/RandomValuePropertySource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,15 +16,21 @@
 
 package org.springframework.boot.env;
 
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 import java.util.Random;
 import java.util.UUID;
+import java.util.function.BiPredicate;
+import java.util.function.Function;
+import java.util.function.Predicate;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.PropertySource;
 import org.springframework.core.env.StandardEnvironment;
+import org.springframework.util.Assert;
 import org.springframework.util.DigestUtils;
 import org.springframework.util.StringUtils;
 
@@ -44,11 +50,13 @@
  * suffix whose syntax is:
  * <p>
  * {@code OPEN value (,max) CLOSE} where the {@code OPEN,CLOSE} are any character and
- * {@code value,max} are integers. If {@code max} is provided then {@code value} is the
- * minimum value and {@code max} is the maximum (exclusive).
+ * {@code value,max} are integers. If {@code max} is not provided, then 0 is used as the
+ * lower bound and {@code value} is the upper bound. If {@code max} is provided then
+ * {@code value} is the minimum value and {@code max} is the maximum (exclusive).
  *
  * @author Dave Syer
  * @author Matt Benson
+ * @author Madhura Bhave
  * @since 1.0.0
  */
 public class RandomValuePropertySource extends PropertySource<Random> {
@@ -113,22 +121,21 @@ private String getRange(String type, String prefix) {
 	}
 
 	private int getNextIntInRange(String range) {
-		String[] tokens = StringUtils.commaDelimitedListToStringArray(range);
-		int start = Integer.parseInt(tokens[0]);
-		if (tokens.length == 1) {
-			return getSource().nextInt(start);
+		Range<Integer> intRange = Range.get(range, Integer::parseInt, (t) -> t > 0, 0, (t1, t2) -> t1 < t2);
+		OptionalInt first = getSource().ints(1, intRange.getMin(), intRange.getMax()).findFirst();
+		if (!first.isPresent()) {
+			throw new RuntimeException("Could not get random number for range '" + range + "'");
 		}
-		return start + getSource().nextInt(Integer.parseInt(tokens[1]) - start);
+		return first.getAsInt();
 	}
 
 	private long getNextLongInRange(String range) {
-		String[] tokens = StringUtils.commaDelimitedListToStringArray(range);
-		if (tokens.length == 1) {
-			return Math.abs(getSource().nextLong() % Long.parseLong(tokens[0]));
+		Range<Long> longRange = Range.get(range, Long::parseLong, (t) -> t > 0L, 0L, (t1, t2) -> t1 < t2);
+		OptionalLong first = getSource().longs(1, longRange.getMin(), longRange.getMax()).findFirst();
+		if (!first.isPresent()) {
+			throw new RuntimeException("Could not get random number for range '" + range + "'");
 		}
-		long lowerBound = Long.parseLong(tokens[0]);
-		long upperBound = Long.parseLong(tokens[1]) - lowerBound;
-		return lowerBound + Math.abs(getSource().nextLong() % upperBound);
+		return first.getAsLong();
 	}
 
 	private Object getRandomBytes() {
@@ -143,4 +150,39 @@ public static void addToEnvironment(ConfigurableEnvironment environment) {
 		logger.trace("RandomValuePropertySource add to Environment");
 	}
 
+	static final class Range<T extends Number> {
+
+		private final T min;
+
+		private final T max;
+
+		private Range(T min, T max) {
+			this.min = min;
+			this.max = max;
+
+		}
+
+		static <T extends Number> Range<T> get(String range, Function<String, T> parse, Predicate<T> boundValidator,
+				T defaultMin, BiPredicate<T, T> rangeValidator) {
+			String[] tokens = StringUtils.commaDelimitedListToStringArray(range);
+			T token1 = parse.apply(tokens[0]);
+			if (tokens.length == 1) {
+				Assert.isTrue(boundValidator.test(token1), "Bound must be positive.");
+				return new Range<>(defaultMin, token1);
+			}
+			T token2 = parse.apply(tokens[1]);
+			Assert.isTrue(rangeValidator.test(token1, token2), "Lower bound must be less than upper bound.");
+			return new Range<>(token1, token2);
+		}
+
+		T getMin() {
+			return this.min;
+		}
+
+		T getMax() {
+			return this.max;
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/RandomValuePropertySourceTests.java

@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.spy;
 
@@ -66,12 +67,37 @@ void intRange() {
 		assertThat(value < 10).isTrue();
 	}
 
+	@Test
+	void intRangeWhenLowerBoundEqualsUpperBoundShouldFailWithIllegalArgumentException() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.int[4,4]"))
+				.withMessage("Lower bound must be less than upper bound.");
+	}
+
+	@Test
+	void intRangeWhenLowerBoundNegative() {
+		Integer value = (Integer) this.source.getProperty("random.int[-4,4]");
+		assertThat(value >= -4).isTrue();
+		assertThat(value < 4).isTrue();
+	}
+
 	@Test
 	void intMax() {
 		Integer value = (Integer) this.source.getProperty("random.int(10)");
 		assertThat(value).isNotNull().isLessThan(10);
 	}
 
+	@Test
+	void intMaxZero() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.int(0)"))
+				.withMessage("Bound must be positive.");
+	}
+
+	@Test
+	void intNegativeBound() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.int(-5)"))
+				.withMessage("Bound must be positive.");
+	}
+
 	@Test
 	void longValue() {
 		Long value = (Long) this.source.getProperty("random.long");
@@ -84,12 +110,37 @@ void longRange() {
 		assertThat(value).isNotNull().isBetween(4L, 10L);
 	}
 
+	@Test
+	void longRangeWhenLowerBoundEqualsUpperBoundShouldFailWithIllegalArgumentException() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.long[4,4]"))
+				.withMessage("Lower bound must be less than upper bound.");
+	}
+
+	@Test
+	void longRangeWhenLowerBoundNegativeShouldFailWithIllegalArgumentException() {
+		Long value = (Long) this.source.getProperty("random.long[-4,4]");
+		assertThat(value >= -4).isTrue();
+		assertThat(value < 4).isTrue();
+	}
+
 	@Test
 	void longMax() {
 		Long value = (Long) this.source.getProperty("random.long(10)");
 		assertThat(value).isNotNull().isLessThan(10L);
 	}
 
+	@Test
+	void longMaxZero() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.long(0)"))
+				.withMessage("Bound must be positive.");
+	}
+
+	@Test
+	void longNegativeBound() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.source.getProperty("random.long(-5)"))
+				.withMessage("Bound must be positive.");
+	}
+
 	@Test
 	void longOverflow() {
 		RandomValuePropertySource source = spy(this.source);