Configure HttpClient connection timeout via ConnectionConfig#3892
Open
Configure HttpClient connection timeout via ConnectionConfig#3892
Conversation
Replace deprecated HttpComponentsClientHttpRequestFactory.setConnectTimeout() with ConnectionConfig.setConnectTimeout() on PoolingHttpClientConnectionManager. The connection timeout is now configured at the connection manager level using the recommended ConnectionConfig.Builder API.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the outbound Apache HttpClient configuration in UaaHttpRequestUtils to apply connect/read timeouts at the connection manager level (via ConnectionConfig / SocketConfig), and adjusts unit tests accordingly.
Changes:
- Move connect timeout configuration to
PoolingHttpClientConnectionManager#setDefaultConnectionConfig(ConnectionConfig). - Move socket/read timeout configuration to
PoolingHttpClientConnectionManager#setDefaultSocketConfig(SocketConfig). - Update
UaaHttpRequestUtilsTestto use the new builder/request-factory signatures and add a test asserting the configured timeouts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaHttpRequestUtils.java | Sets connect/read timeouts via connection manager defaults; simplifies request factory to only set connection-request timeout. |
| server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaHttpRequestUtilsTest.java | Updates call sites and adds a reflection-based assertion that connection/socket timeouts are applied. |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
…bled semantics (#3900) Previously, the `> 0` guards skipped timeout configuration when callers passed 0, changing behavior from the old factory-level setters where 0 explicitly disabled timeouts. Now ConnectionConfig and SocketConfig are always applied unconditionally, and Timeout.ofMilliseconds(0) correctly maps to a disabled timeout. Also rename the ambiguous `timeoutInMs` parameter to `connectionRequestTimeoutInMs` to clarify it is the pool lease timeout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HttpComponentsClientHttpRequestFactory.setConnectTimeout() was deprecated in Spring 6 and later removed in Spring 7. It is replaced with ConnectionConfig.setConnectTimeout() on PoolingHttpClientConnectionManager. The connection timeout is now configured at the connection manager level using the recommended ConnectionConfig.Builder API.
https://github.com/spring-projects/spring-framework/blob/9f431e2eac1b6d8d5ca385d0cc367bac94dd37e7/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java#L128-L133