Merge branch '4.1.x' into pr/3598

Author: ryanjbaxter

.github/workflows/maven.yml

@@ -5,9 +5,9 @@ name: Build
 
 on:
   push:
-    branches: [ main, 3.1.x ]
+    branches: [ main, 4.1.x, 3.1.x ]
   pull_request:
-    branches: [ main, 3.1.x ]
+    branches: [ main, 4.1.x, 3.1.x ]
 
 jobs:
   build:

README.adoc

@@ -224,7 +224,7 @@ Spring Cloud Build brings along the  `basepom:duplicate-finder-maven-plugin`, th
 [[duplicate-finder-configuration]]
 === Duplicate Finder configuration
 
-Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the projecst's `pom.xml`.
+Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the project's `pom.xml`.
 
 .pom.xml
 [source,xml]

docs/modules/ROOT/pages/spring-cloud-gateway-server-mvc/filters/retry.adoc

@@ -9,6 +9,7 @@ The `Retry` filter supports the following parameters:
 * `methods`: The HTTP methods that should be retried, represented by using `org.springframework.http.HttpMethod`.
 * `series`: The series of status codes to be retried, represented by using `org.springframework.http.HttpStatus.Series`.
 * `exceptions`: A list of thrown exceptions that should be retried.
+* `cacheBody`: A flag to signal if the request body should be cached. If set to `true`, the `adaptCacheBody` filter must be used to send the cached body downstream.
 //* `backoff`: The configured exponential backoff for the retries.
 //Retries are performed after a backoff interval of `firstBackoff * (factor ^ n)`, where `n` is the iteration.
 //If `maxBackoff` is configured, the maximum backoff applied is limited to `maxBackoff`.
@@ -20,8 +21,11 @@ The following defaults are configured for `Retry` filter, if enabled:
 * `series`: 5XX series
 * `methods`: GET method
 * `exceptions`: `IOException`, `TimeoutException` and `RetryException`
+* `cacheBody`: `false`
 //* `backoff`: disabled
 
+WARNING: Setting `cacheBody` to `true` causes the gateway to read the whole body into memory. This should be used with caution.
+
 The following listing configures a Retry  filter:
 
 .application.yml
@@ -42,11 +46,14 @@ spring:
               retries: 3
               series: SERVER_ERROR
               methods: GET,POST
+              cacheBody: true
+          - name: AdaptCachedBody
 ----
 
 .GatewaySampleApplication.java
 [source,java]
 ----
+import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.adaptCachedBody;
 import static org.springframework.cloud.gateway.server.mvc.filter.RetryFilterFunctions.retry;
 import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route;
 import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.http;
@@ -59,7 +66,8 @@ class RouteConfiguration {
     public RouterFunction<ServerResponse> gatewayRouterFunctionsAddReqHeader() {
 		return route("add_request_parameter_route")
 			.route(host("*.retry.com"), http("https://example.org"))
-				.filter(retry(config -> config.setRetries(3).setSeries(Set.of(HttpStatus.Series.SERVER_ERROR)).setMethods(Set.of(HttpMethod.GET, HttpMethod.POST))))
+				.filter(retry(config -> config.setRetries(3).setSeries(Set.of(HttpStatus.Series.SERVER_ERROR)).setMethods(Set.of(HttpMethod.GET, HttpMethod.POST)).setCacheBody(true)))
+				.filter(adaptCachedBody())
 			.build();
     }
 }

docs/package.json

@@ -4,7 +4,7 @@
     "@antora/atlas-extension": "1.0.0-alpha.2",
     "@antora/collector-extension": "1.0.1",
     "@asciidoctor/tabs": "1.0.0-beta.6",
-    "@springio/antora-extensions": "1.14.2",
-    "@springio/asciidoctor-extensions": "1.0.0-alpha.14"
+    "@springio/antora-extensions": "1.14.4",
+    "@springio/asciidoctor-extensions": "1.0.0-alpha.16"
   }
 }

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/MultipartEnvironmentPostProcessor.java

@@ -37,7 +37,7 @@ public void postProcessEnvironment(ConfigurableEnvironment environment, SpringAp
 			// no user set property, set it to false.
 			MapPropertySource propertySource = new MapPropertySource(MULTIPART_PROPERTY_SOURCE_NAME,
 					Map.of(MULTIPART_ENABLED_PROPERTY, Boolean.FALSE));
-			// environment.getPropertySources().addFirst(propertySource);
+			environment.getPropertySources().addFirst(propertySource);
 		}
 	}
 

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/MvcUtils.java

@@ -116,6 +116,14 @@ public static ByteArrayInputStream cacheBody(ServerRequest request) {
 		}
 	}
 
+	public static ByteArrayInputStream getOrCacheBody(ServerRequest request) {
+		ByteArrayInputStream body = getAttribute(request, MvcUtils.CACHED_REQUEST_BODY_ATTR);
+		if (body != null) {
+			return body;
+		}
+		return cacheBody(request);
+	}
+
 	public static String expand(ServerRequest request, String template) {
 		Assert.notNull(request, "request may not be null");
 		Assert.notNull(template, "template may not be null");

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RetryFilterFunctions.java

@@ -27,6 +27,7 @@
 import java.util.function.Consumer;
 
 import org.springframework.cloud.gateway.server.mvc.common.Configurable;
+import org.springframework.cloud.gateway.server.mvc.common.MvcUtils;
 import org.springframework.cloud.gateway.server.mvc.common.Shortcut;
 import org.springframework.core.NestedRuntimeException;
 import org.springframework.http.HttpMethod;
@@ -71,6 +72,9 @@ public static HandlerFilterFunction<ServerResponse, ServerResponse> retry(RetryC
 			.setPolicies(Arrays.asList(simpleRetryPolicy, new HttpRetryPolicy(config)).toArray(new RetryPolicy[0]));
 		RetryTemplate retryTemplate = retryTemplateBuilder.customPolicy(compositeRetryPolicy).build();
 		return (request, next) -> retryTemplate.execute(context -> {
+			if (config.isCacheBody()) {
+				MvcUtils.getOrCacheBody(request);
+			}
 			ServerResponse serverResponse = next.handle(request);
 
 			if (isRetryableStatusCode(serverResponse.statusCode(), config)
@@ -121,6 +125,8 @@ public static class RetryConfig {
 
 		private Set<HttpMethod> methods = new HashSet<>(List.of(HttpMethod.GET));
 
+		private boolean cacheBody = false;
+
 		// TODO: individual statuses
 		// TODO: backoff
 		// TODO: support more Spring Retry policies
@@ -176,6 +182,15 @@ public RetryConfig addMethods(HttpMethod... methods) {
 			return this;
 		}
 
+		public boolean isCacheBody() {
+			return cacheBody;
+		}
+
+		public RetryConfig setCacheBody(boolean cacheBody) {
+			this.cacheBody = cacheBody;
+			return this;
+		}
+
 	}
 
 	private static class RetryException extends NestedRuntimeException {

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ProxyExchangeHandlerFunction.java

@@ -123,9 +123,9 @@ public ServerResponse handle(ServerRequest serverRequest) {
 	private <REQUEST_OR_RESPONSE> HttpHeaders filterHeaders(List<?> filters, HttpHeaders original,
 			REQUEST_OR_RESPONSE requestOrResponse) {
 		HttpHeaders filtered = original;
-		for (var filter : filters) {
+		for (Object filter : filters) {
 			@SuppressWarnings("unchecked")
-			var typed = ((HttpHeadersFilter<REQUEST_OR_RESPONSE>) filter);
+			HttpHeadersFilter<REQUEST_OR_RESPONSE> typed = ((HttpHeadersFilter<REQUEST_OR_RESPONSE>) filter);
 			filtered = typed.apply(filtered, requestOrResponse);
 		}
 		return filtered;

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/predicate/GatewayRequestPredicates.java

@@ -374,6 +374,9 @@ private static class HostPatternPredicate implements RequestPredicate, ChangePat
 		@Override
 		public boolean test(ServerRequest request) {
 			String host = request.headers().firstHeader(HttpHeaders.HOST);
+			if (host == null) {
+				host = "";
+			}
 			PathContainer pathContainer = PathContainer.parsePath(host, PathContainer.Options.MESSAGE_ROUTE);
 			PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(pathContainer);
 			traceMatch("Pattern", this.pattern.getPatternString(), host, info != null);

spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/ServerMvcIntegrationTests.java

@@ -22,12 +22,11 @@
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 
 import com.github.benmanes.caffeine.cache.Caffeine;
@@ -40,8 +39,6 @@
 import jakarta.servlet.ServletRequest;
 import jakarta.servlet.ServletResponse;
 import jakarta.servlet.http.HttpServletRequest;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -79,10 +76,8 @@
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StreamUtils;
-import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
-import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.servlet.function.HandlerFunction;
 import org.springframework.web.servlet.function.RouterFunction;
@@ -117,7 +112,6 @@
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.addRequestHeader;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.addRequestHeadersIfNotPresent;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.addRequestParameter;
-import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.prefixPath;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.redirectTo;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.removeRequestHeader;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.rewritePath;
@@ -126,7 +120,6 @@
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.setRequestHostHeader;
 import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.stripPrefix;
 import static org.springframework.cloud.gateway.server.mvc.filter.LoadBalancerFilterFunctions.lb;
-import static org.springframework.cloud.gateway.server.mvc.filter.RetryFilterFunctions.retry;
 import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route;
 import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.forward;
 import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.http;
@@ -393,20 +386,6 @@ public void circuitBreakerInvalidFallbackThrowsException() {
 		// @formatter:on
 	}
 
-	@Test
-	public void retryWorks() {
-		restClient.get().uri("/retry?key=get").exchange().expectStatus().isOk().expectBody(String.class).isEqualTo("3");
-		// test for: java.lang.IllegalArgumentException: You have already selected another
-		// retry policy
-		restClient.get()
-			.uri("/retry?key=get2")
-			.exchange()
-			.expectStatus()
-			.isOk()
-			.expectBody(String.class)
-			.isEqualTo("3");
-	}
-
 	@Test
 	public void rateLimitWorks() {
 		restClient.get().uri("/anything/ratelimit").exchange().expectStatus().isOk();
@@ -484,7 +463,7 @@ public void rewritePathPostWorks() {
 	@Test
 	public void rewritePathPostLocalWorks() {
 		restClient.post()
-			.uri("/baz/post")
+			.uri("/baz/localpost")
 			.bodyValue("hello")
 			.header("Host", "www.rewritepathpostlocal.org")
 			.exchange()
@@ -636,8 +615,21 @@ private MultiValueMap<String, HttpEntity<?>> createMultipartData() {
 	private void assertMultipartData(Map responseBody) {
 		Map<String, Object> files = (Map<String, Object>) responseBody.get("files");
 		assertThat(files).containsKey("imgpart");
-		String file = (String) files.get("imgpart");
-		assertThat(file).startsWith("data:").contains(";base64,");
+		Object imgpart = files.get("imgpart");
+		if (imgpart instanceof List l) {
+			String file = (String) l.get(0);
+			assertThat(isPNG(file.getBytes()));
+		}
+		else {
+			String file = (String) imgpart;
+			assertThat(file).startsWith("data:").contains(";base64,");
+		}
+	}
+
+	private static boolean isPNG(byte[] bytes) {
+		byte[] pngSignature = { (byte) 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A };
+		byte[] header = Arrays.copyOf(bytes, pngSignature.length);
+		return Arrays.equals(pngSignature, header);
 	}
 
 	@Test
@@ -991,11 +983,6 @@ TestHandler testHandler() {
 			return new TestHandler();
 		}
 
-		@Bean
-		RetryController retryController() {
-			return new RetryController();
-		}
-
 		@Bean
 		EventController eventController() {
 			return new EventController();
@@ -1180,19 +1167,6 @@ public RouterFunction<ServerResponse> gatewayRouterFunctionsCircuitBreakerNoFall
 			// @formatter:on
 		}
 
-		@Bean
-		public RouterFunction<ServerResponse> gatewayRouterFunctionsRetry() {
-			// @formatter:off
-			return route("testretry")
-					.route(path("/retry"), http())
-					.before(new LocalServerPortUriResolver())
-					.filter(retry(3))
-					//.filter(retry(config -> config.setRetries(3).setSeries(Set.of(HttpStatus.Series.SERVER_ERROR)).setMethods(Set.of(HttpMethod.GET, HttpMethod.POST))))
-					.filter(prefixPath("/do"))
-					.build();
-			// @formatter:on
-		}
-
 		@Bean
 		public RouterFunction<ServerResponse> gatewayRouterFunctionsRateLimit() {
 			// @formatter:off
@@ -1279,8 +1253,7 @@ public RouterFunction<ServerResponse> gatewayRouterFunctionsForm() {
 			// @formatter:off
 			return route("testform")
 					.POST("/post", host("**.testform.org"), http())
-					.before(new LocalServerPortUriResolver())
-					.filter(prefixPath("/test"))
+					.filter(new HttpbinUriResolver())
 					.filter(addRequestHeader("X-Test", "form"))
 					.build();
 			// @formatter:on
@@ -1677,37 +1650,6 @@ public ResponseEntity<Event> messageChannelEvents(@RequestBody Event e) {
 
 	}
 
-	@RestController
-	protected static class RetryController {
-
-		Log log = LogFactory.getLog(getClass());
-
-		ConcurrentHashMap<String, AtomicInteger> map = new ConcurrentHashMap<>();
-
-		@GetMapping("/do/retry")
-		public ResponseEntity<String> retry(@RequestParam("key") String key,
-				@RequestParam(name = "count", defaultValue = "3") int count,
-				@RequestParam(name = "failStatus", required = false) Integer failStatus) {
-			AtomicInteger num = getCount(key);
-			int i = num.incrementAndGet();
-			log.warn("Retry count: " + i);
-			String body = String.valueOf(i);
-			if (i < count) {
-				HttpStatus httpStatus = HttpStatus.INTERNAL_SERVER_ERROR;
-				if (failStatus != null) {
-					httpStatus = HttpStatus.resolve(failStatus);
-				}
-				return ResponseEntity.status(httpStatus).header("X-Retry-Count", body).body("temporarily broken");
-			}
-			return ResponseEntity.status(HttpStatus.OK).header("X-Retry-Count", body).body(body);
-		}
-
-		AtomicInteger getCount(String key) {
-			return map.computeIfAbsent(key, s -> new AtomicInteger());
-		}
-
-	}
-
 	protected static class TestHandler implements HandlerFunction<ServerResponse> {
 
 		@Override