Merge pull request #5727 from FiveOFive/retry-after-fix

zaproxy · Sep 16, 2024 · 354c81b · 354c81b
2 parents e5d3b60 + 60b8761
commit 354c81b
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 1 deletion.
diff --git a/addOns/network/CHANGELOG.md b/addOns/network/CHANGELOG.md
@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Send success/failure stats.
 
+### Changed
+- Stop retrying 429 and 503 responses, instead of waiting for `retry-after` (Issue 8627).
+
 ### Fixed
 - Fix typo in log message.
 

diff --git a/...main/java/org/zaproxy/addon/network/internal/client/apachev5/HttpSenderContextApache.java b/...main/java/org/zaproxy/addon/network/internal/client/apachev5/HttpSenderContextApache.java
@@ -19,10 +19,18 @@
  */
 package org.zaproxy.addon.network.internal.client.apachev5;
 
+import java.io.IOException;
+import java.io.InterruptedIOException;
+import java.net.ConnectException;
+import java.net.NoRouteToHostException;
+import java.net.UnknownHostException;
+import java.util.List;
+import javax.net.ssl.SSLException;
 import org.apache.hc.client5.http.HttpRequestRetryStrategy;
 import org.apache.hc.client5.http.cookie.BasicCookieStore;
 import org.apache.hc.client5.http.cookie.CookieStore;
 import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
+import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.util.TimeValue;
 import org.parosproxy.paros.network.HttpSender;
@@ -39,6 +47,17 @@ enum CookieUsage {
 
     private static final TimeValue RETRY_INTERVAL = TimeValue.ofSeconds(0L);
 
+    private static final List<Class<? extends IOException>> NON_RETRIABLE_EXCEPTIONS =
+            List.of(
+                    InterruptedIOException.class,
+                    UnknownHostException.class,
+                    ConnectException.class,
+                    ConnectionClosedException.class,
+                    NoRouteToHostException.class,
+                    SSLException.class);
+
+    private static final List<Integer> RETRIABLE_CODES = List.of();
+
     private HttpRequestRetryStrategy requestRetryStrategy;
     private CookieUsage cookieUsage;
     private CookieStore localCookieStore;
@@ -52,7 +71,8 @@ public void setMaxRetriesOnIoError(int max) {
         super.setMaxRetriesOnIoError(max);
 
         requestRetryStrategy =
-                new DefaultHttpRequestRetryStrategy(max, RETRY_INTERVAL) {
+                new DefaultHttpRequestRetryStrategy(
+                        max, RETRY_INTERVAL, NON_RETRIABLE_EXCEPTIONS, RETRIABLE_CODES) {
                     @Override
                     protected boolean handleAsIdempotent(HttpRequest request) {
                         return true;

diff --git a/...twork/src/test/java/org/zaproxy/addon/network/internal/client/HttpSenderImplUnitTest.java b/...twork/src/test/java/org/zaproxy/addon/network/internal/client/HttpSenderImplUnitTest.java
@@ -264,6 +264,16 @@ static Stream<Arguments> emptyKeepAliveResponseAndSendAndReceiveMethods() {
                 .flatMap(response -> sendAndReceiveMethods().map(sm -> arguments(response, sm)));
     }
 
+    static Stream<Arguments> statusCodesForRetryAndSendAndReceiveMethods() {
+        var statusCodes = new ArrayList<Integer>();
+        for (int i = 200; i < 600; i++) {
+            statusCodes.add(i);
+        }
+        return statusCodes.stream()
+                .flatMap(
+                        statusCode -> sendAndReceiveMethods().map(sm -> arguments(statusCode, sm)));
+    }
+
     @Nested
     class Request {
 
@@ -572,6 +582,27 @@ void shouldBeSentWithRetriesOnIoError(String requestMethod, SenderMethod method)
             }
         }
 
+        @Timeout(10)
+        @ParameterizedTest
+        @MethodSource(
+                "org.zaproxy.addon.network.internal.client.HttpSenderImplUnitTest#statusCodesForRetryAndSendAndReceiveMethods")
+        void shouldNotBeRetriedOnAnyStatusCode(int statusCode, SenderMethod method)
+                throws Exception {
+            // Given
+            server.setHttpMessageHandler(
+                    (ctx, msg) -> {
+                        msg.setResponseHeader(
+                                "HTTP/1.1 "
+                                        + statusCode
+                                        + "\r\nretry-after: 3600\r\ncontent-length: 13\r\n\r\n");
+                        msg.getResponseBody().setBody("Response Body");
+                    });
+            // When
+            httpSender.sendAndReceive(message);
+            // Then
+            assertThat(server.getReceivedMessages(), hasSize(1));
+        }
+
         @Test
         void shouldBeSentWithExistingNtlmAuthentication() throws Exception {
             // Given