From 78177979702517d226486b971f95e66564e7b76e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 16 Apr 2025 23:05:16 -0700 Subject: [PATCH] Drop custom CustomHttpComponentsClientHttpRequestFactory Replace `CustomHttpComponentsClientHttpRequestFactory` with `HttpComponentsClientHttpRequestFactoryBuilder` to reduce the amount of duplicated logic. Closes gh-43422 --- .../test/web/client/TestRestTemplate.java | 73 ++++++++++++++++--- .../web/client/TestRestTemplateTests.java | 67 ++++++++++++----- .../boot/web/client/RestTemplateBuilder.java | 15 +++- 3 files changed, 124 insertions(+), 31 deletions(-) diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java index a966e509fe6..b769640f990 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java @@ -25,6 +25,8 @@ import java.time.Duration; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.UnaryOperator; import javax.net.ssl.SSLContext; @@ -45,8 +47,11 @@ import org.apache.hc.core5.http.ssl.TLS; import org.apache.hc.core5.ssl.SSLContextBuilder; import org.apache.hc.core5.ssl.TrustStrategy; +import org.springframework.boot.http.client.ClientHttpRequestFactoryBuilder; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; +import org.springframework.boot.http.client.HttpComponentsClientHttpRequestFactoryBuilder; +import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.boot.web.client.RootUriTemplateHandler; import org.springframework.core.ParameterizedTypeReference; @@ -56,7 +61,6 @@ import org.springframework.http.HttpMethod; import org.springframework.http.RequestEntity; import org.springframework.http.RequestEntity.UriTemplateRequestEntity; import org.springframework.http.ResponseEntity; -import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -155,14 +159,12 @@ public class TestRestTemplate { private static RestTemplateBuilder createInitialBuilder(RestTemplateBuilder builder, String username, String password, HttpClientOption... httpClientOptions) { Assert.notNull(builder, "'builder' must not be null"); - if (httpClientOptions != null) { - ClientHttpRequestFactory requestFactory = builder.buildRequestFactory(); - if (requestFactory instanceof HttpComponentsClientHttpRequestFactory) { - builder = builder.redirects(HttpClientOption.ENABLE_REDIRECTS.isPresent(httpClientOptions) - ? Redirects.FOLLOW : Redirects.DONT_FOLLOW); - builder = builder.requestFactoryBuilder( - (settings) -> new CustomHttpComponentsClientHttpRequestFactory(httpClientOptions, settings)); - } + ClientHttpRequestFactoryBuilder requestFactoryBuilder = builder.requestFactoryBuilder(); + if (requestFactoryBuilder instanceof HttpComponentsClientHttpRequestFactoryBuilder) { + builder = builder.requestFactoryBuilder(applyHttpClientOptions( + (HttpComponentsClientHttpRequestFactoryBuilder) requestFactoryBuilder, httpClientOptions)); + builder = builder.redirects(HttpClientOption.ENABLE_REDIRECTS.isPresent(httpClientOptions) + ? Redirects.FOLLOW : Redirects.DONT_FOLLOW); } if (username != null || password != null) { builder = builder.basicAuthentication(username, password); @@ -170,6 +172,16 @@ public class TestRestTemplate { return builder; } + private static HttpComponentsClientHttpRequestFactoryBuilder applyHttpClientOptions( + HttpComponentsClientHttpRequestFactoryBuilder builder, HttpClientOption[] httpClientOptions) { + builder = builder.withDefaultRequestConfigCustomizer( + new CookieSpecCustomizer(HttpClientOption.ENABLE_COOKIES.isPresent(httpClientOptions))); + if (HttpClientOption.SSL.isPresent(httpClientOptions)) { + builder = builder.withTlsSocketStrategyFactory(new SelfSignedTlsSocketStrategyFactory()); + } + return builder; + } + /** * Configure the {@link UriTemplateHandler} to use to expand URI templates. By default * the {@link DefaultUriBuilderFactory} is used which relies on Spring's URI template @@ -1049,7 +1061,10 @@ public class TestRestTemplate { /** * {@link HttpComponentsClientHttpRequestFactory} to apply customizations. + * + * @deprecated since 3.5.0 for removal in 4.0.0 */ + @Deprecated protected static class CustomHttpComponentsClientHttpRequestFactory extends HttpComponentsClientHttpRequestFactory { private final String cookieSpec; @@ -1142,6 +1157,31 @@ public class TestRestTemplate { } + /** + * Factory used to create a {@link TlsSocketStrategy} supporting self-signed + * certificates. + */ + private static class SelfSignedTlsSocketStrategyFactory implements Function { + + private static final String[] SUPPORTED_PROTOCOLS = { TLS.V_1_3.getId(), TLS.V_1_2.getId() }; + + @Override + public TlsSocketStrategy apply(SslBundle sslBundle) { + try { + TrustSelfSignedStrategy trustStrategy = new TrustSelfSignedStrategy(); + SSLContext sslContext = new SSLContextBuilder().loadTrustMaterial(null, trustStrategy).build(); + return new DefaultClientTlsStrategy(sslContext, SUPPORTED_PROTOCOLS, null, null, null); + } + catch (KeyManagementException | NoSuchAlgorithmException | KeyStoreException ex) { + throw new IllegalStateException(ex); + } + } + + } + + /** + * {@link TrustStrategy} supporting self-signed certificates. + */ private static final class TrustSelfSignedStrategy implements TrustStrategy { @Override @@ -1151,4 +1191,19 @@ public class TestRestTemplate { } + private static class CookieSpecCustomizer implements Consumer { + + private final boolean enableCookies; + + CookieSpecCustomizer(boolean enableCookies) { + this.enableCookies = enableCookies; + } + + @Override + public void accept(RequestConfig.Builder builder) { + builder.setCookieSpec(this.enableCookies ? StandardCookieSpec.STRICT : StandardCookieSpec.IGNORE); + } + + } + } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java index 6113165fc5a..675b1a0c848 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the original author or authors. + * Copyright 2012-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,12 +26,15 @@ import java.util.Base64; import java.util.stream.Stream; import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.DefaultRedirectStrategy; +import org.apache.hc.client5.http.impl.classic.RedirectExec; +import org.apache.hc.client5.http.protocol.RedirectStrategy; +import org.assertj.core.extractor.Extractors; import org.junit.jupiter.api.Test; import org.springframework.boot.http.client.ClientHttpRequestFactoryBuilder; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; -import org.springframework.boot.test.web.client.TestRestTemplate.CustomHttpComponentsClientHttpRequestFactory; import org.springframework.boot.test.web.client.TestRestTemplate.HttpClientOption; import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.core.ParameterizedTypeReference; @@ -137,6 +140,7 @@ class TestRestTemplateTests { } @Test + @SuppressWarnings("removal") void options() { RequestConfig config = getRequestConfig( new TestRestTemplate(HttpClientOption.ENABLE_REDIRECTS, HttpClientOption.ENABLE_COOKIES)); @@ -155,25 +159,29 @@ class TestRestTemplateTests { } @Test + @SuppressWarnings("removal") void httpComponentsAreBuiltConsideringSettingsInRestTemplateBuilder() { RestTemplateBuilder builder = new RestTemplateBuilder() .requestFactoryBuilder(ClientHttpRequestFactoryBuilder.httpComponents()); - assertThat(getRequestConfig((RestTemplateBuilder) null).isRedirectsEnabled()).isFalse(); - assertThat(getRequestConfig(null, HttpClientOption.ENABLE_REDIRECTS).isRedirectsEnabled()).isTrue(); - assertThat(getRequestConfig(builder).isRedirectsEnabled()).isFalse(); - assertThat(getRequestConfig(builder, HttpClientOption.ENABLE_REDIRECTS).isRedirectsEnabled()).isTrue(); - assertThat(getRequestConfig(builder.redirects(Redirects.DONT_FOLLOW)).isRedirectsEnabled()).isFalse(); - assertThat(getRequestConfig(builder.redirects(Redirects.DONT_FOLLOW), HttpClientOption.ENABLE_REDIRECTS) - .isRedirectsEnabled()).isTrue(); + assertThat(getRedirectStrategy((RestTemplateBuilder) null)).matches(this::isDontFollowStrategy); + assertThat(getRedirectStrategy(null, HttpClientOption.ENABLE_REDIRECTS)).matches(this::isFollowStrategy); + assertThat(getRedirectStrategy(builder)).matches(this::isDontFollowStrategy); + assertThat(getRedirectStrategy(builder, HttpClientOption.ENABLE_REDIRECTS)).matches(this::isFollowStrategy); + assertThat(getRedirectStrategy(builder.redirects(Redirects.DONT_FOLLOW))).matches(this::isDontFollowStrategy); + assertThat(getRedirectStrategy(builder.redirects(Redirects.DONT_FOLLOW), HttpClientOption.ENABLE_REDIRECTS)) + .matches(this::isFollowStrategy); } @Test void withRequestFactorySettingsRedirectsForHttpComponents() { TestRestTemplate template = new TestRestTemplate(); - assertThat(getRequestConfig(template).isRedirectsEnabled()).isFalse(); - assertThat(getRequestConfig(template - .withRequestFactorySettings(ClientHttpRequestFactorySettings.defaults().withRedirects(Redirects.FOLLOW))) - .isRedirectsEnabled()).isTrue(); + assertThat(getRedirectStrategy(template)).matches(this::isDontFollowStrategy); + assertThat(getRedirectStrategy(template + .withRequestFactorySettings(ClientHttpRequestFactorySettings.defaults().withRedirects(Redirects.FOLLOW)))) + .matches(this::isFollowStrategy); + assertThat(getRedirectStrategy(template.withRequestFactorySettings( + ClientHttpRequestFactorySettings.defaults().withRedirects(Redirects.DONT_FOLLOW)))) + .matches(this::isDontFollowStrategy); } @Test @@ -196,17 +204,36 @@ class TestRestTemplateTests { .followRedirects()).isEqualTo(Redirect.NEVER); } - private RequestConfig getRequestConfig(RestTemplateBuilder builder, HttpClientOption... httpClientOptions) { + private RequestConfig getRequestConfig(TestRestTemplate template) { + ClientHttpRequestFactory requestFactory = template.getRestTemplate().getRequestFactory(); + return (RequestConfig) Extractors.byName("httpClient.defaultConfig").apply(requestFactory); + } + + private RedirectStrategy getRedirectStrategy(RestTemplateBuilder builder, HttpClientOption... httpClientOptions) { builder = (builder != null) ? builder : new RestTemplateBuilder(); TestRestTemplate template = new TestRestTemplate(builder, null, null, httpClientOptions); - return getRequestConfig(template); + return getRedirectStrategy(template); } - private RequestConfig getRequestConfig(TestRestTemplate template) { - CustomHttpComponentsClientHttpRequestFactory factory = (CustomHttpComponentsClientHttpRequestFactory) template - .getRestTemplate() - .getRequestFactory(); - return factory.createRequestConfig(); + private RedirectStrategy getRedirectStrategy(TestRestTemplate template) { + ClientHttpRequestFactory requestFactory = template.getRestTemplate().getRequestFactory(); + Object chain = Extractors.byName("httpClient.execChain").apply(requestFactory); + while (chain != null) { + Object handler = Extractors.byName("handler").apply(chain); + if (handler instanceof RedirectExec) { + return (RedirectStrategy) Extractors.byName("redirectStrategy").apply(handler); + } + chain = Extractors.byName("next").apply(chain); + } + return null; + } + + private boolean isFollowStrategy(RedirectStrategy redirectStrategy) { + return redirectStrategy instanceof DefaultRedirectStrategy; + } + + private boolean isDontFollowStrategy(RedirectStrategy redirectStrategy) { + return redirectStrategy.getClass().getName().contains("NoFollow"); } private HttpClient getJdkHttpClient(TestRestTemplate template) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java index 0aa652eb582..cc3829c968e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java @@ -750,11 +750,22 @@ public class RestTemplateBuilder { * @since 2.2.0 */ public ClientHttpRequestFactory buildRequestFactory() { + ClientHttpRequestFactoryBuilder requestFactoryBuilder = requestFactoryBuilder(); + return (requestFactoryBuilder != null) ? requestFactoryBuilder.build(this.requestFactorySettings) : null; + } + + /** + * Return a {@link ClientHttpRequestFactoryBuilder} instance using the settings of + * this builder. + * @return a {@link ClientHttpRequestFactoryBuilder} or {@code null} + * @since 3.5.0 + */ + public ClientHttpRequestFactoryBuilder requestFactoryBuilder() { if (this.requestFactoryBuilder != null) { - return this.requestFactoryBuilder.build(this.requestFactorySettings); + return this.requestFactoryBuilder; } if (this.detectRequestFactory) { - return ClientHttpRequestFactoryBuilder.detect().build(this.requestFactorySettings); + return ClientHttpRequestFactoryBuilder.detect(); } return null; }