Fix overflow bugs in backoff strategy.

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java

@@ -24,6 +24,12 @@
 @FunctionalInterface
 public interface BackoffStrategy {
 
+    /**
+     * Max permitted retry times. To prevent exponentialDelay from overflow, there must be 2 ^ retriesAttempted
+     * <= 2 ^ 31 - 1, which means retriesAttempted <= 30, so that is the ceil for retriesAttempted.
+     */
+    int RETRIES_ATTEMPTED_CEILING = (int) Math.floor(Math.log(Integer.MAX_VALUE) / Math.log(2));
+
     /**
      * Compute the delay before the next retry request. This strategy is only consulted when there will be a next retry.
      *
@@ -33,7 +39,8 @@ public interface BackoffStrategy {
     Duration computeDelayBeforeNextRetry(RetryPolicyContext context);
 
     default int calculateExponentialDelay(int retriesAttempted, Duration baseDelay, Duration maxBackoffTime) {
-        return (int) Math.min((1L << retriesAttempted) * baseDelay.toMillis(), maxBackoffTime.toMillis());
+        int cappedRetries = Math.min(retriesAttempted, RETRIES_ATTEMPTED_CEILING);
+        return (int) Math.min(baseDelay.multipliedBy(1L << cappedRetries).toMillis(), maxBackoffTime.toMillis());
     }
 
     static BackoffStrategy defaultStrategy() {

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java

@@ -15,6 +15,7 @@
 
 package software.amazon.awssdk.core.retry.backoff;
 
+import static software.amazon.awssdk.utils.NumericUtils.min;
 import static software.amazon.awssdk.utils.Validate.isNotNegative;
 
 import java.time.Duration;
@@ -44,13 +45,21 @@ public final class EqualJitterBackoffStrategy implements BackoffStrategy,
                                                          ToCopyableBuilder<EqualJitterBackoffStrategy.Builder,
                                                              EqualJitterBackoffStrategy> {
 
+    private static final Duration BASE_DELAY_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days
+    private static final Duration MAX_BACKOFF_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days
+
     private final Duration baseDelay;
     private final Duration maxBackoffTime;
-    private final Random random = new Random();
+    private final Random random;
 
     private EqualJitterBackoffStrategy(BuilderImpl builder) {
-        this.baseDelay = isNotNegative(builder.baseDelay, "baseDelay");
-        this.maxBackoffTime = isNotNegative(builder.maxBackoffTime, "maxBackoffTime");
+        this(builder.baseDelay, builder.maxBackoffTime, new Random());
+    }
+
+    EqualJitterBackoffStrategy(final Duration baseDelay, final Duration maxBackoffTime, final Random random) {
+        this.baseDelay = min(isNotNegative(baseDelay, "baseDelay"), BASE_DELAY_CEILING);
+        this.maxBackoffTime = min(isNotNegative(maxBackoffTime, "maxBackoffTime"), MAX_BACKOFF_CEILING);
+        this.random = random;
     }
 
     @Override

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java

@@ -15,6 +15,7 @@
 
 package software.amazon.awssdk.core.retry.backoff;
 
+import static software.amazon.awssdk.utils.NumericUtils.min;
 import static software.amazon.awssdk.utils.Validate.isNotNegative;
 
 import java.time.Duration;
@@ -40,14 +41,21 @@
 public final class FullJitterBackoffStrategy implements BackoffStrategy,
                                                         ToCopyableBuilder<FullJitterBackoffStrategy.Builder,
                                                             FullJitterBackoffStrategy> {
+    private static final Duration BASE_DELAY_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days
+    private static final Duration MAX_BACKOFF_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days
 
     private final Duration baseDelay;
     private final Duration maxBackoffTime;
-    private final Random random = new Random();
+    private final Random random;
 
     private FullJitterBackoffStrategy(BuilderImpl builder) {
-        this.baseDelay = isNotNegative(builder.baseDelay, "baseDelay");
-        this.maxBackoffTime = isNotNegative(builder.maxBackoffTime, "maxBackoffTime");
+        this(builder.baseDelay, builder.maxBackoffTime, new Random());
+    }
+
+    FullJitterBackoffStrategy(final Duration baseDelay, final Duration maxBackoffTime, final Random random) {
+        this.baseDelay = min(isNotNegative(baseDelay, "baseDelay"), BASE_DELAY_CEILING);
+        this.maxBackoffTime = min(isNotNegative(maxBackoffTime, "maxBackoffTime"), MAX_BACKOFF_CEILING);
+        this.random = random;
     }
 
     @Override

core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategyTest.java

@@ -0,0 +1,116 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.core.retry.backoff;
+
+import org.junit.Test;
+import org.mockito.Mock;
+import software.amazon.awssdk.core.retry.RetryPolicyContext;
+
+import java.time.Duration;
+import java.util.Random;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertThrows;
+
+public class EqualJitterBackoffStrategyTest {
+
+    private static final int RANDOM_RESULT = 12345;
+    private static final Duration FIVE_DAYS = Duration.ofDays(5);
+    private static final Duration MAX_DURATION = Duration.ofSeconds(Long.MAX_VALUE);
+    private static final Duration ONE_SECOND = Duration.ofSeconds(1);
+    private static final Duration ONE_NANO_SECOND = Duration.ofNanos(1);
+    private static final int NANO_IN_MILLISECONDS = 1_000_000;
+    private static final Duration NEGATIVE_ONE_SECOND = Duration.ofSeconds(-1);
+
+    @Mock
+    private Random mockRandom = mock(Random.class);
+
+    @Test
+    public void exponentialDelayOverflowWithMaxBackoffTest() {
+        test(FIVE_DAYS, MAX_DURATION, 3, Integer.MAX_VALUE);
+    }
+
+    @Test
+    public void exponentialDelayOverflowWithMaxBaseDelayTest() {
+        test(MAX_DURATION, MAX_DURATION, 1, Integer.MAX_VALUE);
+    }
+
+    @Test
+    public void maxBaseDelayShortBackoffTest() {
+        test(MAX_DURATION, ONE_SECOND, 1, (int) ONE_SECOND.toMillis());
+    }
+
+    @Test
+    public void normalConditionTest() {
+        test(ONE_SECOND, MAX_DURATION, 10, (1 << 10) * (int) ONE_SECOND.toMillis());
+    }
+
+    @Test
+    public void tinyBackoffNormalRetriesTest() {
+        test(MAX_DURATION, ONE_NANO_SECOND, 10, 0);
+    }
+
+    @Test
+    public void tinyBaseDelayNormalRetriesTest() {
+        test(ONE_NANO_SECOND, MAX_DURATION, 30,  (int) (1L << 30) / NANO_IN_MILLISECONDS);
+    }
+
+    @Test
+    public void tinyBaseDelayExtraRetriesTest() {
+        test(ONE_NANO_SECOND, MAX_DURATION, 100,
+                (int) (1L << 30) / NANO_IN_MILLISECONDS); // RETRIES_ATTEMPTED_CEILING == 30
+    }
+
+    @Test
+    public void exponentialDelayOverflowWithExtraRetriesTest() {
+        test(MAX_DURATION, MAX_DURATION, 100, Integer.MAX_VALUE);
+    }
+
+    @Test
+    public void tinyBaseDelayUnderflowTest() {
+        test(ONE_NANO_SECOND, MAX_DURATION, 0, 0);
+    }
+
+    @Test
+    public void negativeBaseDelayTest() {
+        assertThrows(IllegalArgumentException.class, () ->
+                test(NEGATIVE_ONE_SECOND, MAX_DURATION, 1, 0));
+    }
+
+    @Test
+    public void negativeBackoffTest() {
+        assertThrows(IllegalArgumentException.class, () ->
+                test(ONE_SECOND, NEGATIVE_ONE_SECOND, 1, 0));
+    }
+
+    private void test(final Duration baseDelay, final Duration maxBackoffTime,
+                      final int retriesAttempted, final int expectedCeilingMillis) {
+        final BackoffStrategy backoffStrategy = new EqualJitterBackoffStrategy(baseDelay, maxBackoffTime, mockRandom);
+
+        when(mockRandom.nextInt(expectedCeilingMillis /2 + 1)).thenReturn(RANDOM_RESULT);
+
+        assertThat(backoffStrategy.computeDelayBeforeNextRetry(toRetryContext(retriesAttempted)),
+                is(Duration.ofMillis(expectedCeilingMillis / 2 + RANDOM_RESULT)));
+    }
+
+    private static RetryPolicyContext toRetryContext(final int retriesAttempted) {
+        return RetryPolicyContext.builder().retriesAttempted(retriesAttempted).build();
+    }
+
+}

utils/src/main/java/software/amazon/awssdk/utils/NumericUtils.java

@@ -15,6 +15,7 @@
 
 package software.amazon.awssdk.utils;
 
+import java.time.Duration;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 
 @SdkProtectedApi
@@ -41,4 +42,12 @@ public static int saturatedCast(long value) {
         return (int) value;
     }
 
+    public static Duration min(Duration a, Duration b) {
+        return (a.compareTo(b) < 0) ? a : b;
+    }
+
+    public static Duration max(Duration a, Duration b) {
+        return (a.compareTo(b) > 0) ? a : b;
+    }
+
 }

utils/src/test/java/software/amazon/awssdk/utils/NumericUtilsTest.java

@@ -0,0 +1,70 @@
+package software.amazon.awssdk.utils;
+
+import java.time.Duration;
+
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+import static software.amazon.awssdk.utils.NumericUtils.max;
+import static software.amazon.awssdk.utils.NumericUtils.min;
+
+public class NumericUtilsTest {
+
+    private final Duration SHORT_DURATION = Duration.ofMillis(10);
+    private final Duration SHORT_SAME_DURATION = Duration.ofMillis(10);
+    private final Duration LONG_DURATION = Duration.ofMillis(100);
+    private final Duration NEGATIVE_SHORT_DURATION = Duration.ofMillis(-10);
+    private final Duration NEGATIVE_SHORT_SAME_DURATION = Duration.ofMillis(-10);
+    private final Duration NEGATIVE_LONG_DURATION = Duration.ofMillis(-100);
+
+    @Test
+    public void minTestDifferentDurations() {
+        assertThat(min(SHORT_DURATION, LONG_DURATION), is(SHORT_DURATION));
+    }
+
+    @Test
+    public void minTestDifferentDurationsReverse() {
+        assertThat(min(LONG_DURATION, SHORT_DURATION), is(SHORT_DURATION));
+    }
+
+    @Test
+    public void minTestSameDurations() {
+        assertThat(min(SHORT_DURATION, SHORT_SAME_DURATION), is(SHORT_SAME_DURATION));
+    }
+
+    @Test
+    public void minTestDifferentNegativeDurations() {
+        assertThat(min(NEGATIVE_SHORT_DURATION, NEGATIVE_LONG_DURATION), is(NEGATIVE_LONG_DURATION));
+    }
+
+    @Test
+    public void minTestNegativeSameDurations() {
+        assertThat(min(NEGATIVE_SHORT_DURATION, NEGATIVE_SHORT_SAME_DURATION), is(NEGATIVE_SHORT_DURATION));
+    }
+
+    @Test
+    public void maxTestDifferentDurations() {
+        assertThat(max(LONG_DURATION, SHORT_DURATION), is(LONG_DURATION));
+    }
+
+    @Test
+    public void maxTestDifferentDurationsReverse() {
+        assertThat(max(SHORT_DURATION, LONG_DURATION), is(LONG_DURATION));
+    }
+
+    @Test
+    public void maxTestSameDurations() {
+        assertThat(max(SHORT_DURATION, SHORT_SAME_DURATION), is(SHORT_SAME_DURATION));
+    }
+
+    @Test
+    public void maxTestDifferentNegativeDurations() {
+        assertThat(max(NEGATIVE_SHORT_DURATION, NEGATIVE_LONG_DURATION), is(NEGATIVE_SHORT_DURATION));
+    }
+
+    @Test
+    public void maxTestNegativeSameDurations() {
+        assertThat(max(NEGATIVE_SHORT_DURATION, NEGATIVE_SHORT_SAME_DURATION), is(NEGATIVE_SHORT_DURATION));
+    }
+}