Add additional tests for redirects with different HTTP methods

Closes gh-42879
spring-projects · Oct 25, 2024 · 97b20e9 · 97b20e9
1 parent 85b1c55
commit 97b20e9
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 17 deletions.
diff --git a/...in/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java b/...in/java/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilder.java
@@ -41,6 +41,10 @@ protected AbstractClientHttpRequestFactoryBuilder(List<Consumer<T>> customizers)
 		this.customizers = (customizers != null) ? customizers : Collections.emptyList();
 	}
 
+	protected final List<Consumer<T>> getCustomizers() {
+		return this.customizers;
+	}
+
 	protected final List<Consumer<T>> mergedCustomizers(Consumer<T> customizer) {
 		Assert.notNull(this.customizers, "'customizer' must not be null");
 		return merge(this.customizers, List.of(customizer));

diff --git a/...va/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java b/...va/org/springframework/boot/http/client/AbstractClientHttpRequestFactoryBuilderTests.java
@@ -21,6 +21,7 @@
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
+import java.util.function.Function;
 
 import javax.net.ssl.SSLHandshakeException;
 
@@ -62,6 +63,8 @@
 @DirtiesUrlFactories
 abstract class AbstractClientHttpRequestFactoryBuilderTests<T extends ClientHttpRequestFactory> {
 
+	private static final Function<HttpMethod, HttpStatus> ALWAYS_FOUND = (method) -> HttpStatus.FOUND;
+
 	private final Class<T> requestFactoryType;
 
 	private final ClientHttpRequestFactoryBuilder<T> builder;
@@ -120,24 +123,31 @@ void connectWithSslBundle(String httpMethod) throws Exception {
 		}
 	}
 
-	@Test
-	void redirectDefault() throws Exception {
-		testRedirect(null, HttpStatus.OK);
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "PATCH", "DELETE" })
+	void redirectDefault(String httpMethod) throws Exception {
+		testRedirect(null, HttpMethod.valueOf(httpMethod), this::getExpectedRedirect);
 	}
 
-	@Test
-	void redirectFollow() throws Exception {
-		testRedirect(ClientHttpRequestFactorySettings.defaults().withRedirects(Redirects.FOLLOW), HttpStatus.OK);
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "PATCH", "DELETE" })
+	void redirectFollow(String httpMethod) throws Exception {
+		ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.defaults()
+			.withRedirects(Redirects.FOLLOW);
+		testRedirect(settings, HttpMethod.valueOf(httpMethod), this::getExpectedRedirect);
 	}
 
-	@Test
-	void redirectDontFollow() throws Exception {
-		testRedirect(ClientHttpRequestFactorySettings.defaults().withRedirects(Redirects.DONT_FOLLOW),
-				HttpStatus.FOUND);
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "PATCH", "DELETE" })
+	void redirectDontFollow(String httpMethod) throws Exception {
+		ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.defaults()
+			.withRedirects(Redirects.DONT_FOLLOW);
+		testRedirect(settings, HttpMethod.valueOf(httpMethod), ALWAYS_FOUND);
 	}
 
-	private void testRedirect(ClientHttpRequestFactorySettings settings, HttpStatus expectedStatus)
-			throws URISyntaxException, IOException {
+	protected final void testRedirect(ClientHttpRequestFactorySettings settings, HttpMethod httpMethod,
+			Function<HttpMethod, HttpStatus> expectedStatusForMethod) throws URISyntaxException, IOException {
+		HttpStatus expectedStatus = expectedStatusForMethod.apply(httpMethod);
 		TomcatServletWebServerFactory webServerFactory = new TomcatServletWebServerFactory(0);
 		WebServer webServer = webServerFactory
 			.getWebServer((context) -> context.addServlet("test", TestServlet.class).addMapping("/"));
@@ -146,12 +156,11 @@ private void testRedirect(ClientHttpRequestFactorySettings settings, HttpStatus
 			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);
+			ClientHttpRequest request = requestFactory.createRequest(uri, httpMethod);
 			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");
+				assertThat(response.getBody()).asString(StandardCharsets.UTF_8).contains("request to /redirected");
 			}
 		}
 		finally {
@@ -178,6 +187,10 @@ protected final SslBundle sslBundle() {
 		return SslBundle.of(stores, SslBundleKey.of("password"));
 	}
 
+	protected HttpStatus getExpectedRedirect(HttpMethod httpMethod) {
+		return HttpStatus.OK;
+	}
+
 	protected abstract long connectTimeout(T requestFactory);
 
 	protected abstract long readTimeout(T requestFactory);

diff --git a/...gframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilderTests.java b/...gframework/boot/http/client/ReflectiveComponentsClientHttpRequestFactoryBuilderTests.java
@@ -54,15 +54,15 @@ void connectWithSslBundle(String httpMethod) throws Exception {
 	}
 
 	@Override
-	void redirectFollow() throws Exception {
+	void redirectFollow(String httpMethod) throws Exception {
 		ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.defaults()
 			.withRedirects(Redirects.FOLLOW);
 		assertThatIllegalStateException().isThrownBy(() -> ofTestRequestFactory().build(settings))
 			.withMessage("Unable to set redirect follow using reflection");
 	}
 
 	@Override
-	void redirectDontFollow() throws Exception {
+	void redirectDontFollow(String httpMethod) throws Exception {
 		ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.defaults()
 			.withRedirects(Redirects.DONT_FOLLOW);
 		assertThatIllegalStateException().isThrownBy(() -> ofTestRequestFactory().build(settings))

diff --git a/...java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java b/...java/org/springframework/boot/http/client/SimpleClientHttpRequestFactoryBuilderTests.java
@@ -16,6 +16,11 @@
 
 package org.springframework.boot.http.client;
 
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.client.SimpleClientHttpRequestFactory;
 import org.springframework.test.util.ReflectionTestUtils;
 
@@ -42,4 +47,30 @@ protected long readTimeout(SimpleClientHttpRequestFactory requestFactory) {
 		return (int) ReflectionTestUtils.getField(requestFactory, "readTimeout");
 	}
 
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "DELETE" })
+	@Override
+	void redirectDefault(String httpMethod) throws Exception {
+		super.redirectDefault(httpMethod);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "DELETE" })
+	@Override
+	void redirectFollow(String httpMethod) throws Exception {
+		super.redirectFollow(httpMethod);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = { "GET", "POST", "PUT", "DELETE" })
+	@Override
+	void redirectDontFollow(String httpMethod) throws Exception {
+		super.redirectDontFollow(httpMethod);
+	}
+
+	@Override
+	protected HttpStatus getExpectedRedirect(HttpMethod httpMethod) {
+		return (httpMethod != HttpMethod.GET) ? HttpStatus.FOUND : HttpStatus.OK;
+	}
+
 }