From f450b28a7c063a1d006d6fc6569b58c896317cdc Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Fri, 22 Nov 2024 17:12:46 +0800 Subject: [PATCH 1/2] Add `redirects(...)` method to `RestTemplateBuilder` Add `redirects(...)` method to `RestTemplateBuilder` to allow redirect customization. This new method is required in 3.4 since the default redirect strategy for some clients has changed and users need a way to restore the old behavior. See gh-43258 --- .../test/web/client/TestRestTemplate.java | 24 ++++++++++++++++++- .../web/client/TestRestTemplateTests.java | 18 ++++++++++++++ .../boot/web/client/RestTemplateBuilder.java | 15 ++++++++++++ .../web/client/RestTemplateBuilderTests.java | 9 +++++++ 4 files changed, 65 insertions(+), 1 deletion(-) 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 a8d7cf2e9390..af119e1266af 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 @@ -47,6 +47,7 @@ import org.apache.hc.core5.ssl.TrustStrategy; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.boot.web.client.RootUriTemplateHandler; import org.springframework.core.ParameterizedTypeReference; @@ -92,6 +93,7 @@ * @author Andy Wilkinson * @author Kristine Jetzke * @author Dmytro Nosan + * @author Yanming Zhou * @since 1.4.0 */ public class TestRestTemplate { @@ -946,6 +948,22 @@ public TestRestTemplate withBasicAuth(String username, String password) { return template; } + /** + * Creates a new {@code TestRestTemplate} with the same configuration as this one, + * except that it will use the given {@code redirects} strategies. The request factory + * used is a new instance of the underlying {@link RestTemplate}'s request factory + * type (when possible). + * @param redirects the redirects + * @return the new template + * @since 3.4.1 + */ + public TestRestTemplate withRedirects(Redirects redirects) { + TestRestTemplate template = new TestRestTemplate(this.builder.redirects(redirects), null, null, + this.httpClientOptions); + template.setUriTemplateHandler(getRestTemplate().getUriTemplateHandler()); + return template; + } + @SuppressWarnings({ "rawtypes", "unchecked" }) private RequestEntity createRequestEntityWithRootAppliedUri(RequestEntity requestEntity) { return new RequestEntity(requestEntity.getBody(), requestEntity.getHeaders(), requestEntity.getMethod(), @@ -988,7 +1006,10 @@ public enum HttpClientOption { /** * Enable redirects. + * @deprecated since 3.4.1 for removal in 3.6.0 in favor of + * {@link #withRedirects(Redirects)} */ + @Deprecated(since = "3.4.1", forRemoval = true) ENABLE_REDIRECTS, /** @@ -1032,7 +1053,8 @@ public CustomHttpComponentsClientHttpRequestFactory(HttpClientOption[] httpClien Set options = new HashSet<>(Arrays.asList(httpClientOptions)); this.cookieSpec = (options.contains(HttpClientOption.ENABLE_COOKIES) ? StandardCookieSpec.STRICT : StandardCookieSpec.IGNORE); - this.enableRedirects = options.contains(HttpClientOption.ENABLE_REDIRECTS); + this.enableRedirects = options.contains(HttpClientOption.ENABLE_REDIRECTS) + || settings.redirects() != Redirects.DONT_FOLLOW; boolean ssl = options.contains(HttpClientOption.SSL); if (settings.readTimeout() != null || ssl) { setHttpClient(createHttpClient(settings.readTimeout(), ssl)); 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 8d5ba9c055e2..3363b3dbe7af 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 @@ -26,6 +26,7 @@ import org.apache.hc.client5.http.config.RequestConfig; import org.junit.jupiter.api.Test; +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; @@ -66,6 +67,7 @@ * @author Stephane Nicoll * @author Andy Wilkinson * @author Kristine Jetzke + * @author Yanming Zhou */ class TestRestTemplateTests { @@ -129,6 +131,7 @@ void authenticated() { assertBasicAuthorizationCredentials(restTemplate, "user", "password"); } + @SuppressWarnings("deprecation") @Test void options() { TestRestTemplate template = new TestRestTemplate(HttpClientOption.ENABLE_REDIRECTS); @@ -139,6 +142,21 @@ void options() { assertThat(config.isRedirectsEnabled()).isTrue(); } + @Test + void redirects() { + TestRestTemplate template = new TestRestTemplate().withRedirects(Redirects.DONT_FOLLOW); + CustomHttpComponentsClientHttpRequestFactory factory = (CustomHttpComponentsClientHttpRequestFactory) template + .getRestTemplate() + .getRequestFactory(); + RequestConfig config = factory.createRequestConfig(); + assertThat(config.isRedirectsEnabled()).isFalse(); + + template = new TestRestTemplate().withRedirects(Redirects.FOLLOW); + factory = (CustomHttpComponentsClientHttpRequestFactory) template.getRestTemplate().getRequestFactory(); + config = factory.createRequestConfig(); + assertThat(config.isRedirectsEnabled()).isTrue(); + } + @Test void restOperationsAreAvailable() { RestTemplate delegate = mock(RestTemplate.class); 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 2f5e35b5abd6..d8f574b506ef 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 @@ -33,6 +33,7 @@ import org.springframework.beans.BeanUtils; 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.ssl.SslBundle; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; @@ -64,6 +65,7 @@ * @author Kevin Strijbos * @author Ilya Lukyanovich * @author Scott Frederick + * @author Yanming Zhou * @since 1.4.0 */ public class RestTemplateBuilder { @@ -501,6 +503,19 @@ public RestTemplateBuilder readTimeout(Duration readTimeout) { this.defaultHeaders, this.customizers, this.requestCustomizers); } + /** + * Sets the redirect strategy on the underlying {@link ClientHttpRequestFactory}. + * @param redirects the redirect strategy + * @return a new builder instance. + * @since 3.4.1 + */ + public RestTemplateBuilder redirects(Redirects redirects) { + return new RestTemplateBuilder(this.requestFactorySettings.withRedirects(redirects), this.detectRequestFactory, + this.rootUri, this.messageConverters, this.interceptors, this.requestFactoryBuilder, + this.uriTemplateHandler, this.errorHandler, this.basicAuthentication, this.defaultHeaders, + this.customizers, this.requestCustomizers); + } + /** * Sets the SSL bundle on the underlying {@link ClientHttpRequestFactory}. * @param sslBundle the SSL bundle diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java index 0b2e39a75cab..c62b22b58b7d 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java @@ -32,6 +32,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.boot.http.client.ClientHttpRequestFactorySettings; +import org.springframework.boot.http.client.ClientHttpRequestFactorySettings.Redirects; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; @@ -73,6 +74,7 @@ * @author Kevin Strijbos * @author Ilya Lukyanovich * @author Brian Clozel + * @author Yanming Zhou */ @ExtendWith(MockitoExtension.class) class RestTemplateBuilderTests { @@ -486,6 +488,13 @@ void unwrappingDoesNotAffectRequestFactoryThatIsSetOnTheBuiltTemplate() { assertThat(template.getRequestFactory()).isInstanceOf(BufferingClientHttpRequestFactory.class); } + @Test + void configureRedirects() { + assertThat(this.builder.redirects(Redirects.DONT_FOLLOW)).extracting("requestFactorySettings") + .extracting("redirects") + .isSameAs(Redirects.DONT_FOLLOW); + } + private ClientHttpRequest createRequest(RestTemplate template) { return ReflectionTestUtils.invokeMethod(template, "createRequest", URI.create("http://localhost"), HttpMethod.GET); From 8b83afdb681a9e889857dd06c4df557a27e1c042 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 4 Dec 2024 13:33:12 -0800 Subject: [PATCH 2/2] Polish 'Add `redirects(...)` method to `RestTemplateBuilder`' Remove deprecations and new methods in `TestRestTemplate` in favor of passing in a configured `RestTemplateBuilder`. See gh-43258 --- .../test/web/client/TestRestTemplate.java | 19 ------- .../web/client/TestRestTemplateTests.java | 57 ++++++++++++++----- 2 files changed, 43 insertions(+), 33 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 af119e1266af..f0629aa3dbb2 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 @@ -948,22 +948,6 @@ public TestRestTemplate withBasicAuth(String username, String password) { return template; } - /** - * Creates a new {@code TestRestTemplate} with the same configuration as this one, - * except that it will use the given {@code redirects} strategies. The request factory - * used is a new instance of the underlying {@link RestTemplate}'s request factory - * type (when possible). - * @param redirects the redirects - * @return the new template - * @since 3.4.1 - */ - public TestRestTemplate withRedirects(Redirects redirects) { - TestRestTemplate template = new TestRestTemplate(this.builder.redirects(redirects), null, null, - this.httpClientOptions); - template.setUriTemplateHandler(getRestTemplate().getUriTemplateHandler()); - return template; - } - @SuppressWarnings({ "rawtypes", "unchecked" }) private RequestEntity createRequestEntityWithRootAppliedUri(RequestEntity requestEntity) { return new RequestEntity(requestEntity.getBody(), requestEntity.getHeaders(), requestEntity.getMethod(), @@ -1006,10 +990,7 @@ public enum HttpClientOption { /** * Enable redirects. - * @deprecated since 3.4.1 for removal in 3.6.0 in favor of - * {@link #withRedirects(Redirects)} */ - @Deprecated(since = "3.4.1", forRemoval = true) ENABLE_REDIRECTS, /** 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 3363b3dbe7af..1d866608885f 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 @@ -20,12 +20,15 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpClient.Redirect; import java.util.Base64; import java.util.stream.Stream; import org.apache.hc.client5.http.config.RequestConfig; import org.junit.jupiter.api.Test; +import org.springframework.boot.http.client.ClientHttpRequestFactoryBuilder; 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; @@ -39,6 +42,7 @@ import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; +import org.springframework.http.client.JdkClientHttpRequestFactory; import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.mock.env.MockEnvironment; import org.springframework.mock.http.client.MockClientHttpRequest; @@ -131,30 +135,55 @@ void authenticated() { assertBasicAuthorizationCredentials(restTemplate, "user", "password"); } - @SuppressWarnings("deprecation") @Test void options() { - TestRestTemplate template = new TestRestTemplate(HttpClientOption.ENABLE_REDIRECTS); - CustomHttpComponentsClientHttpRequestFactory factory = (CustomHttpComponentsClientHttpRequestFactory) template - .getRestTemplate() - .getRequestFactory(); - RequestConfig config = factory.createRequestConfig(); + RequestConfig config = getRequestConfig( + new TestRestTemplate(HttpClientOption.ENABLE_REDIRECTS, HttpClientOption.ENABLE_COOKIES)); assertThat(config.isRedirectsEnabled()).isTrue(); + assertThat(config.getCookieSpec()).isEqualTo("strict"); } @Test - void redirects() { - TestRestTemplate template = new TestRestTemplate().withRedirects(Redirects.DONT_FOLLOW); + void jdkBuilderCanBeSpecifiedWithSpecificRedirects() { + RestTemplateBuilder builder = new RestTemplateBuilder() + .requestFactoryBuilder(ClientHttpRequestFactoryBuilder.jdk()); + TestRestTemplate templateWithRedirects = new TestRestTemplate(builder.redirects(Redirects.FOLLOW)); + assertThat(getJdkHttpClient(templateWithRedirects).followRedirects()).isEqualTo(Redirect.NORMAL); + TestRestTemplate templateWithoutRedirects = new TestRestTemplate(builder.redirects(Redirects.DONT_FOLLOW)); + assertThat(getJdkHttpClient(templateWithoutRedirects).followRedirects()).isEqualTo(Redirect.NEVER); + } + + @Test + void httpComponentsAreBuildConsideringSettingsInRestTemplateBuilder() { + RestTemplateBuilder builder = new RestTemplateBuilder() + .requestFactoryBuilder(ClientHttpRequestFactoryBuilder.httpComponents()); + assertThat(getRequestConfig((RestTemplateBuilder) null).isRedirectsEnabled()).isTrue(); + assertThat(getRequestConfig(null, HttpClientOption.ENABLE_REDIRECTS).isRedirectsEnabled()).isTrue(); + assertThat(getRequestConfig(builder).isRedirectsEnabled()).isTrue(); + 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(); + } + + private RequestConfig getRequestConfig(RestTemplateBuilder builder, HttpClientOption... httpClientOptions) { + builder = (builder != null) ? builder : new RestTemplateBuilder(); + TestRestTemplate template = new TestRestTemplate(builder, null, null, httpClientOptions); + return getRequestConfig(template); + } + + private RequestConfig getRequestConfig(TestRestTemplate template) { CustomHttpComponentsClientHttpRequestFactory factory = (CustomHttpComponentsClientHttpRequestFactory) template .getRestTemplate() .getRequestFactory(); - RequestConfig config = factory.createRequestConfig(); - assertThat(config.isRedirectsEnabled()).isFalse(); + return factory.createRequestConfig(); + } - template = new TestRestTemplate().withRedirects(Redirects.FOLLOW); - factory = (CustomHttpComponentsClientHttpRequestFactory) template.getRestTemplate().getRequestFactory(); - config = factory.createRequestConfig(); - assertThat(config.isRedirectsEnabled()).isTrue(); + private HttpClient getJdkHttpClient(TestRestTemplate templateWithRedirects) { + JdkClientHttpRequestFactory requestFactory = (JdkClientHttpRequestFactory) templateWithRedirects + .getRestTemplate() + .getRequestFactory(); + return (HttpClient) ReflectionTestUtils.getField(requestFactory, "httpClient"); } @Test