From 36a22fcd5969050269dc4d652e7bb7a686ac4f51 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 25 Oct 2024 00:46:01 -0700 Subject: [PATCH] Unify HTTP client redirect behavior and provide configuration option Update `ClientHttpRequestFactoryBuilder` implementations to ensure that all libraries have consistent redirect follow behavior. Following of redirects is enabled by default. The `ClientHttpRequestFactorySettings` may be used to change if redirects should be followed. The `spring.http.client.redirects` property may also be used to update the default behavior. Closes gh-42879 --- .../client/HttpClientAutoConfiguration.java | 4 +- .../http/client/HttpClientProperties.java | 14 ++++ .../HttpClientAutoConfigurationTests.java | 6 +- .../test/web/client/TestRestTemplate.java | 2 +- .../ClientHttpRequestFactorySettings.java | 49 ++++++++++++-- ...onentsClientHttpRequestFactoryBuilder.java | 64 +++++++++++++++---- .../JdkClientHttpRequestFactoryBuilder.java | 25 +++++--- .../JettyClientHttpRequestFactoryBuilder.java | 45 +++++++++---- ...eactorClientHttpRequestFactoryBuilder.java | 21 ++++-- ...onentsClientHttpRequestFactoryBuilder.java | 5 +- ...SimpleClientHttpRequestFactoryBuilder.java | 20 +++--- .../ClientHttpRequestFactorySettings.java | 2 +- ...tClientHttpRequestFactoryBuilderTests.java | 47 ++++++++++++++ ...ClientHttpRequestFactorySettingsTests.java | 20 +++++- ...sClientHttpRequestFactoryBuilderTests.java | 17 +++++ .../SampleActuatorUiApplicationPortTests.java | 3 +- .../ui/SampleActuatorUiApplicationTests.java | 7 +- .../SampleSessionJdbcApplicationTests.java | 22 ++++++- .../SampleGroovyTemplateApplicationTests.java | 4 +- .../SampleWebUiApplicationTests.java | 4 +- 20 files changed, 307 insertions(+), 74 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfiguration.java index ee4483caf2e..faa6273442d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfiguration.java @@ -58,8 +58,8 @@ public class HttpClientAutoConfiguration { ClientHttpRequestFactorySettings clientHttpRequestFactorySettings(HttpClientProperties httpClientProperties, ObjectProvider sslBundles) { SslBundle sslBundle = getSslBundle(httpClientProperties.getSsl(), sslBundles); - return new ClientHttpRequestFactorySettings(httpClientProperties.getConnectTimeout(), - httpClientProperties.getReadTimeout(), sslBundle); + return new ClientHttpRequestFactorySettings(httpClientProperties.getRedirects(), + httpClientProperties.getConnectTimeout(), httpClientProperties.getReadTimeout(), sslBundle); } private SslBundle getSslBundle(HttpClientProperties.Ssl properties, ObjectProvider sslBundles) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientProperties.java index f11faf7aaa4..3117da8484e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/client/HttpClientProperties.java @@ -21,6 +21,7 @@ import java.util.function.Supplier; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.http.client.ClientHttpRequestFactoryBuilder; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; /** * {@link ConfigurationProperties @ConfigurationProperties} for a Spring's blocking HTTP @@ -37,6 +38,11 @@ public class HttpClientProperties { */ private Factory factory; + /** + * Handling for HTTP redirects. + */ + private Redirects redirects = Redirects.FOLLOW_WHEN_POSSIBLE; + /** * Default connect timeout for a client HTTP request. */ @@ -60,6 +66,14 @@ public class HttpClientProperties { this.factory = factory; } + public Redirects getRedirects() { + return this.redirects; + } + + public void setRedirects(Redirects redirects) { + this.redirects = redirects; + } + public Duration getConnectTimeout() { return this.connectTimeout; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfigurationTests.java index 8f3849354bf..85a23fae4a0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/http/client/HttpClientAutoConfigurationTests.java @@ -26,6 +26,7 @@ import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration; 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.SimpleClientHttpRequestFactoryBuilder; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; @@ -60,10 +61,11 @@ class HttpClientAutoConfigurationTests { @Test void configuresClientHttpRequestFactorySettings() { this.contextRunner.withPropertyValues(sslPropertyValues().toArray(String[]::new)) - .withPropertyValues("spring.http.client.connect-timeout=10s", "spring.http.client.read-timeout=20s", - "spring.http.client.ssl.bundle=test") + .withPropertyValues("spring.http.client.redirects=dont-follow", "spring.http.client.connect-timeout=10s", + "spring.http.client.read-timeout=20s", "spring.http.client.ssl.bundle=test") .run((context) -> { ClientHttpRequestFactorySettings settings = context.getBean(ClientHttpRequestFactorySettings.class); + assertThat(settings.redirects()).isEqualTo(Redirects.DONT_FOLLOW); assertThat(settings.connectTimeout()).isEqualTo(Duration.ofSeconds(10)); assertThat(settings.readTimeout()).isEqualTo(Duration.ofSeconds(20)); assertThat(settings.sslBundle().getKey().getAlias()).isEqualTo("alias1"); 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 c3c69d87594..a8d7cf2e939 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 @@ -1018,7 +1018,7 @@ public class TestRestTemplate { @SuppressWarnings("removal") public CustomHttpComponentsClientHttpRequestFactory(HttpClientOption[] httpClientOptions, org.springframework.boot.web.client.ClientHttpRequestFactorySettings settings) { - this(httpClientOptions, new ClientHttpRequestFactorySettings(settings.connectTimeout(), + this(httpClientOptions, new ClientHttpRequestFactorySettings(null, settings.connectTimeout(), settings.readTimeout(), settings.sslBundle())); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactorySettings.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactorySettings.java index 43180c871d2..37a418383f4 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactorySettings.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ClientHttpRequestFactorySettings.java @@ -24,6 +24,8 @@ import org.springframework.http.client.ClientHttpRequestFactory; /** * Settings that can be applied when creating a {@link ClientHttpRequestFactory}. * + * @param redirects the follow redirect strategy to use or null to redirect whenever the + * underlying library allows it * @param connectTimeout the connect timeout * @param readTimeout the read timeout * @param sslBundle the SSL bundle providing SSL configuration @@ -33,10 +35,15 @@ import org.springframework.http.client.ClientHttpRequestFactory; * @since 3.4.0 * @see ClientHttpRequestFactoryBuilder */ -public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration readTimeout, SslBundle sslBundle) { +public record ClientHttpRequestFactorySettings(Redirects redirects, Duration connectTimeout, Duration readTimeout, + SslBundle sslBundle) { private static final ClientHttpRequestFactorySettings defaults = new ClientHttpRequestFactorySettings(null, null, - null); + null, null); + + public ClientHttpRequestFactorySettings { + redirects = (redirects != null) ? redirects : Redirects.FOLLOW_WHEN_POSSIBLE; + } /** * Return a new {@link ClientHttpRequestFactorySettings} instance with an updated @@ -45,7 +52,7 @@ public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration * @return a new {@link ClientHttpRequestFactorySettings} instance */ public ClientHttpRequestFactorySettings withConnectTimeout(Duration connectTimeout) { - return new ClientHttpRequestFactorySettings(connectTimeout, this.readTimeout, this.sslBundle); + return new ClientHttpRequestFactorySettings(this.redirects, connectTimeout, this.readTimeout, this.sslBundle); } /** @@ -56,7 +63,7 @@ public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration */ public ClientHttpRequestFactorySettings withReadTimeout(Duration readTimeout) { - return new ClientHttpRequestFactorySettings(this.connectTimeout, readTimeout, this.sslBundle); + return new ClientHttpRequestFactorySettings(this.redirects, this.connectTimeout, readTimeout, this.sslBundle); } /** @@ -66,7 +73,17 @@ public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration * @return a new {@link ClientHttpRequestFactorySettings} instance */ public ClientHttpRequestFactorySettings withSslBundle(SslBundle sslBundle) { - return new ClientHttpRequestFactorySettings(this.connectTimeout, this.readTimeout, sslBundle); + return new ClientHttpRequestFactorySettings(this.redirects, this.connectTimeout, this.readTimeout, sslBundle); + } + + /** + * Return a new {@link ClientHttpRequestFactorySettings} instance with an updated + * redirect setting. + * @param redirects the new redirects setting + * @return a new {@link ClientHttpRequestFactorySettings} instance + */ + public ClientHttpRequestFactorySettings withRedirects(Redirects redirects) { + return new ClientHttpRequestFactorySettings(redirects, this.connectTimeout, this.readTimeout, this.sslBundle); } /** @@ -88,4 +105,26 @@ public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration return defaults; } + /** + * Redirect strategies. + */ + public enum Redirects { + + /** + * Follow redirects (if the underlying library has support). + */ + FOLLOW_WHEN_POSSIBLE, + + /** + * Follow redirects (fail if the underlying library has not support). + */ + FOLLOW, + + /** + * Don't follow redirects (fail if the underlying library has not support). + */ + DONT_FOLLOW + + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/HttpComponentsClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/HttpComponentsClientHttpRequestFactoryBuilder.java index cb8efd0a57c..f5cac48423e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/HttpComponentsClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/HttpComponentsClientHttpRequestFactoryBuilder.java @@ -16,6 +16,7 @@ package org.springframework.boot.http.client; +import java.net.URI; import java.time.Duration; import java.util.Collection; import java.util.Collections; @@ -24,14 +25,21 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.client5.http.impl.DefaultRedirectStrategy; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.client5.http.protocol.RedirectStrategy; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.io.SocketConfig; +import org.apache.hc.core5.http.protocol.HttpContext; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslOptions; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; @@ -72,31 +80,35 @@ public final class HttpComponentsClientHttpRequestFactoryBuilder @Override protected HttpComponentsClientHttpRequestFactory createClientHttpRequestFactory( ClientHttpRequestFactorySettings settings) { - HttpClient httpClient = createHttpClient(settings.readTimeout(), settings.sslBundle()); + HttpClient httpClient = createHttpClient(settings); HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory(httpClient); PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); map.from(settings::connectTimeout).asInt(Duration::toMillis).to(factory::setConnectTimeout); return factory; } - private HttpClient createHttpClient(Duration readTimeout, SslBundle sslBundle) { + private HttpClient createHttpClient(ClientHttpRequestFactorySettings settings) { return HttpClientBuilder.create() .useSystemProperties() - .setConnectionManager(createConnectionManager(readTimeout, sslBundle)) + .setRedirectStrategy(asRedirectStrategy(settings.redirects())) + .setConnectionManager(createConnectionManager(settings)) .build(); } - private PoolingHttpClientConnectionManager createConnectionManager(Duration readTimeout, SslBundle sslBundle) { - PoolingHttpClientConnectionManagerBuilder connectionManagerBuilder = PoolingHttpClientConnectionManagerBuilder - .create(); - if (readTimeout != null) { - connectionManagerBuilder.setDefaultSocketConfig(createSocketConfig(readTimeout)); - } - if (sslBundle != null) { - connectionManagerBuilder.setTlsSocketStrategy(createTlsSocketStrategy(sslBundle)); - } - PoolingHttpClientConnectionManager connectionManager = connectionManagerBuilder.useSystemProperties().build(); - return connectionManager; + private RedirectStrategy asRedirectStrategy(Redirects redirects) { + return switch (redirects) { + case FOLLOW_WHEN_POSSIBLE -> DefaultRedirectStrategy.INSTANCE; + case FOLLOW -> DefaultRedirectStrategy.INSTANCE; + case DONT_FOLLOW -> NoFollowRedirectStrategy.INSTANCE; + }; + } + + private PoolingHttpClientConnectionManager createConnectionManager(ClientHttpRequestFactorySettings settings) { + PoolingHttpClientConnectionManagerBuilder builder = PoolingHttpClientConnectionManagerBuilder.create(); + PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); + map.from(settings::readTimeout).as(this::createSocketConfig).to(builder::setDefaultSocketConfig); + map.from(settings::sslBundle).as(this::createTlsSocketStrategy).to(builder::setTlsSocketStrategy); + return builder.useSystemProperties().build(); } private DefaultClientTlsStrategy createTlsSocketStrategy(SslBundle sslBundle) { @@ -110,6 +122,30 @@ public final class HttpComponentsClientHttpRequestFactoryBuilder return SocketConfig.custom().setSoTimeout((int) readTimeout.toMillis(), TimeUnit.MILLISECONDS).build(); } + /** + * {@link RedirectStrategy} that never follows redirects. + */ + private static final class NoFollowRedirectStrategy implements RedirectStrategy { + + private static final RedirectStrategy INSTANCE = new NoFollowRedirectStrategy(); + + private NoFollowRedirectStrategy() { + } + + @Override + public boolean isRedirected(HttpRequest request, HttpResponse response, HttpContext context) + throws HttpException { + return false; + } + + @Override + public URI getLocationURI(HttpRequest request, HttpResponse response, HttpContext context) + throws HttpException { + return null; + } + + } + static class Classes { static final String HTTP_CLIENTS = "org.apache.hc.client5.http.impl.classic.HttpClients"; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JdkClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JdkClientHttpRequestFactoryBuilder.java index 2847dc384ec..deb12aa9f49 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JdkClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JdkClientHttpRequestFactoryBuilder.java @@ -17,12 +17,13 @@ package org.springframework.boot.http.client; import java.net.http.HttpClient; -import java.time.Duration; +import java.net.http.HttpClient.Redirect; import java.util.Collection; import java.util.List; import java.util.function.Consumer; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.http.client.JdkClientHttpRequestFactory; import org.springframework.util.ClassUtils; @@ -59,24 +60,30 @@ public class JdkClientHttpRequestFactoryBuilder @Override protected JdkClientHttpRequestFactory createClientHttpRequestFactory(ClientHttpRequestFactorySettings settings) { - HttpClient httpClient = createHttpClient(settings.connectTimeout(), settings.sslBundle()); + HttpClient httpClient = createHttpClient(settings); JdkClientHttpRequestFactory requestFactory = new JdkClientHttpRequestFactory(httpClient); PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); map.from(settings::readTimeout).to(requestFactory::setReadTimeout); return requestFactory; } - private HttpClient createHttpClient(Duration connectTimeout, SslBundle sslBundle) { + private HttpClient createHttpClient(ClientHttpRequestFactorySettings settings) { HttpClient.Builder httpClientBuilder = HttpClient.newBuilder(); - if (connectTimeout != null) { - httpClientBuilder.connectTimeout(connectTimeout); - } - if (sslBundle != null) { - httpClientBuilder.sslContext(sslBundle.createSslContext()); - } + PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); + map.from(settings::connectTimeout).to(httpClientBuilder::connectTimeout); + map.from(settings::sslBundle).as(SslBundle::createSslContext).to(httpClientBuilder::sslContext); + map.from(settings::redirects).as(this::asHttpClientRedirect).to(httpClientBuilder::followRedirects); return httpClientBuilder.build(); } + private Redirect asHttpClientRedirect(Redirects redirects) { + return switch (redirects) { + case FOLLOW_WHEN_POSSIBLE -> Redirect.NORMAL; + case FOLLOW -> Redirect.NORMAL; + case DONT_FOLLOW -> Redirect.NEVER; + }; + } + static class Classes { static final String HTTP_CLIENT = "java.net.http.HttpClient"; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JettyClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JettyClientHttpRequestFactoryBuilder.java index 0363656e7e6..17efd009a11 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JettyClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/JettyClientHttpRequestFactoryBuilder.java @@ -24,11 +24,14 @@ import java.util.function.Consumer; import javax.net.ssl.SSLContext; import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.client.transport.HttpClientTransportDynamic; +import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.http.client.JettyClientHttpRequestFactory; import org.springframework.util.ClassUtils; @@ -65,24 +68,44 @@ public final class JettyClientHttpRequestFactoryBuilder @Override protected JettyClientHttpRequestFactory createClientHttpRequestFactory(ClientHttpRequestFactorySettings settings) { - JettyClientHttpRequestFactory requestFactory = createRequestFactory(settings.sslBundle()); + JettyClientHttpRequestFactory requestFactory = createRequestFactory(settings); PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); map.from(settings::connectTimeout).asInt(Duration::toMillis).to(requestFactory::setConnectTimeout); map.from(settings::readTimeout).asInt(Duration::toMillis).to(requestFactory::setReadTimeout); return requestFactory; } - private static JettyClientHttpRequestFactory createRequestFactory(SslBundle sslBundle) { - if (sslBundle != null) { - SSLContext sslContext = sslBundle.createSslContext(); - SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(); - sslContextFactory.setSslContext(sslContext); - ClientConnector connector = new ClientConnector(); - connector.setSslContextFactory(sslContextFactory); - HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic(connector)); - return new JettyClientHttpRequestFactory(httpClient); + private JettyClientHttpRequestFactory createRequestFactory(ClientHttpRequestFactorySettings settings) { + HttpClientTransport transport = createTransport(settings); + HttpClient httpClient = new HttpClient(transport); + PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); + map.from(settings::redirects).as(this::followRedirects).to(httpClient::setFollowRedirects); + return new JettyClientHttpRequestFactory(httpClient); + } + + private HttpClientTransport createTransport(ClientHttpRequestFactorySettings settings) { + if (settings.sslBundle() == null) { + return new HttpClientTransportOverHTTP(); } - return new JettyClientHttpRequestFactory(); + ClientConnector connector = createClientConnector(settings.sslBundle()); + return new HttpClientTransportDynamic(connector); + } + + private ClientConnector createClientConnector(SslBundle sslBundle) { + SSLContext sslContext = sslBundle.createSslContext(); + SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(); + sslContextFactory.setSslContext(sslContext); + ClientConnector connector = new ClientConnector(); + connector.setSslContextFactory(sslContextFactory); + return connector; + } + + private boolean followRedirects(Redirects redirects) { + return switch (redirects) { + case FOLLOW_WHEN_POSSIBLE -> true; + case FOLLOW -> true; + case DONT_FOLLOW -> false; + }; } static class Classes { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReactorClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReactorClientHttpRequestFactoryBuilder.java index 1c45655fa62..e443a25952d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReactorClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReactorClientHttpRequestFactoryBuilder.java @@ -28,6 +28,7 @@ import reactor.netty.http.client.HttpClient; import reactor.netty.tcp.SslProvider.SslContextSpec; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslManagerBundle; import org.springframework.boot.ssl.SslOptions; @@ -68,22 +69,30 @@ public final class ReactorClientHttpRequestFactoryBuilder @Override protected ReactorClientHttpRequestFactory createClientHttpRequestFactory( ClientHttpRequestFactorySettings settings) { - ReactorClientHttpRequestFactory requestFactory = createRequestFactory(settings.sslBundle()); + ReactorClientHttpRequestFactory requestFactory = createRequestFactory(settings); PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); map.from(settings::connectTimeout).asInt(Duration::toMillis).to(requestFactory::setConnectTimeout); map.from(settings::readTimeout).asInt(Duration::toMillis).to(requestFactory::setReadTimeout); return requestFactory; } - private ReactorClientHttpRequestFactory createRequestFactory(SslBundle sslBundle) { - HttpClient httpClient = HttpClient.create(); - httpClient = applyDefaults(httpClient); - if (sslBundle != null) { - httpClient = httpClient.secure((ThrowingConsumer.of((spec) -> configureSsl(spec, sslBundle)))); + private ReactorClientHttpRequestFactory createRequestFactory(ClientHttpRequestFactorySettings settings) { + HttpClient httpClient = applyDefaults(HttpClient.create()); + httpClient = httpClient.followRedirect(followRedirects(settings.redirects())); + if (settings.sslBundle() != null) { + httpClient = httpClient.secure((ThrowingConsumer.of((spec) -> configureSsl(spec, settings.sslBundle())))); } return new ReactorClientHttpRequestFactory(httpClient); } + private boolean followRedirects(Redirects redirects) { + return switch (redirects) { + case FOLLOW_WHEN_POSSIBLE -> true; + case FOLLOW -> true; + case DONT_FOLLOW -> false; + }; + } + HttpClient applyDefaults(HttpClient httpClient) { // Aligns with ReactorClientHttpRequestFactory defaults return httpClient.compress(true); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilder.java index 3693e23f444..ef9697a66b4 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilder.java @@ -23,6 +23,7 @@ import java.time.Duration; import java.util.function.Supplier; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.http.client.AbstractClientHttpRequestFactoryWrapper; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.util.Assert; @@ -73,7 +74,9 @@ final class ReflectiveComponentsClientHttpRequestFactoryBuilder setConnectTimeout(unwrapped, connectTimeout)); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilder.java index 6d92620be83..d6e4feee9b7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilder.java @@ -27,6 +27,7 @@ import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.util.Assert; @@ -64,8 +65,7 @@ public final class SimpleClientHttpRequestFactoryBuilder @Override protected SimpleClientHttpRequestFactory createClientHttpRequestFactory(ClientHttpRequestFactorySettings settings) { SslBundle sslBundle = settings.sslBundle(); - SimpleClientHttpRequestFactory requestFactory = (sslBundle != null) - ? new SimpleClientHttpsRequestFactory(sslBundle) : new SimpleClientHttpRequestFactory(); + SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpsRequestFactory(settings); Assert.state(sslBundle == null || !sslBundle.getOptions().isSpecified(), "SSL Options cannot be specified with Java connections"); PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); @@ -75,23 +75,27 @@ public final class SimpleClientHttpRequestFactoryBuilder } /** - * {@link SimpleClientHttpsRequestFactory} to configure SSL from an {@link SslBundle}. + * {@link SimpleClientHttpsRequestFactory} to configure SSL from an {@link SslBundle} + * and {@link Redirects}. */ private static class SimpleClientHttpsRequestFactory extends SimpleClientHttpRequestFactory { - private final SslBundle sslBundle; + private final ClientHttpRequestFactorySettings settings; - SimpleClientHttpsRequestFactory(SslBundle sslBundle) { - this.sslBundle = sslBundle; + SimpleClientHttpsRequestFactory(ClientHttpRequestFactorySettings settings) { + this.settings = settings; } @Override protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException { super.prepareConnection(connection, httpMethod); - if (this.sslBundle != null && connection instanceof HttpsURLConnection secureConnection) { - SSLSocketFactory socketFactory = this.sslBundle.createSslContext().getSocketFactory(); + if (this.settings.sslBundle() != null && connection instanceof HttpsURLConnection secureConnection) { + SSLSocketFactory socketFactory = this.settings.sslBundle().createSslContext().getSocketFactory(); secureConnection.setSSLSocketFactory(socketFactory); } + if (this.settings.redirects() == Redirects.DONT_FOLLOW) { + connection.setInstanceFollowRedirects(false); + } } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactorySettings.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactorySettings.java index 163651cf3ab..74098734777 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactorySettings.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactorySettings.java @@ -79,7 +79,7 @@ public record ClientHttpRequestFactorySettings(Duration connectTimeout, Duration } org.springframework.boot.http.client.ClientHttpRequestFactorySettings adapt() { - return new org.springframework.boot.http.client.ClientHttpRequestFactorySettings(connectTimeout(), + return new org.springframework.boot.http.client.ClientHttpRequestFactorySettings(null, connectTimeout(), readTimeout(), sslBundle()); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java index bc44711c9a3..f2f5cff13fd 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.http.client; import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -31,6 +32,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslBundleKey; import org.springframework.boot.ssl.jks.JksSslStoreBundle; @@ -41,8 +43,10 @@ import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.Ssl.ClientAuth; import org.springframework.boot.web.server.WebServer; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.ClientHttpResponse; import org.springframework.util.StreamUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -116,6 +120,45 @@ abstract class AbstractClientHttpRequestFactoryBuilderTests context.addServlet("test", TestServlet.class).addMapping("/")); + try { + webServer.start(); + int port = webServer.getPort(); + URI uri = new URI("http://localhost:%s".formatted(port) + "/redirect"); + ClientHttpRequestFactory requestFactory = this.builder.build(settings); + ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.GET); + ClientHttpResponse response = request.execute(); + assertThat(response.getStatusCode()).isEqualTo(expectedStatus); + if (expectedStatus == HttpStatus.OK) { + assertThat(response.getBody()).asString(StandardCharsets.UTF_8) + .contains("Received GET request to /redirected"); + } + } + finally { + webServer.stop(); + } + } + private ClientHttpRequest request(ClientHttpRequestFactory factory, URI uri, String method) throws IOException { return factory.createRequest(uri, HttpMethod.valueOf(method)); } @@ -143,6 +186,10 @@ abstract class AbstractClientHttpRequestFactoryBuilderTests ofTestRequestFactory().build(settings)) + .withMessage("Unable to set redirect follow using reflection"); + } + + @Override + void redirectDontFollow() throws Exception { + ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.defaults() + .withRedirects(Redirects.DONT_FOLLOW); + assertThatIllegalStateException().isThrownBy(() -> ofTestRequestFactory().build(settings)) + .withMessage("Unable to set redirect follow using reflection"); + } + @Test void buildWithClassCreatesFactory() { assertThat(ofTestRequestFactory().build()).isInstanceOf(TestClientHttpRequestFactory.class); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationPortTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationPortTests.java index 2b3cbdfa609..80730806649 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationPortTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationPortTests.java @@ -36,8 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Dave Syer */ -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, - properties = { "management.server.port:0", "spring.http.client.factory=simple" }) +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = { "management.server.port:0" }) class SampleActuatorUiApplicationPortTests { @LocalServerPort diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationTests.java index 93a078a6335..6613f35a0ef 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-ui/src/test/java/smoketest/actuator/ui/SampleActuatorUiApplicationTests.java @@ -39,8 +39,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Dave Syer */ -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, - properties = { "server.error.include-message=always", "spring.http.client.factory=simple" }) +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = { "server.error.include-message=always" }) class SampleActuatorUiApplicationTests { @Autowired @@ -51,7 +50,7 @@ class SampleActuatorUiApplicationTests { HttpHeaders headers = new HttpHeaders(); headers.setAccept(Arrays.asList(MediaType.TEXT_HTML)); ResponseEntity entity = this.restTemplate.withBasicAuth("user", getPassword()) - .exchange("/", HttpMethod.GET, new HttpEntity<>(headers), String.class); + .exchange("/", HttpMethod.GET, new HttpEntity(headers), String.class); assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(entity.getBody()).contains("Hello"); } @@ -75,7 +74,7 @@ class SampleActuatorUiApplicationTests { HttpHeaders headers = new HttpHeaders(); headers.setAccept(Arrays.asList(MediaType.TEXT_HTML)); ResponseEntity<String> entity = this.restTemplate.withBasicAuth("user", getPassword()) - .exchange("/error", HttpMethod.GET, new HttpEntity<>(headers), String.class); + .exchange("/error", HttpMethod.GET, new HttpEntity<Void>(headers), String.class); assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); assertThat(entity.getBody()).contains("<html>") .contains("<body>") diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-session-jdbc/src/test/java/smoketest/session/SampleSessionJdbcApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-session-jdbc/src/test/java/smoketest/session/SampleSessionJdbcApplicationTests.java index 17d9fc58f0a..76597c8f146 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-session-jdbc/src/test/java/smoketest/session/SampleSessionJdbcApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-session-jdbc/src/test/java/smoketest/session/SampleSessionJdbcApplicationTests.java @@ -25,8 +25,12 @@ import java.util.Map; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; @@ -37,6 +41,7 @@ import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.RestTemplate; import static org.assertj.core.api.Assertions.assertThat; @@ -48,12 +53,22 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Madhura Bhave */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, - properties = { "server.servlet.session.timeout:2", "spring.http.client.factory=simple" }) + properties = { "server.servlet.session.timeout:2" }) class SampleSessionJdbcApplicationTests { + private static final ClientHttpRequestFactorySettings DONT_FOLLOW_REDIRECTS = ClientHttpRequestFactorySettings + .defaults() + .withRedirects(Redirects.DONT_FOLLOW); + + @Autowired + private RestTemplateBuilder restTemplateBuilder; + @Autowired private TestRestTemplate restTemplate; + @LocalServerPort + private String port; + private static final URI ROOT_URI = URI.create("/"); @Test @@ -68,14 +83,15 @@ class SampleSessionJdbcApplicationTests { } private String performLogin() { + RestTemplate restTemplate = this.restTemplateBuilder.requestFactorySettings(DONT_FOLLOW_REDIRECTS).build(); HttpHeaders headers = new HttpHeaders(); headers.setAccept(Collections.singletonList(MediaType.TEXT_HTML)); headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); MultiValueMap<String, String> form = new LinkedMultiValueMap<>(); form.set("username", "user"); form.set("password", "password"); - ResponseEntity<String> entity = this.restTemplate.exchange("/login", HttpMethod.POST, - new HttpEntity<>(form, headers), String.class); + ResponseEntity<String> entity = restTemplate.exchange("http://localhost:" + this.port + "/login", + HttpMethod.POST, new HttpEntity<>(form, headers), String.class); return entity.getHeaders().getFirst("Set-Cookie"); } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-groovy-templates/src/test/java/smoketest/groovytemplates/SampleGroovyTemplateApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-groovy-templates/src/test/java/smoketest/groovytemplates/SampleGroovyTemplateApplicationTests.java index a52c86691ae..c39aab49c3c 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-groovy-templates/src/test/java/smoketest/groovytemplates/SampleGroovyTemplateApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-groovy-templates/src/test/java/smoketest/groovytemplates/SampleGroovyTemplateApplicationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2024 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. @@ -37,7 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Dave Syer */ -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = "spring.http.client.redirects=dont-follow") class SampleGroovyTemplateApplicationTests { @LocalServerPort diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-thymeleaf/src/test/java/smoketest/web/thymeleaf/SampleWebUiApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-thymeleaf/src/test/java/smoketest/web/thymeleaf/SampleWebUiApplicationTests.java index d658586cb12..4da6f8fd433 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-thymeleaf/src/test/java/smoketest/web/thymeleaf/SampleWebUiApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-thymeleaf/src/test/java/smoketest/web/thymeleaf/SampleWebUiApplicationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2024 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. @@ -37,7 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Dave Syer */ -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = "spring.http.client.redirects=dont-follow") class SampleWebUiApplicationTests { @Autowired