Fix ZAP stuck from 'retry-after' header on 429, 503

Signed-off-by: FiveOFive <[email protected]>
zaproxy · Sep 16, 2024 · ef249a6 · ef249a6
1 parent 7518dbc
commit ef249a6
Show file tree

Hide file tree

Showing 3 changed files with 64 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.
 
+### Changes
+- Stop retrying 429 and 503 responses.
+
 ### 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
@@ -958,6 +958,46 @@ void shouldNotContainUserObjectWithConnectionClosedIfNotClosed(
             assertThat(message.getUserObject(), is(nullValue()));
         }
 
+        @Test
+        @Timeout(10)
+        void shouldNotRetry429WithRetryAfter() throws Exception {
+            // Given - a large enough retry-after value to reach the test timeout if obeyed
+            String responseHeader =
+                    "HTTP/1.1 429 Reason\r\nretry-after: 3600\r\ncontent-length: 13\r\n\r\n";
+            String responseBody = "Response Body";
+            server.setHttpMessageHandler(
+                    (ctx, msg) -> {
+                        msg.setResponseHeader(responseHeader);
+                        msg.setResponseBody(responseBody);
+                    });
+            // When
+            httpSender.sendAndReceive(message);
+            // Then
+            assertThat(server.getReceivedMessages(), hasSize(1));
+            assertThat(message.getResponseHeader().toString(), is(equalTo(responseHeader)));
+            assertThat(message.getResponseBody().toString(), is(equalTo(responseBody)));
+        }
+
+        @Test
+        @Timeout(10)
+        void shouldNotRetry503WithRetryAfter() throws Exception {
+            // Given - a large enough retry-after value to reach the test timeout if obeyed
+            String responseHeader =
+                    "HTTP/1.1 503 Reason\r\nretry-after: 3600\r\ncontent-length: 13\r\n\r\n";
+            String responseBody = "Response Body";
+            server.setHttpMessageHandler(
+                    (ctx, msg) -> {
+                        msg.setResponseHeader(responseHeader);
+                        msg.setResponseBody(responseBody);
+                    });
+            // When
+            httpSender.sendAndReceive(message);
+            // Then
+            assertThat(server.getReceivedMessages(), hasSize(1));
+            assertThat(message.getResponseHeader().toString(), is(equalTo(responseHeader)));
+            assertThat(message.getResponseBody().toString(), is(equalTo(responseBody)));
+        }
+
         @Nested
         class Chunked {