HttpComponentsClientHttpRequestFactory setConnectionRequestTimeout not working with httpclient 5.3.1

[datagitlies]

An example project that reproduces the issue is attached 👉 timeout.zip

Dependency versions (managed by spring-boot-starter-parent v3.3.11)

• spring-boot v3.3.11
• spring-framework v6.1.19
• httpclient5 v5.3.1

Calling the setConnectionRequestTimeout method does not actually apply the timeout to the request factory.

RestTemplate restTemplate = builder
        .setReadTimeout(Duration.ofMillis(3000))
        .setConnectTimeout(Duration.ofMillis(1000))
        .requestFactory(settings -> {
            HttpComponentsClientHttpRequestFactory requestFactory = ClientHttpRequestFactories.get(HttpComponentsClientHttpRequestFactory.class, settings);
            requestFactory.setConnectionRequestTimeout(Duration.ofMillis(2000));
            return requestFactory;
        }).build();

Using the attached project run the following commands to reproduce the issue:

mvn clean install spring-boot:start
curl http://localhost:8080/test

The project has DEBUG logs enabled for httpclient5 which will show the following:

o.s.web.servlet.DispatcherServlet        : Initializing Servlet 'dispatcherServlet'
o.s.web.servlet.DispatcherServlet        : Completed initialization in 1 ms
o.a.h.c.h.i.classic.InternalHttpClient   : ex-0000000001 preparing request execution
o.a.h.c.http.impl.classic.ProtocolExec   : ex-0000000001 target auth state: UNCHALLENGED
o.a.h.c.http.impl.classic.ProtocolExec   : ex-0000000001 proxy auth state: UNCHALLENGED
o.a.h.c.http.impl.classic.ConnectExec    : ex-0000000001 acquiring connection with route {s}->https://spring.io:443
-o.a.h.c.h.i.classic.InternalHttpClient  : ex-0000000001 acquiring endpoint (3 MINUTES)
h.i.i.PoolingHttpClientConnectionManager : ex-0000000001 endpoint lease request (3 MINUTES) [route: {s}->https://spring.io:443][total available: 0; route allocated: 0 of 5; total allocated: 0 of 25]
h.i.i.PoolingHttpClientConnectionManager : ex-0000000001 endpoint leased [route: {s}->https://spring.io:443][total available: 0; route allocated: 1 of 5; total allocated: 1 of 25]
h.i.i.PoolingHttpClientConnectionManager : ex-0000000001 acquired ep-0000000001
o.a.h.c.h.i.classic.InternalHttpClient   : ex-0000000001 acquired endpoint ep-0000000001
o.a.h.c.http.impl.classic.ConnectExec    : ex-0000000001 opening connection {s}->https://spring.io:443
o.a.h.c.h.i.classic.InternalHttpClient   : ep-0000000001 connecting endpoint (null)
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 connecting endpoint to https://spring.io:443 (3 MINUTES)
.i.i.DefaultHttpClientConnectionOperator : spring.io resolving remote address
.i.i.DefaultHttpClientConnectionOperator : spring.io resolved to [spring.io/104.18.42.155, spring.io/172.64.145.101]
.i.i.DefaultHttpClientConnectionOperator : spring.io:443 connecting null->spring.io/104.18.42.155:443 (3 MINUTES)
h.i.i.DefaultManagedHttpClientConnection : http-outgoing-0 set socket timeout to 5000 MILLISECONDS
.i.i.DefaultHttpClientConnectionOperator : spring.io:443 connected null->spring.io/104.18.42.155:443 as http-outgoing-0
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 connected http-outgoing-0
o.a.h.c.h.i.classic.InternalHttpClient   : ep-0000000001 endpoint connected
o.a.h.c.h.impl.classic.MainClientExec    : ex-0000000001 executing GET /projects/spring-framework/ HTTP/1.1
o.a.h.c.h.i.classic.InternalHttpClient   : ep-0000000001 start execution ex-0000000001
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 executing exchange ex-0000000001 over http-outgoing-0
o.a.h.c.h.impl.classic.MainClientExec    : ex-0000000001 connection can be kept alive for 3 MINUTES
o.a.h.c.h.i.classic.InternalHttpClient   : ep-0000000001 releasing valid endpoint
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 releasing endpoint
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 connection http-outgoing-0 can be kept alive for 3 MINUTES
h.i.i.PoolingHttpClientConnectionManager : ep-0000000001 connection released [route: {s}->https://spring.io:443][total available: 1; route allocated: 1 of 5; total allocated: 1 of 25]

Updating the pom.xml in the attached example project to use spring-boot-starter-parent v3.4.5 fixes the issue and re-running the commands above will produce the expected result (as seen in the DEBUG logs):

+o.a.h.c.h.i.classic.InternalHttpClient  : ex-0000000001 acquiring endpoint (1000 MILLISECONDS)

It appears this issue was introduced by 11ebcee which closed #33806

[jhoeller]

The change for #33806 was trying to do it the HttpClient 5.4 and the HttpClient 5.3 way at the same time, aiming for backwards compatibility. I suppose something mismatches there on 5.3 now, possibly the check for an existing RequestConfig in the given HttpContext.

Generally we aim for supporting HttpClient 5.3 even with Spring Framework 6.2 still, so we should try to find an implementation that works on HttpClient 5.4 as well as HttpClient 5.3 for Spring Framework 6.2.7, and then backport the change to 6.1.20.

[bclozel]

I have successfully tested the following change against Spring Boot 3.3.11 and 3.4.5 using the given sample.
I guess the other behavior change that we didn't notice was that prior to 5.4, the default request config would not be null, but actually set to RequestConfig.DEFAULT.

--- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java
+++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java
@@ -235,9 +235,8 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
                        context = HttpClientContext.create();
                }

-               // Request configuration not set in the context
-               if (!(context instanceof HttpClientContext clientContext && clientContext.getRequestConfig() != null) &&
-                               context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
+               // No custom request configuration was set
+               if (!hasCustomRequestConfig(context)) {
                        RequestConfig config = null;
                        // Use request configuration given by the user, when available
                        if (httpRequest instanceof Configurable configurable) {
@@ -256,6 +255,18 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
                return new HttpComponentsClientHttpRequest(client, httpRequest, context);
        }

+       @SuppressWarnings("deprecation")  // HttpClientContext.REQUEST_CONFIG
+       private static boolean hasCustomRequestConfig(HttpContext context) {
+               if (context instanceof HttpClientContext clientContext) {
+                       // Prior to 5.4, the default config was set to RequestConfig.DEFAULT
+                       // As of 5.4, it is set to null
+                       RequestConfig requestConfig = clientContext.getRequestConfig();
+                       return requestConfig != null && !requestConfig.equals(RequestConfig.DEFAULT);
+               }
+               // Prior to 5.4, the config was stored as an attribute
+               return context.getAttribute(HttpClientContext.REQUEST_CONFIG) != null;
+       }
+

        /**
         * Create a default {@link RequestConfig} to use with the given client.

I'll push and backport the fix shortly; don't hesitate to ping me here if I've missed something.