From 859e2231e8aca63a0ed09455decb1637fb0e6660 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Tue, 18 Aug 2026 12:31:05 +0530 Subject: [PATCH 1/2] Fix unbounded server-controlled retry sleep on HTTP 429 (OKTA-1235975) The 429 retry delay was computed entirely from the server-supplied x-rate-limit-reset header with no upper bound, so a malicious or MitM server could force the client thread to sleep indefinitely. Since this can happen inside OAuth2ClientCredentials.applyToParams (synchronized), one blocked thread could stall every other caller waiting on token refresh. RetryUtil.get429DelayMillis now clamps the computed delay to a maxDelayMillis ceiling. OktaHttpRequestRetryStrategy derives that ceiling from retryMaxElapsed if explicitly configured, otherwise falls back to the same 20s cap already used for 503/504/IOException retries. DefaultClientBuilder now actually wires clientConfig.getRetryMaxElapsed() into the strategy, which was previously set on ClientConfiguration but never read anywhere. Co-Authored-By: Claude Code --- README.md | 2 + .../sdk/impl/client/DefaultClientBuilder.java | 2 +- .../retry/OktaHttpRequestRetryStrategy.java | 26 +++- .../com/okta/sdk/impl/retry/RetryUtil.java | 18 ++- .../client/DefaultClientBuilderTest.groovy | 88 +++++++++++++ .../OktaHttpRequestRetryStrategyTest.java | 99 +++++++++++++++ .../okta/sdk/impl/retry/RetryUtilTest.java | 118 +++++++++++++++++- 7 files changed, 340 insertions(+), 13 deletions(-) create mode 100644 impl/src/test/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategyTest.java diff --git a/README.md b/README.md index 5bba9ca3058..24af4cd400f 100644 --- a/README.md +++ b/README.md @@ -723,6 +723,8 @@ okta.client.requestTimeout = 0 //Sets the maximum number of seconds to wait when okta.client.rateLimit.maxRetries = 4 //Sets the maximum number of attempts to retrying before giving up. ``` +Note: a `429` response includes a server-supplied `x-rate-limit-reset` header telling the SDK how long to wait before retrying. That value is untrusted input, so the SDK always caps how long it will actually sleep for a single `429` retry: to `requestTimeout` if you've set it explicitly, or to a 20 second default ceiling otherwise (the same ceiling already applied to `503`/`504` retries). Set `requestTimeout` explicitly if your use case legitimately needs a longer per-retry wait than 20 seconds. + For interactive clients (i.e. web pages) it is optimal to set `requestTimeout` to be 10 sec (or less, based on your needs), and the `maxRetries` attempts to be 0. This means the requests will retry as many times as possible within 10 seconds: diff --git a/impl/src/main/java/com/okta/sdk/impl/client/DefaultClientBuilder.java b/impl/src/main/java/com/okta/sdk/impl/client/DefaultClientBuilder.java index 7a3101566b3..494c1b42eca 100644 --- a/impl/src/main/java/com/okta/sdk/impl/client/DefaultClientBuilder.java +++ b/impl/src/main/java/com/okta/sdk/impl/client/DefaultClientBuilder.java @@ -408,7 +408,7 @@ protected HttpClientBuilder createHttpClientBuilder(ClientConfiguration clientCo HttpClientBuilder httpClientBuilder = HttpClients.custom() .setDefaultRequestConfig(createHttpRequestConfigBuilder(clientConfig).build()) .setConnectionManager(createHttpClientConnectionManagerBuilder(clientConfig).build()) - .setRetryStrategy(new OktaHttpRequestRetryStrategy(clientConfig.getRetryMaxAttempts())) + .setRetryStrategy(new OktaHttpRequestRetryStrategy(clientConfig.getRetryMaxAttempts(), clientConfig.getRetryMaxElapsed())) .setConnectionBackoffStrategy(new DefaultBackoffStrategy()) .setKeepAliveStrategy(new DefaultConnectionKeepAliveStrategy()) .setConnectionReuseStrategy(new DefaultConnectionReuseStrategy()) diff --git a/impl/src/main/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategy.java b/impl/src/main/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategy.java index 0da9d5ada27..ae4dd8581bc 100644 --- a/impl/src/main/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategy.java +++ b/impl/src/main/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategy.java @@ -40,19 +40,37 @@ public final class OktaHttpRequestRetryStrategy extends DefaultHttpRequestRetryS private final int maxRetries; private final Set> nonRetriableIOExceptionClasses; private final Set retriableCodes; + private final long maxRetryDelayMillis; - public OktaHttpRequestRetryStrategy(int maxRetries, Collection> clazzes, Collection codes) { + public OktaHttpRequestRetryStrategy(int maxRetries, Collection> clazzes, Collection codes, int retryMaxElapsedSeconds) { Assert.isTrue(maxRetries >= 0, "maxRetries should be >= 0"); this.maxRetries = maxRetries; this.nonRetriableIOExceptionClasses = new HashSet<>(clazzes); this.retriableCodes = new HashSet<>(codes); + // A server (or a MitM) can return an arbitrarily large 429 rate-limit-reset value, so a + // client-side ceiling is always enforced. When the caller has not opted into a specific + // elapsed-time budget (retryMaxElapsedSeconds <= 0), fall back to the same cap already + // used for other retriable responses (503/504/IOExceptions). Callers who need a 429 wait + // longer than that fallback can still opt in via ClientBuilder#setRetryMaxElapsed / + // okta.client.requestTimeout, which is threaded through as retryMaxElapsedSeconds here. + this.maxRetryDelayMillis = retryMaxElapsedSeconds > 0 + ? TimeUnit.SECONDS.toMillis(retryMaxElapsedSeconds) + : RetryUtil.DEFAULT_MAX_BACKOFF_IN_MILLISECONDS; } - public OktaHttpRequestRetryStrategy(int maxRetries) { + public OktaHttpRequestRetryStrategy(int maxRetries, Collection> clazzes, Collection codes) { + this(maxRetries, clazzes, codes, 0); + } + + public OktaHttpRequestRetryStrategy(int maxRetries, int retryMaxElapsedSeconds) { this(maxRetries, Arrays.asList(InterruptedIOException.class, UnknownHostException.class, ConnectException.class, ConnectionClosedException.class, NoRouteToHostException.class, SSLException.class), - Arrays.asList(429, 503, 504)); + Arrays.asList(429, 503, 504), retryMaxElapsedSeconds); + } + + public OktaHttpRequestRetryStrategy(int maxRetries) { + this(maxRetries, 0); } @Override @@ -98,7 +116,7 @@ public TimeValue getRetryInterval(HttpResponse response, int execCount, HttpCont long delay; if (response.getCode() == 429) { - delay = RetryUtil.get429DelayMillis(response); + delay = RetryUtil.get429DelayMillis(response, maxRetryDelayMillis); } else { delay = RetryUtil.getDefaultDelayMillis(execCount); } diff --git a/impl/src/main/java/com/okta/sdk/impl/retry/RetryUtil.java b/impl/src/main/java/com/okta/sdk/impl/retry/RetryUtil.java index 15aae53aaba..6c879764bba 100644 --- a/impl/src/main/java/com/okta/sdk/impl/retry/RetryUtil.java +++ b/impl/src/main/java/com/okta/sdk/impl/retry/RetryUtil.java @@ -33,8 +33,10 @@ public class RetryUtil { /** * Maximum exponential back-off time before retrying a request (20 seconds). + * Also used as the fallback ceiling for {@link #get429DelayMillis} when no + * explicit {@code retryMaxElapsed} value has been configured. */ - private static final int DEFAULT_MAX_BACKOFF_IN_MILLISECONDS = 20 * 1000; + static final int DEFAULT_MAX_BACKOFF_IN_MILLISECONDS = 20 * 1000; /** * Initial backoff delay in milliseconds for exponential backoff calculation. @@ -75,11 +77,16 @@ static long getDefaultDelayMillis(int retries) { /** * Calculates the delay in milliseconds for a 429 (Too Many Requests) response. * Uses the x-rate-limit-reset header to determine when the rate limit resets. - * + * + *

The server-supplied reset time is untrusted input (e.g. a MitM or malicious + * server could return an arbitrarily large value), so the result is always capped + * at {@code maxDelayMillis} to prevent an unbounded thread sleep.

+ * * @param response The HTTP response containing rate limit headers + * @param maxDelayMillis The maximum delay to allow, regardless of the reset header value * @return The delay in milliseconds, or -1 if headers are missing/invalid */ - static long get429DelayMillis(HttpResponse response) { + static long get429DelayMillis(HttpResponse response, long maxDelayMillis) { // the time at which the rate limit will reset, specified in UTC epoch time. long resetLimit = getRateLimitResetValue(response); if (resetLimit == -1L) { @@ -95,8 +102,9 @@ static long get429DelayMillis(HttpResponse response) { long waitUntil = resetLimit * 1000L; long requestTime = requestDate.getTime(); long delay = Math.max(waitUntil - requestTime + RATE_LIMIT_BUFFER_MS, MIN_RETRY_DELAY_MS); - logger.debug("429 wait: Math.max({} - {} + {}ms), {}ms = {})", - waitUntil, requestTime, RATE_LIMIT_BUFFER_MS, MIN_RETRY_DELAY_MS, delay); + delay = Math.min(delay, maxDelayMillis); + logger.debug("429 wait: Math.min(Math.max({} - {} + {}ms, {}ms), {}ms) = {})", + waitUntil, requestTime, RATE_LIMIT_BUFFER_MS, MIN_RETRY_DELAY_MS, maxDelayMillis, delay); return delay; } diff --git a/impl/src/test/groovy/com/okta/sdk/impl/client/DefaultClientBuilderTest.groovy b/impl/src/test/groovy/com/okta/sdk/impl/client/DefaultClientBuilderTest.groovy index 11294db5719..c01e8d824ca 100644 --- a/impl/src/test/groovy/com/okta/sdk/impl/client/DefaultClientBuilderTest.groovy +++ b/impl/src/test/groovy/com/okta/sdk/impl/client/DefaultClientBuilderTest.groovy @@ -32,16 +32,25 @@ import com.okta.sdk.impl.test.RestoreEnvironmentVariables import com.okta.sdk.impl.test.RestoreSystemProperties import com.okta.sdk.resource.client.ApiClient import com.okta.sdk.resource.client.Configuration +import org.apache.hc.client5.http.HttpRequestRetryStrategy +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder import org.apache.hc.client5.http.impl.classic.HttpClients +import org.apache.hc.core5.http.Header +import org.apache.hc.core5.http.HttpResponse +import org.apache.hc.core5.http.protocol.HttpContext +import org.apache.hc.core5.util.TimeValue import org.mockito.invocation.InvocationOnMock import org.mockito.stubbing.Answer import org.testng.annotations.Listeners import org.testng.annotations.Test +import java.lang.reflect.Field import java.nio.file.Path import java.security.KeyPair import java.security.KeyPairGenerator import java.security.PrivateKey +import java.text.SimpleDateFormat +import java.util.concurrent.TimeUnit import static org.hamcrest.MatcherAssert.assertThat import static org.hamcrest.Matchers.is @@ -441,8 +450,87 @@ class DefaultClientBuilderTest { assertThat clientBuilder.clientConfiguration.getKid(), is("kid-value") } + // Regression coverage for OKTA-1235975: an unbounded server-controlled retry sleep on HTTP 429. + // A malicious/MitM server can return an arbitrarily large x-rate-limit-reset value; the retry + // strategy built here must always cap the resulting sleep, whether or not retryMaxElapsed was + // explicitly configured. + // + // Unlike OktaHttpRequestRetryStrategyTest (which exercises the capping math directly against a + // strategy instance), these tests go through the real DefaultClientBuilder.createHttpClientBuilder + // wiring - so a regression like swapping getRetryMaxAttempts()/getRetryMaxElapsed() arguments at + // the call site would be caught here even though the strategy's own unit tests would still pass. + + @Test + void testRetryStrategyDefaultRetryMaxElapsedStillBoundsMalicious429Delay() { + clearOktaEnvAndSysProps() + DefaultClientBuilder clientBuilder = new DefaultClientBuilder(noDefaultYamlNoAppYamlResourceFactory()) + // retryMaxElapsed intentionally left unconfigured (defaults to 0 / "no explicit limit") + + HttpRequestRetryStrategy retryStrategy = buildRetryStrategy(clientBuilder) + TimeValue retryInterval = retryStrategy.getRetryInterval(maliciousRateLimitResponse(), 1, mock(HttpContext)) + + // must never honor the malicious ~1 year delay; a sane client-side ceiling always applies + assertTrue retryInterval.toMilliseconds() <= TimeUnit.MINUTES.toMillis(1) + } + + @Test + void testRetryStrategyHonorsConfiguredRetryMaxElapsedFor429Delay() { + clearOktaEnvAndSysProps() + DefaultClientBuilder clientBuilder = new DefaultClientBuilder(noDefaultYamlNoAppYamlResourceFactory()) + clientBuilder.setRetryMaxElapsed(5) // seconds + + HttpRequestRetryStrategy retryStrategy = buildRetryStrategy(clientBuilder) + TimeValue retryInterval = retryStrategy.getRetryInterval(maliciousRateLimitResponse(), 1, mock(HttpContext)) + + assertEquals(TimeUnit.SECONDS.toMillis(5) as long, retryInterval.toMilliseconds() as long) + } + // helper methods + /** + * Builds the retry strategy exactly as DefaultClientBuilder wires it up in createHttpClientBuilder, + * so the test exercises the real wiring rather than re-testing OktaHttpRequestRetryStrategy in isolation. + * + * HttpClientBuilder doesn't expose a public getter for the configured retry strategy, so this + * reaches into its private `retryStrategy` field via reflection. That's a known trade-off: it + * would break if Apache HttpClient renamed/removed that field. Accepted here because it's the + * only way to catch a wiring regression (e.g. swapped constructor arguments) without it; if + * client5 ever restructures HttpClientBuilder, delete these two tests rather than fight the + * reflection - the direct OktaHttpRequestRetryStrategy tests remain the source of truth for the + * capping behavior itself. + */ + static HttpRequestRetryStrategy buildRetryStrategy(DefaultClientBuilder clientBuilder) { + HttpClientBuilder httpClientBuilder = + clientBuilder.createHttpClientBuilder(clientBuilder.getClientConfiguration()) + Field field = HttpClientBuilder.class.getDeclaredField("retryStrategy") + field.setAccessible(true) + return (HttpRequestRetryStrategy) field.get(httpClientBuilder) + } + + /** + * A 429 response with an x-rate-limit-reset one year in the future, simulating a malicious or + * MitM server trying to force an excessively long client-side sleep. + */ + static HttpResponse maliciousRateLimitResponse() { + HttpResponse response = mock(HttpResponse) + when(response.getCode()).thenReturn(429) + + long currentTime = System.currentTimeMillis() + long maliciousResetTime = currentTime / 1000 + TimeUnit.DAYS.toSeconds(365) + + Header resetHeader = mock(Header) + when(resetHeader.getValue()).thenReturn(String.valueOf(maliciousResetTime)) + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader) + + Header dateHeader = mock(Header) + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US) + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")) + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))) + when(response.getHeader("Date")).thenReturn(dateHeader) + + return response + } + static generatePrivateKey(String algorithm, int keySize, String fileNamePrefix, String fileNameSuffix) { KeyPairGenerator keyGen = KeyPairGenerator.getInstance(algorithm) keyGen.initialize(keySize) diff --git a/impl/src/test/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategyTest.java b/impl/src/test/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategyTest.java new file mode 100644 index 00000000000..ccac707b742 --- /dev/null +++ b/impl/src/test/java/com/okta/sdk/impl/retry/OktaHttpRequestRetryStrategyTest.java @@ -0,0 +1,99 @@ +/* + * Copyright 2026-Present Okta, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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 com.okta.sdk.impl.retry; + +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.TimeValue; +import org.testng.annotations.Test; + +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.Locale; +import java.util.TimeZone; +import java.util.concurrent.TimeUnit; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +public class OktaHttpRequestRetryStrategyTest { + + /** + * Regression test for a server (or MitM) returning an unbounded x-rate-limit-reset value on a + * 429 response. Without a client-side ceiling, this would cause getRetryInterval to sleep for + * an excessively long time (e.g. years), which can block callers indefinitely. + */ + @Test + public void testGetRetryIntervalCaps429DelayWithDefaultConfig() throws ProtocolException { + // no explicit retryMaxElapsed configured -> falls back to the default 20s cap + OktaHttpRequestRetryStrategy strategy = new OktaHttpRequestRetryStrategy(4); + + HttpResponse response = maliciousRateLimitResponse(); + TimeValue retryInterval = strategy.getRetryInterval(response, 1, mock(HttpContext.class)); + + assertEquals(RetryUtil.DEFAULT_MAX_BACKOFF_IN_MILLISECONDS, retryInterval.toMilliseconds()); + } + + @Test + public void testGetRetryIntervalHonorsConfiguredRetryMaxElapsed() throws ProtocolException { + int retryMaxElapsedSeconds = 5; + OktaHttpRequestRetryStrategy strategy = new OktaHttpRequestRetryStrategy(4, retryMaxElapsedSeconds); + + HttpResponse response = maliciousRateLimitResponse(); + TimeValue retryInterval = strategy.getRetryInterval(response, 1, mock(HttpContext.class)); + + assertEquals(TimeUnit.SECONDS.toMillis(retryMaxElapsedSeconds), retryInterval.toMilliseconds()); + } + + @Test + public void testGetRetryIntervalForNon429UsesExponentialBackoff() { + OktaHttpRequestRetryStrategy strategy = new OktaHttpRequestRetryStrategy(4); + + HttpResponse response = mock(HttpResponse.class); + when(response.getCode()).thenReturn(503); + + TimeValue retryInterval = strategy.getRetryInterval(response, 3, mock(HttpContext.class)); + assertTrue(retryInterval.toMilliseconds() > 0 && retryInterval.toMilliseconds() <= RetryUtil.DEFAULT_MAX_BACKOFF_IN_MILLISECONDS); + } + + /** + * Builds a 429 response with an x-rate-limit-reset one year in the future, simulating a + * malicious/MitM server trying to force an excessively long client-side sleep. + */ + private static HttpResponse maliciousRateLimitResponse() throws ProtocolException { + HttpResponse response = mock(HttpResponse.class); + when(response.getCode()).thenReturn(429); + + long currentTime = System.currentTimeMillis(); + long maliciousResetTime = currentTime / 1000 + TimeUnit.DAYS.toSeconds(365); + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(maliciousResetTime)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))); + when(response.getHeader("Date")).thenReturn(dateHeader); + + return response; + } +} diff --git a/impl/src/test/java/com/okta/sdk/impl/retry/RetryUtilTest.java b/impl/src/test/java/com/okta/sdk/impl/retry/RetryUtilTest.java index 5834874dd96..86656117f8c 100644 --- a/impl/src/test/java/com/okta/sdk/impl/retry/RetryUtilTest.java +++ b/impl/src/test/java/com/okta/sdk/impl/retry/RetryUtilTest.java @@ -25,6 +25,7 @@ import java.util.Date; import java.util.Locale; import java.util.TimeZone; +import java.util.concurrent.TimeUnit; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -119,16 +120,127 @@ public void testGet429DelayMillis() throws ProtocolException { when(dateHeader.getValue()).thenReturn(httpDateString); when(response.getHeader("Date")).thenReturn(dateHeader); - // Delay should be approximately 30 seconds (30000ms) plus the 1000ms padding - long delay = RetryUtil.get429DelayMillis(response); + // Delay should be approximately 30 seconds (30000ms) plus the 1000ms padding, + // well within the (generous) 60 second max used here + long delay = RetryUtil.get429DelayMillis(response, 60_000L); assertTrue(delay >= 30000 && delay <= 32000); // Test with missing rate limit header HttpResponse noResetResponse = mock(HttpResponse.class); when(noResetResponse.getFirstHeader("x-rate-limit-reset")).thenReturn(null); - assertEquals(-1, RetryUtil.get429DelayMillis(noResetResponse)); + assertEquals(-1, RetryUtil.get429DelayMillis(noResetResponse, 60_000L)); } + @Test + public void testGet429DelayMillisIsCappedByMaxDelay() throws ProtocolException { + // Simulates a malicious/MitM server returning an x-rate-limit-reset far in the future + // to try to force an excessively long thread sleep. + HttpResponse response = mock(HttpResponse.class); + + long currentTime = System.currentTimeMillis(); + long resetTime = currentTime / 1000 + TimeUnit.DAYS.toSeconds(365); // 1 year in the future + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(resetTime)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))); + when(response.getHeader("Date")).thenReturn(dateHeader); + + long maxDelayMillis = 20_000L; + long delay = RetryUtil.get429DelayMillis(response, maxDelayMillis); + assertEquals(maxDelayMillis, delay); + } + + @Test + public void testGet429DelayMillisBelowMaxDelayIsUnaffected() throws ProtocolException { + HttpResponse response = mock(HttpResponse.class); + + long currentTime = System.currentTimeMillis(); + long resetTime = currentTime / 1000 + 5; // 5 seconds in the future + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(resetTime)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))); + when(response.getHeader("Date")).thenReturn(dateHeader); + + // 5s wait + 1s buffer = ~6s, well under the 20s max, so the max should not kick in + long delay = RetryUtil.get429DelayMillis(response, 20_000L); + assertTrue(delay >= 6000 && delay <= 7000); + } + + @Test + public void testGet429DelayMillisCapWinsOverMinimumFloor() throws ProtocolException { + // Even a "normal" (small, non-malicious) reset value must still respect an aggressively + // small configured max, i.e. the safety ceiling always wins over MIN_RETRY_DELAY_MS. + HttpResponse response = mock(HttpResponse.class); + + long currentTime = System.currentTimeMillis(); + long resetTime = currentTime / 1000 + 30; + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(resetTime)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))); + when(response.getHeader("Date")).thenReturn(dateHeader); + + long maxDelayMillis = 500L; // below MIN_RETRY_DELAY_MS (1000ms) + assertEquals(maxDelayMillis, RetryUtil.get429DelayMillis(response, maxDelayMillis)); + } + + @Test + public void testGet429DelayMillisWithOverflowingResetHeaderIsStillBounded() throws ProtocolException { + // A malicious server could try to overflow the internal long math (resetLimit * 1000L) by + // sending a value near Long.MAX_VALUE. Regardless of how that overflow resolves, the final + // Math.min(..., maxDelayMillis) clamp must guarantee the result never exceeds maxDelayMillis. + HttpResponse response = mock(HttpResponse.class); + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(Long.MAX_VALUE)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date())); + when(response.getHeader("Date")).thenReturn(dateHeader); + + long maxDelayMillis = 20_000L; + long delay = RetryUtil.get429DelayMillis(response, maxDelayMillis); + assertTrue(delay >= 0 && delay <= maxDelayMillis); + } + + @Test + public void testGet429DelayMillisWithExpiredResetTimeUsesMinimum() throws ProtocolException { + // Reset time already in the past should fall back to MIN_RETRY_DELAY_MS, not a negative delay. + HttpResponse response = mock(HttpResponse.class); + long currentTime = System.currentTimeMillis(); + long resetTime = currentTime / 1000 - 3600; // 1 hour in the past + + Header resetHeader = mock(Header.class); + when(resetHeader.getValue()).thenReturn(String.valueOf(resetTime)); + when(response.getFirstHeader("x-rate-limit-reset")).thenReturn(resetHeader); + + Header dateHeader = mock(Header.class); + SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + when(dateHeader.getValue()).thenReturn(dateFormat.format(new Date(currentTime))); + when(response.getHeader("Date")).thenReturn(dateHeader); + + assertEquals(1000L, RetryUtil.get429DelayMillis(response, 20_000L)); + } } From 11412bf21f30b5bb6744a050c7101b2d648448c3 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Tue, 18 Aug 2026 12:31:20 +0530 Subject: [PATCH 2/2] fix: bump httpclient5 to 5.6.4 to resolve CVE-2026-64607 httpclient5 5.6.2 fails to release the underlying connection back to the connection manager on an invalid/unsupported Content-Encoding header value in the classic I/O model, allowing connection pool exhaustion (DoS). Fixed upstream in 5.6.3+; bumping to the latest 5.6.x patch release. Co-Authored-By: Claude Code --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index bc09998c6a3..276015487eb 100644 --- a/pom.xml +++ b/pom.xml @@ -38,7 +38,7 @@ 2.4 1.84 0.12.6 - 5.6.2 + 5.6.4 24.0.1 2.0.1 1.1.1