From 06a0327af0f5516bde9ecce17a4f7302dc09816e Mon Sep 17 00:00:00 2001 From: JoeCqupt Date: Mon, 19 Oct 2026 00:55:47 +0800 Subject: [PATCH 1/3] Fix after addResponseHeader failing on built-in filter and handler responses Built-in filters and handlers (redirectTo, rateLimit, circuitBreaker, fn, stream) returned ServerResponse instances with read-only headers, so after filters such as addResponseHeader threw UnsupportedOperationException. Return GatewayServerResponse instead so response headers stay mutable. Fixes gh-4168 Signed-off-by: JoeCqupt --- .../config/RouterFunctionHolderFactory.java | 3 +- .../mvc/filter/Bucket4jFilterFunctions.java | 5 +- .../filter/CircuitBreakerFilterFunctions.java | 2 +- .../server/mvc/filter/FilterFunctions.java | 6 +- ...unctionHandlerRequestProcessingHelper.java | 8 +- .../server/mvc/handler/HandlerFunctions.java | 6 +- ...erFilterFunctionsResponseHeadersTests.java | 345 ++++++++++++++++++ 7 files changed, 362 insertions(+), 13 deletions(-) create mode 100644 spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java index a7fee09813..5646c24cbc 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java @@ -48,6 +48,7 @@ import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; import org.springframework.cloud.gateway.server.mvc.filter.FilterBeanFactoryDiscoverer; import org.springframework.cloud.gateway.server.mvc.filter.FilterDiscoverer; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.cloud.gateway.server.mvc.handler.HandlerDiscoverer; import org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctionDefinition; import org.springframework.cloud.gateway.server.mvc.invoke.InvocationContext; @@ -101,7 +102,7 @@ public String toString() { }; private static final RouterFunction NEVER_ROUTE = RouterFunctions.route(neverPredicate, - request -> ServerResponse.notFound().build()); + request -> GatewayServerResponse.notFound().build()); private final Log log = LogFactory.getLog(getClass()); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java index 8df51990ed..5b5f85ef88 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java @@ -32,6 +32,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.core.task.TaskExecutor; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; @@ -78,7 +79,7 @@ public static HandlerFilterFunction rateLimit( String key = config.getKeyResolver() != null ? config.getKeyResolver().apply(request) : ""; if (!StringUtils.hasText(key)) { // TODO: configurable empty key status code - return ServerResponse.status(HttpStatus.FORBIDDEN).build(); + return GatewayServerResponse.status(HttpStatus.FORBIDDEN).build(); } TaskExecutor taskExecutor = MvcUtils.getApplicationContext(request) .getBean(APPLICATION_TASK_EXECUTOR_BEAN_NAME, TaskExecutor.class); @@ -100,7 +101,7 @@ public static HandlerFilterFunction rateLimit( serverResponse.headers().add(config.getHeaderName(), String.valueOf(remainingTokens)); return serverResponse; } - return ServerResponse.status(config.getStatusCode()) + return GatewayServerResponse.status(config.getStatusCode()) .header(config.getHeaderName(), String.valueOf(remainingTokens)) .build(); }; diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java index d2f218ccf6..e001c73a37 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java @@ -114,7 +114,7 @@ public static HandlerFilterFunction circuitBreak } // if resume without error, return ok response if (config.isResumeWithoutError()) { - return ServerResponse.ok().build(); + return GatewayServerResponse.ok().build(); } throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, throwable != null ? throwable.getMessage() : null, throwable); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java index d21ce90102..2e5c1390b8 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java @@ -24,6 +24,7 @@ import org.springframework.cloud.gateway.server.mvc.common.Shortcut; import org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.DedupeStrategy; import org.springframework.cloud.gateway.server.mvc.filter.BeforeFilterFunctions.FallbackHeadersConfig; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatusCode; import org.springframework.util.Assert; @@ -129,8 +130,9 @@ static HandlerFilterFunction redirectTo(HttpStat static HandlerFilterFunction redirectTo(HttpStatusHolder status, URI uri) { Assert.isTrue(status.is3xxRedirection(), "status must be a 3xx code, but was " + status); - return (request, - next) -> ServerResponse.status(status.resolve()).header(HttpHeaders.LOCATION, uri.toString()).build(); + return (request, next) -> GatewayServerResponse.status(status.resolve()) + .header(HttpHeaders.LOCATION, uri.toString()) + .build(); } @Shortcut diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java index fd48f6c031..40b0e1c5ff 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java @@ -68,7 +68,7 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } if (function == null) { - return ServerResponse.notFound().build(); + return GatewayServerResponse.notFound().build(); } HttpHeaders headers = request.headers().asHttpHeaders(); @@ -95,13 +95,13 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } Object result = function.apply(inputMessage); if (function.isConsumer()) { - return HttpMethod.DELETE.equals(request.method()) ? ServerResponse.ok().build() - : ServerResponse.accepted() + return HttpMethod.DELETE.equals(request.method()) ? GatewayServerResponse.ok().build() + : GatewayServerResponse.accepted() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))) .build(); } - BodyBuilder responseOkBuilder = ServerResponse.ok() + BodyBuilder responseOkBuilder = GatewayServerResponse.ok() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))); if (result instanceof Iterable i) { List aggregatedResult = (List) StreamSupport.stream(i.spliterator(), false).map(m -> { diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java index 3bd102d583..128279d373 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java @@ -96,7 +96,7 @@ public static HandlerFunction fn(String functionName) { return processRequest(request, function, body, false, Collections.emptyList(), Collections.emptyList(), additionalRequestHeaders); } - return ServerResponse.notFound().build(); + return GatewayServerResponse.notFound().build(); }; } @@ -111,9 +111,9 @@ public static HandlerFunction stream(String bindingName) { .fromHttp(FunctionHandlerHeaderUtils.sanitize(request.headers().asHttpHeaders())); boolean send = streamOps.send(expandedBindingName, MessageBuilder.createMessage(body, messageHeaders)); if (send) { - return ServerResponse.accepted().build(); + return GatewayServerResponse.accepted().build(); } - return ServerResponse.badRequest().build(); + return GatewayServerResponse.badRequest().build(); }; } diff --git a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java new file mode 100644 index 0000000000..1970897571 --- /dev/null +++ b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java @@ -0,0 +1,345 @@ +/* + * Copyright 2026-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.server.mvc.filter; + +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; + +import com.github.benmanes.caffeine.cache.Caffeine; +import io.github.bucket4j.caffeine.CaffeineProxyManager; +import io.github.bucket4j.distributed.proxy.AsyncProxyManager; +import io.github.bucket4j.distributed.remote.RemoteBucketState; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.WebApplicationContextRunner; +import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; +import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; +import org.springframework.cloud.function.context.FunctionCatalog; +import org.springframework.cloud.function.context.config.ContextFunctionCatalogAutoConfiguration; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; +import org.springframework.cloud.stream.function.StreamOperations; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.task.SyncTaskExecutor; +import org.springframework.core.task.TaskExecutor; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.converter.ByteArrayHttpMessageConverter; +import org.springframework.http.converter.StringHttpMessageConverter; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.function.RouterFunction; +import org.springframework.web.servlet.function.ServerRequest; +import org.springframework.web.servlet.function.ServerResponse; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.springframework.boot.autoconfigure.task.TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.addResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.Bucket4jFilterFunctions.rateLimit; +import static org.springframework.cloud.gateway.server.mvc.filter.CircuitBreakerFilterFunctions.circuitBreaker; +import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.redirectTo; +import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route; +import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.fn; +import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.stream; +import static org.springframework.web.servlet.function.RequestPredicates.path; + +class AfterFilterFunctionsResponseHeadersTests { + + @Test + void addResponseHeaderOnServerResponseBuild() { + assertThatThrownBy(() -> apply(addResponseHeader("X-Test", "TestFoo"), ServerResponse.ok().build())) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void addResponseHeaderOnGatewayServerResponseBuild() { + ServerResponse response = apply(addResponseHeader("X-Test", "TestFoo"), GatewayServerResponse.ok().build()); + + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestFoo"); + } + + @Test + void addResponseHeaderAfterRedirectToFilter() throws Exception { + RouterFunction redirectRoute = route("redirectRoute") + .GET(path("/redirect"), req -> GatewayServerResponse.ok().build()) + .after(addResponseHeader("X-Test", "TestRedirect")) + .filter(redirectTo(HttpStatus.FOUND, URI.create("https://example.org"))) + .build(); + + ServerResponse response = handle(redirectRoute, request("GET", "/redirect", null)); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.FOUND); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestRedirect"); + } + + @Test + void addResponseHeaderAfterFnHandler() { + new WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ContextFunctionCatalogAutoConfiguration.class)) + .withUserConfiguration(UpperFunctionConfiguration.class) + .run(context -> { + RouterFunction fnRoute = route("fnRoute").POST(path("/fn"), fn("upper")) + .after(addResponseHeader("X-Test", "TestFn")) + .build(); + ServerRequest request = request("POST", "/fn", context, "hello"); + + ServerResponse response = handle(fnRoute, request); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestFn"); + }); + } + + @Test + void addResponseHeaderAfterFnHandlerFunctionNotFound() { + new WebApplicationContextRunner().withBean(FunctionCatalog.class, () -> mock(FunctionCatalog.class)) + .run(context -> { + RouterFunction fnRoute = route("fnRoute").GET(path("/fn"), fn("missing")) + .after(addResponseHeader("X-Test", "TestFn")) + .build(); + + ServerResponse response = handle(fnRoute, request("GET", "/fn", context)); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestFn"); + }); + } + + @Test + void addResponseHeaderAfterFnHandlerConsumerPost() { + consumerContextRunner().run(context -> { + RouterFunction fnRoute = route("fnRoute").POST(path("/fn"), fn("sink")) + .after(addResponseHeader("X-Test", "TestFn")) + .build(); + + ServerResponse response = handle(fnRoute, request("POST", "/fn", context, "hello")); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.ACCEPTED); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestFn"); + }); + } + + @Test + void addResponseHeaderAfterFnHandlerConsumerDelete() { + consumerContextRunner().run(context -> { + RouterFunction fnRoute = route("fnRoute").DELETE(path("/fn"), fn("sink")) + .after(addResponseHeader("X-Test", "TestFn")) + .build(); + + ServerResponse response = handle(fnRoute, request("DELETE", "/fn", context, "hello")); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestFn"); + }); + } + + @Test + void addResponseHeaderAfterStreamHandlerSendFailed() { + StreamOperations streamOperations = mock(StreamOperations.class); + given(streamOperations.send(anyString(), any())).willReturn(false); + new WebApplicationContextRunner().withBean(StreamOperations.class, () -> streamOperations).run(context -> { + RouterFunction streamRoute = route("streamRoute").POST(path("/stream"), stream("binding")) + .after(addResponseHeader("X-Test", "TestStream")) + .build(); + + ServerResponse response = handle(streamRoute, request("POST", "/stream", context, "hello")); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestStream"); + }); + } + + @Test + void rateLimitAllowedAddsHeaderToStreamHandlerResponse() { + StreamOperations streamOperations = mock(StreamOperations.class); + given(streamOperations.send(anyString(), any())).willReturn(true); + rateLimitContextRunner().withBean(StreamOperations.class, () -> streamOperations).run(context -> { + RouterFunction rateLimitRoute = route("rateLimitRoute") + .POST(path("/limited"), stream("binding")) + .filter(rateLimit(1, Duration.ofMinutes(1), req -> "client")) + .build(); + + ServerResponse response = handle(rateLimitRoute, request("POST", "/limited", context, "hello")); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.ACCEPTED); + assertThat(response.headers().getFirst(Bucket4jFilterFunctions.DEFAULT_HEADER_NAME)).isEqualTo("0"); + }); + } + + @Test + void addResponseHeaderAfterStreamHandler() { + StreamOperations streamOperations = mock(StreamOperations.class); + given(streamOperations.send(anyString(), any())).willReturn(true); + new WebApplicationContextRunner().withBean(StreamOperations.class, () -> streamOperations).run(context -> { + RouterFunction streamRoute = route("streamRoute").POST(path("/stream"), stream("binding")) + .after(addResponseHeader("X-Test", "TestStream")) + .build(); + ServerRequest request = request("POST", "/stream", context, "hello"); + + ServerResponse response = handle(streamRoute, request); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.ACCEPTED); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestStream"); + }); + } + + @Test + void addResponseHeaderAfterRateLimitRejectsEmptyKey() { + rateLimitContextRunner().run(context -> { + RouterFunction rateLimitRoute = route("rateLimitRoute") + .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) + .after(addResponseHeader("X-Test", "TestRateLimit")) + .filter(rateLimit(1, Duration.ofMinutes(1), req -> "")) + .build(); + + ServerResponse response = handle(rateLimitRoute, request("GET", "/limited", context)); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestRateLimit"); + }); + } + + @Test + void addResponseHeaderAfterRateLimitExceeded() { + rateLimitContextRunner().run(context -> { + RouterFunction rateLimitRoute = route("rateLimitRoute") + .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) + .after(addResponseHeader("X-Test", "TestRateLimit")) + .filter(rateLimit(1, Duration.ofMinutes(1), req -> "client")) + .build(); + handle(rateLimitRoute, request("GET", "/limited", context)); + + ServerResponse response = handle(rateLimitRoute, request("GET", "/limited", context)); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.TOO_MANY_REQUESTS); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestRateLimit"); + }); + } + + @Test + void addResponseHeaderAfterCircuitBreakerResumeWithoutError() { + CircuitBreakerFactory circuitBreakerFactory = mock(CircuitBreakerFactory.class); + given(circuitBreakerFactory.create("cb")).willReturn(new FallbackOnErrorCircuitBreaker()); + new WebApplicationContextRunner().withBean(CircuitBreakerFactory.class, () -> circuitBreakerFactory) + .run(context -> { + RouterFunction circuitBreakerRoute = route("circuitBreakerRoute") + .GET(path("/cb"), req -> { + throw new IllegalStateException("boom"); + }) + .after(addResponseHeader("X-Test", "TestCircuitBreaker")) + .filter(circuitBreaker(config -> config.setId("cb").setResumeWithoutError(true))) + .build(); + + ServerResponse response = handle(circuitBreakerRoute, request("GET", "/cb", context)); + + assertThat(response.statusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.headers().getFirst("X-Test")).isEqualTo("TestCircuitBreaker"); + }); + } + + private static WebApplicationContextRunner consumerContextRunner() { + return new WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ContextFunctionCatalogAutoConfiguration.class)) + .withUserConfiguration(SinkConsumerConfiguration.class); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static WebApplicationContextRunner rateLimitContextRunner() { + Caffeine caffeine = (Caffeine) Caffeine.newBuilder().maximumSize(100); + AsyncProxyManager proxyManager = new CaffeineProxyManager<>(caffeine, Duration.ofMinutes(1)).asAsync(); + return new WebApplicationContextRunner().withBean(AsyncProxyManager.class, () -> proxyManager) + .withBean(APPLICATION_TASK_EXECUTOR_BEAN_NAME, TaskExecutor.class, SyncTaskExecutor::new); + } + + private static ServerResponse handle(RouterFunction routerFunction, ServerRequest request) + throws Exception { + return routerFunction.route(request).orElseThrow().handle(request); + } + + private static ServerResponse apply(BiFunction filter, + ServerResponse response) { + return filter.apply(request("GET", "/foo", null), response); + } + + private static ServerRequest request(String method, String path, @Nullable WebApplicationContext context) { + return request(method, path, context, null); + } + + private static ServerRequest request(String method, String path, @Nullable WebApplicationContext context, + @Nullable String body) { + MockHttpServletRequest servletRequest = new MockHttpServletRequest(method, path); + if (context != null) { + servletRequest.setAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE, context); + } + if (body != null) { + servletRequest.setContentType(MediaType.TEXT_PLAIN_VALUE); + servletRequest.setContent(body.getBytes(StandardCharsets.UTF_8)); + } + return ServerRequest.create(servletRequest, + List.of(new StringHttpMessageConverter(), new ByteArrayHttpMessageConverter())); + } + + @Configuration(proxyBeanMethods = false) + static class UpperFunctionConfiguration { + + @Bean + Function upper() { + return String::toUpperCase; + } + + } + + @Configuration(proxyBeanMethods = false) + static class SinkConsumerConfiguration { + + @Bean + Consumer sink() { + return value -> { + }; + } + + } + + static class FallbackOnErrorCircuitBreaker implements CircuitBreaker { + + @Override + public T run(Supplier toRun, Function<@Nullable Throwable, T> fallback) { + try { + return toRun.get(); + } + catch (RuntimeException ex) { + return fallback.apply(ex); + } + } + + } + +} From ec78008b0b94cce0aa13a4b7ea54d7aa6db3108c Mon Sep 17 00:00:00 2001 From: joecqupt Date: Wed, 21 Oct 2026 17:08:33 +0800 Subject: [PATCH 2/3] Preserve native MVC response writing when adapting response headers Adapt responses at Gateway filter boundaries instead of replacing Spring response builders, preserving body writing and error handling behavior. See gh-4168 Signed-off-by: joecqupt --- .../config/RouterFunctionHolderFactory.java | 3 +- .../mvc/filter/Bucket4jFilterFunctions.java | 5 +- .../filter/CircuitBreakerFilterFunctions.java | 2 +- .../server/mvc/filter/FilterFunctions.java | 6 +- .../mvc/handler/DelegatingServerResponse.java | 332 ++++++++++ ...unctionHandlerRequestProcessingHelper.java | 8 +- .../mvc/handler/GatewayRouterFunctions.java | 3 +- .../GatewayRouterFunctionsBuilder.java | 66 +- .../mvc/handler/GatewayServerResponse.java | 4 + .../server/mvc/handler/HandlerFunctions.java | 6 +- ...erFilterFunctionsResponseHeadersTests.java | 473 ++++++++++++++- .../DelegatingServerResponseTests.java | 569 ++++++++++++++++++ 12 files changed, 1443 insertions(+), 34 deletions(-) create mode 100644 spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java create mode 100644 spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java index 5646c24cbc..a7fee09813 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java @@ -48,7 +48,6 @@ import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; import org.springframework.cloud.gateway.server.mvc.filter.FilterBeanFactoryDiscoverer; import org.springframework.cloud.gateway.server.mvc.filter.FilterDiscoverer; -import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.cloud.gateway.server.mvc.handler.HandlerDiscoverer; import org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctionDefinition; import org.springframework.cloud.gateway.server.mvc.invoke.InvocationContext; @@ -102,7 +101,7 @@ public String toString() { }; private static final RouterFunction NEVER_ROUTE = RouterFunctions.route(neverPredicate, - request -> GatewayServerResponse.notFound().build()); + request -> ServerResponse.notFound().build()); private final Log log = LogFactory.getLog(getClass()); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java index 5b5f85ef88..8df51990ed 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java @@ -32,7 +32,6 @@ import org.jspecify.annotations.Nullable; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; -import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.core.task.TaskExecutor; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; @@ -79,7 +78,7 @@ public static HandlerFilterFunction rateLimit( String key = config.getKeyResolver() != null ? config.getKeyResolver().apply(request) : ""; if (!StringUtils.hasText(key)) { // TODO: configurable empty key status code - return GatewayServerResponse.status(HttpStatus.FORBIDDEN).build(); + return ServerResponse.status(HttpStatus.FORBIDDEN).build(); } TaskExecutor taskExecutor = MvcUtils.getApplicationContext(request) .getBean(APPLICATION_TASK_EXECUTOR_BEAN_NAME, TaskExecutor.class); @@ -101,7 +100,7 @@ public static HandlerFilterFunction rateLimit( serverResponse.headers().add(config.getHeaderName(), String.valueOf(remainingTokens)); return serverResponse; } - return GatewayServerResponse.status(config.getStatusCode()) + return ServerResponse.status(config.getStatusCode()) .header(config.getHeaderName(), String.valueOf(remainingTokens)) .build(); }; diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java index e001c73a37..d2f218ccf6 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java @@ -114,7 +114,7 @@ public static HandlerFilterFunction circuitBreak } // if resume without error, return ok response if (config.isResumeWithoutError()) { - return GatewayServerResponse.ok().build(); + return ServerResponse.ok().build(); } throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, throwable != null ? throwable.getMessage() : null, throwable); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java index 2e5c1390b8..d21ce90102 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java @@ -24,7 +24,6 @@ import org.springframework.cloud.gateway.server.mvc.common.Shortcut; import org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.DedupeStrategy; import org.springframework.cloud.gateway.server.mvc.filter.BeforeFilterFunctions.FallbackHeadersConfig; -import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatusCode; import org.springframework.util.Assert; @@ -130,9 +129,8 @@ static HandlerFilterFunction redirectTo(HttpStat static HandlerFilterFunction redirectTo(HttpStatusHolder status, URI uri) { Assert.isTrue(status.is3xxRedirection(), "status must be a 3xx code, but was " + status); - return (request, next) -> GatewayServerResponse.status(status.resolve()) - .header(HttpHeaders.LOCATION, uri.toString()) - .build(); + return (request, + next) -> ServerResponse.status(status.resolve()).header(HttpHeaders.LOCATION, uri.toString()).build(); } @Shortcut diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java new file mode 100644 index 0000000000..85435909e3 --- /dev/null +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java @@ -0,0 +1,332 @@ +/* + * Copyright 2026-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.server.mvc.handler; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.Collection; +import java.util.Collections; +import java.util.Enumeration; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpServletResponseWrapper; +import org.jspecify.annotations.Nullable; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatusCode; +import org.springframework.util.MultiValueMap; +import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.servlet.function.AsyncServerResponse; +import org.springframework.web.servlet.function.EntityResponse; +import org.springframework.web.servlet.function.RenderingResponse; +import org.springframework.web.servlet.function.ServerResponse; + +class DelegatingServerResponse implements ServerResponse { + + private final ServerResponse delegate; + + private @Nullable HttpHeaders originalHeaders; + + private @Nullable HttpHeaders headers; + + private DelegatingServerResponse(ServerResponse delegate) { + this.delegate = delegate; + } + + static ServerResponse adapt(ServerResponse response) { + if (response instanceof GatewayServerResponse || response instanceof DelegatingServerResponse) { + return response; + } + if (response instanceof EntityResponse entityResponse) { + return new DelegatingEntityResponse<>(entityResponse); + } + if (response instanceof RenderingResponse renderingResponse) { + return new DelegatingRenderingResponse(renderingResponse); + } + if (response instanceof AsyncServerResponse asyncResponse) { + return new DelegatingAsyncServerResponse(asyncResponse); + } + return new DelegatingServerResponse(response); + } + + ServerResponse delegate() { + return this.delegate; + } + + @Override + public HttpStatusCode statusCode() { + return this.delegate.statusCode(); + } + + @Override + public HttpHeaders headers() { + if (this.headers == null) { + // An unfinished AsyncServerResponse cannot expose its metadata yet. + this.originalHeaders = HttpHeaders.copyOf(this.delegate.headers()); + this.headers = HttpHeaders.copyOf(this.originalHeaders); + } + return this.headers; + } + + @Override + public MultiValueMap cookies() { + return this.delegate.cookies(); + } + + @Override + public @Nullable ModelAndView writeTo(HttpServletRequest request, HttpServletResponse response, Context context) + throws ServletException, IOException { + Set changedNames = changedNames(); + if (changedNames.isEmpty()) { + return this.delegate.writeTo(request, response, context); + } + HttpHeaders effectiveHeaders = headers(); + for (String name : changedNames) { + List values = effectiveHeaders.getOrEmpty(name); + response.setHeader(name, values.isEmpty() ? null : values.get(0)); + for (int i = 1; i < values.size(); i++) { + response.addHeader(name, values.get(i)); + } + } + if ((changedNames.contains(HttpHeaders.ETAG) || changedNames.contains(HttpHeaders.LAST_MODIFIED)) + && ("GET".equals(request.getMethod()) || "HEAD".equals(request.getMethod()))) { + response.setStatus(statusCode().value()); + // Avoid writing validators twice when the delegate replays its headers. + HttpServletResponse conditionalResponse = new ChangedHeadersResponseWrapper(response, + Set.of(HttpHeaders.ETAG, HttpHeaders.LAST_MODIFIED)); + if (new ServletWebRequest(request, conditionalResponse).checkNotModified(effectiveHeaders.getETag(), + effectiveHeaders.getLastModified())) { + effectiveHeaders.forEach((name, values) -> { + if (!changedNames.contains(name)) { + values.forEach(value -> response.addHeader(name, value)); + } + }); + cookies().values().stream().flatMap(Collection::stream).forEach(response::addCookie); + return null; + } + request = new ConditionalHeadersRequestWrapper(request); + } + return this.delegate.writeTo(request, new ChangedHeadersResponseWrapper(response, changedNames), context); + } + + private Set changedNames() { + Set names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + if (this.headers != null && this.originalHeaders != null) { + names.addAll(this.originalHeaders.headerNames()); + names.addAll(this.headers.headerNames()); + names.removeIf(name -> Objects.equals(this.originalHeaders.get(name), this.headers.get(name))); + } + return names; + } + + private static final class ChangedHeadersResponseWrapper extends HttpServletResponseWrapper { + + private final Set changedNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + + private ChangedHeadersResponseWrapper(HttpServletResponse response, Set changedNames) { + super(response); + this.changedNames.addAll(changedNames); + } + + @Override + public void addHeader(String name, @Nullable String value) { + if (!this.changedNames.contains(name)) { + super.addHeader(name, value); + } + } + + @Override + public void setHeader(String name, @Nullable String value) { + if (!this.changedNames.contains(name)) { + super.setHeader(name, value); + } + } + + @Override + public void addDateHeader(String name, long date) { + if (!this.changedNames.contains(name)) { + super.addDateHeader(name, date); + } + } + + @Override + public void setDateHeader(String name, long date) { + if (!this.changedNames.contains(name)) { + super.setDateHeader(name, date); + } + } + + @Override + public void addIntHeader(String name, int value) { + if (!this.changedNames.contains(name)) { + super.addIntHeader(name, value); + } + } + + @Override + public void setIntHeader(String name, int value) { + if (!this.changedNames.contains(name)) { + super.setIntHeader(name, value); + } + } + + @Override + public void setContentType(@Nullable String type) { + if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { + super.setContentType(type); + } + } + + @Override + public void setContentLength(int length) { + if (!this.changedNames.contains(HttpHeaders.CONTENT_LENGTH)) { + super.setContentLength(length); + } + } + + @Override + public void setContentLengthLong(long length) { + if (!this.changedNames.contains(HttpHeaders.CONTENT_LENGTH)) { + super.setContentLengthLong(length); + } + } + + @Override + public void setCharacterEncoding(@Nullable String charset) { + // Servlet charset setters also modify Content-Type. + if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { + super.setCharacterEncoding(charset); + } + } + + @Override + public void setCharacterEncoding(Charset charset) { + if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { + super.setCharacterEncoding(charset); + } + } + + } + + private static final class ConditionalHeadersRequestWrapper extends HttpServletRequestWrapper { + + private static final Set CONDITIONAL_HEADERS = Set.of(HttpHeaders.IF_MATCH, HttpHeaders.IF_NONE_MATCH, + HttpHeaders.IF_MODIFIED_SINCE, HttpHeaders.IF_UNMODIFIED_SINCE); + + private ConditionalHeadersRequestWrapper(HttpServletRequest request) { + super(request); + } + + private boolean isConditional(String name) { + return CONDITIONAL_HEADERS.stream().anyMatch(name::equalsIgnoreCase); + } + + @Override + public @Nullable String getHeader(String name) { + return isConditional(name) ? null : super.getHeader(name); + } + + @Override + public Enumeration getHeaders(String name) { + return isConditional(name) ? Collections.emptyEnumeration() : super.getHeaders(name); + } + + @Override + public Enumeration getHeaderNames() { + return Collections.enumeration( + Collections.list(super.getHeaderNames()).stream().filter(name -> !isConditional(name)).toList()); + } + + @Override + public long getDateHeader(String name) { + return isConditional(name) ? -1 : super.getDateHeader(name); + } + + @Override + public int getIntHeader(String name) { + return isConditional(name) ? -1 : super.getIntHeader(name); + } + + } + + private static final class DelegatingAsyncServerResponse extends DelegatingServerResponse + implements AsyncServerResponse { + + private final AsyncServerResponse delegate; + + private DelegatingAsyncServerResponse(AsyncServerResponse delegate) { + super(delegate); + this.delegate = delegate; + } + + @Override + public ServerResponse block() { + return this.delegate.block(); + } + + } + + private static final class DelegatingEntityResponse extends DelegatingServerResponse + implements EntityResponse { + + private final EntityResponse delegate; + + private DelegatingEntityResponse(EntityResponse delegate) { + super(delegate); + this.delegate = delegate; + } + + @Override + public T entity() { + return this.delegate.entity(); + } + + } + + private static final class DelegatingRenderingResponse extends DelegatingServerResponse + implements RenderingResponse { + + private final RenderingResponse delegate; + + private DelegatingRenderingResponse(RenderingResponse delegate) { + super(delegate); + this.delegate = delegate; + } + + @Override + public String name() { + return this.delegate.name(); + } + + @Override + public Map model() { + return this.delegate.model(); + } + + } + +} diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java index 40b0e1c5ff..fd48f6c031 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java @@ -68,7 +68,7 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } if (function == null) { - return GatewayServerResponse.notFound().build(); + return ServerResponse.notFound().build(); } HttpHeaders headers = request.headers().asHttpHeaders(); @@ -95,13 +95,13 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } Object result = function.apply(inputMessage); if (function.isConsumer()) { - return HttpMethod.DELETE.equals(request.method()) ? GatewayServerResponse.ok().build() - : GatewayServerResponse.accepted() + return HttpMethod.DELETE.equals(request.method()) ? ServerResponse.ok().build() + : ServerResponse.accepted() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))) .build(); } - BodyBuilder responseOkBuilder = GatewayServerResponse.ok() + BodyBuilder responseOkBuilder = ServerResponse.ok() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))); if (result instanceof Iterable i) { List aggregatedResult = (List) StreamSupport.stream(i.spliterator(), false).map(m -> { diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java index ad0c82334f..d8509d0f0c 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java @@ -35,8 +35,7 @@ public static Builder route() { } public static Builder route(String routeId) { - Builder builder = RouterFunctions.route().before(routeId(routeId)); - return new GatewayRouterFunctionsBuilder(builder, routeId); + return new GatewayRouterFunctionsBuilder(RouterFunctions.route(), routeId).before(routeId(routeId)); } public static RouterFunction route(RequestPredicate predicate, diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java index 6a602d25e2..40033a3c30 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java @@ -16,6 +16,8 @@ package org.springframework.cloud.gateway.server.mvc.handler; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.BiConsumer; @@ -24,10 +26,12 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.stream.Stream; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; +import org.springframework.util.Assert; import org.springframework.web.servlet.function.HandlerFilterFunction; import org.springframework.web.servlet.function.HandlerFunction; import org.springframework.web.servlet.function.RequestPredicate; @@ -42,6 +46,10 @@ public class GatewayRouterFunctionsBuilder implements RouterFunctions.Builder { private final String routeId; + private final List> filters = new ArrayList<>(); + + private final List> errorHandlers = new ArrayList<>(); + public GatewayRouterFunctionsBuilder(RouterFunctions.Builder builder, String routeId) { this.builder = builder; this.routeId = routeId; @@ -50,6 +58,12 @@ public GatewayRouterFunctionsBuilder(RouterFunctions.Builder builder, String rou @Override public RouterFunction build() { RouterFunction routerFunction = builder.build(); + Optional> filter = Stream + .concat(this.filters.stream(), this.errorHandlers.stream()) + .reduce(HandlerFilterFunction::andThen); + if (filter.isPresent()) { + routerFunction = routerFunction.filter(filter.get()); + } RouterFunction withAttributes = routerFunction.withAttribute(MvcUtils.GATEWAY_ROUTE_ID_ATTR, this.routeId); return new GatewayDelegatingRouterFunction<>(withAttributes, this.routeId); @@ -255,13 +269,15 @@ public RouterFunctions.Builder add(RouterFunction @Override public RouterFunctions.Builder resource(RequestPredicate predicate, Resource resource) { - return builder.resource(predicate, resource); + builder.resource(predicate, resource); + return this; } @Override public RouterFunctions.Builder resource(RequestPredicate predicate, Resource resource, BiConsumer headersConsumer) { - return builder.resource(predicate, resource, headersConsumer); + builder.resource(predicate, resource, headersConsumer); + return this; } @Override @@ -299,7 +315,13 @@ public RouterFunctions.Builder nest(RequestPredicate @Override public RouterFunctions.Builder nest(RequestPredicate predicate, Consumer builderConsumer) { - builder.nest(predicate, builderConsumer); + Assert.notNull(builderConsumer, "BuilderConsumer must not be null"); + builder.nest(predicate, () -> { + GatewayRouterFunctionsBuilder nested = new GatewayRouterFunctionsBuilder(RouterFunctions.route(), + this.routeId); + builderConsumer.accept(nested); + return nested.build(); + }); return this; } @@ -312,42 +334,64 @@ public RouterFunctions.Builder path(String pattern, @Override public RouterFunctions.Builder path(String pattern, Consumer builderConsumer) { - builder.path(pattern, builderConsumer); + Assert.notNull(builderConsumer, "BuilderConsumer must not be null"); + builder.path(pattern, () -> { + GatewayRouterFunctionsBuilder nested = new GatewayRouterFunctionsBuilder(RouterFunctions.route(), + this.routeId); + builderConsumer.accept(nested); + return nested.build(); + }); return this; } @Override + @SuppressWarnings("unchecked") public RouterFunctions.Builder filter( HandlerFilterFunction filterFunction) { - builder.filter(filterFunction); + Assert.notNull(filterFunction, "FilterFunction must not be null"); + HandlerFilterFunction adaptingFilter = (request, next) -> { + HandlerFunction adaptingNext = downstreamRequest -> (T) GatewayServerResponse + .adapt(next.handle(downstreamRequest)); + return GatewayServerResponse.adapt(filterFunction.filter(request, adaptingNext)); + }; + this.filters.add(adaptingFilter); return this; } @Override public RouterFunctions.Builder before(Function requestProcessor) { - builder.before(requestProcessor); + this.filters.add(HandlerFilterFunction.ofRequestProcessor(requestProcessor)); return this; } @Override public RouterFunctions.Builder after( BiFunction responseProcessor) { - builder.after(responseProcessor); - return this; + return filter(HandlerFilterFunction.ofResponseProcessor(responseProcessor)); } @Override public RouterFunctions.Builder onError(Predicate predicate, BiFunction responseProvider) { - builder.onError(predicate, responseProvider); + Assert.notNull(responseProvider, "ResponseProvider must not be null"); + HandlerFilterFunction errorFilter = HandlerFilterFunction + .ofErrorHandler(predicate, (failure, request) -> responseProvider.apply(failure, request)); + this.errorHandlers.add(0, (request, next) -> errorFilter.filter(request, downstreamRequest -> { + ServerResponse response = next.handle(downstreamRequest); + if (response instanceof DelegatingServerResponse delegatingResponse) { + // Spring registers write errors only on its own response base class. + errorFilter.filter(downstreamRequest, ignored -> delegatingResponse.delegate()); + } + return response; + })); return this; } @Override public RouterFunctions.Builder onError(Class exceptionType, BiFunction responseProvider) { - builder.onError(exceptionType, responseProvider); - return this; + Assert.notNull(exceptionType, "ExceptionType must not be null"); + return onError(exceptionType::isInstance, responseProvider); } @Override diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java index b54add76ce..c0607fa55c 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java @@ -30,6 +30,10 @@ public interface GatewayServerResponse extends ServerResponse { void setStatusCode(HttpStatusCode statusCode); + static ServerResponse adapt(ServerResponse response) { + return DelegatingServerResponse.adapt(response); + } + // Static methods /** diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java index 128279d373..3bd102d583 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java @@ -96,7 +96,7 @@ public static HandlerFunction fn(String functionName) { return processRequest(request, function, body, false, Collections.emptyList(), Collections.emptyList(), additionalRequestHeaders); } - return GatewayServerResponse.notFound().build(); + return ServerResponse.notFound().build(); }; } @@ -111,9 +111,9 @@ public static HandlerFunction stream(String bindingName) { .fromHttp(FunctionHandlerHeaderUtils.sanitize(request.headers().asHttpHeaders())); boolean send = streamOps.send(expandedBindingName, MessageBuilder.createMessage(body, messageHeaders)); if (send) { - return GatewayServerResponse.accepted().build(); + return ServerResponse.accepted().build(); } - return GatewayServerResponse.badRequest().build(); + return ServerResponse.badRequest().build(); }; } diff --git a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java index 1970897571..fb2c669d5d 100644 --- a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java +++ b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java @@ -16,10 +16,14 @@ package org.springframework.cloud.gateway.server.mvc.filter; +import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.ArrayList; import java.util.List; +import java.util.Locale; +import java.util.concurrent.CompletableFuture; import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; @@ -31,6 +35,7 @@ import io.github.bucket4j.distributed.remote.RemoteBucketState; import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; @@ -42,6 +47,7 @@ import org.springframework.cloud.stream.function.StreamOperations; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.io.ByteArrayResource; import org.springframework.core.task.SyncTaskExecutor; import org.springframework.core.task.TaskExecutor; import org.springframework.http.HttpStatus; @@ -49,9 +55,18 @@ import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.result.MockMvcResultMatchers; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.function.AsyncServerResponse; +import org.springframework.web.servlet.function.EntityResponse; +import org.springframework.web.servlet.function.HandlerFunction; import org.springframework.web.servlet.function.RouterFunction; +import org.springframework.web.servlet.function.RouterFunctions; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.servlet.function.ServerResponse; @@ -63,16 +78,429 @@ import static org.mockito.Mockito.mock; import static org.springframework.boot.autoconfigure.task.TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME; import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.addResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.dedupeResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.removeResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.rewriteLocationResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.rewriteResponseHeader; +import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.setResponseHeader; import static org.springframework.cloud.gateway.server.mvc.filter.Bucket4jFilterFunctions.rateLimit; import static org.springframework.cloud.gateway.server.mvc.filter.CircuitBreakerFilterFunctions.circuitBreaker; import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.redirectTo; import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route; import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.fn; import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.stream; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.web.servlet.function.RequestPredicates.path; class AfterFilterFunctionsResponseHeadersTests { + @Test + void addResponseHeaderOnCustomHandlerRoute() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET(path("/foo"), req -> ServerResponse.ok().build()) + .after(addResponseHeader("X-Test", "TestFoo")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("")); + } + + @Test + void responseHeaderFiltersPreserveEntityBody() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", + req -> ServerResponse.ok() + .header("X-Source", "old") + .header("X-Remove", "old") + .header("X-Duplicate", "same") + .header("X-Rewrite", "old") + .body("hello")) + .after(dedupeResponseHeader("X-Duplicate")) + .after(addResponseHeader("X-Duplicate", "same")) + .after(rewriteResponseHeader("X-Rewrite", "old", "new")) + .after(setResponseHeader("X-Source", "new")) + .after(removeResponseHeader("X-Remove")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(header().stringValues("X-Source", "new")) + .andExpect(header().doesNotExist("X-Remove")) + .andExpect(header().stringValues("X-Duplicate", "same")) + .andExpect(header().string("X-Rewrite", "new")) + .andExpect(content().string("hello")); + } + + @Test + void locationResponseHeaderFilterPreservesBody() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.ok().header("Location", "https://upstream.example/foo").body("hello")) + .after(rewriteLocationResponseHeader()) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo").header("Host", "gateway.example")) + .andExpect(status().isOk()) + .andExpect(header().string("Location", "https://gateway.example/foo")) + .andExpect(content().string("hello")); + } + + @Test + void customFilterCanMutateDownstreamResponse() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.ok().body("hello")) + .filter((request, next) -> { + ServerResponse response = next.handle(request); + response.headers().add("X-Custom", "yes"); + return response; + }) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(header().string("X-Custom", "yes")) + .andExpect(content().string("hello")); + } + + @Test + void customFilterShortCircuitResponseIsAdapted() throws Exception { + RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> { + throw new AssertionError("Handler must not be called"); + }) + .after(addResponseHeader("X-Test", "TestFoo")) + .filter((request, next) -> ServerResponse.status(HttpStatus.FORBIDDEN).body("denied")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isForbidden()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("denied")); + } + + @Test + void responseReplacedByAfterFilterIsAdapted() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.ok().body("original")) + .after(addResponseHeader("X-Test", "TestFoo")) + .after((request, response) -> ServerResponse.accepted().body("replacement")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isAccepted()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("replacement")); + } + + @Test + void responseFiltersKeepTheirExecutionOrder() throws Exception { + RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.ok().build()) + .after(addResponseHeader("X-Order", "outer")) + .after(addResponseHeader("X-Order", "inner")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(header().stringValues("X-Order", "inner", "outer")); + } + + @Test + void pathNestedResponseFiltersAreAdapted() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .path("/api", + nested -> nested.GET("/foo", req -> ServerResponse.ok().body("hello")) + .after(addResponseHeader("X-Test", "TestFoo"))) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/api/foo")) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("hello")); + } + + @Test + void predicateNestedResponseFiltersAreAdapted() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .nest(path("/api"), + nested -> nested.GET("/foo", req -> ServerResponse.ok().body("hello")) + .after(addResponseHeader("X-Test", "TestFoo"))) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/api/foo")) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("hello")); + } + + @Test + void resourceFluentResponseFiltersAreAdapted() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .resource(path("/foo"), new ByteArrayResource("hello".getBytes(StandardCharsets.UTF_8))) + .after(addResponseHeader("X-Test", "TestFoo")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("hello")); + } + + @Test + void typedEntityResponseProcessorStillReceivesEntityResponse() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> EntityResponse.fromObject("hello").build()) + .after((ServerRequest request, EntityResponse response) -> { + assertThat(response.entity()).isEqualTo("hello"); + response.headers().add("X-Typed", "yes"); + return response; + }) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(header().string("X-Typed", "yes")) + .andExpect(content().string("hello")); + } + + @Test + void responseHeaderFilterPreservesCustomBodyWriter() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.ok().header("X-Source", "old").build((request, response) -> { + response.getWriter().write("custom"); + response.flushBuffer(); + return null; + })) + .after(setResponseHeader("X-Source", "new")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(header().stringValues("X-Source", "new")) + .andExpect(content().string("custom")); + } + + @Test + void typedAsyncResponseFilterStillReceivesAsyncServerResponse() throws Exception { + ServerResponse completed = ServerResponse.ok().body("hello"); + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.async(CompletableFuture.completedFuture(completed))) + .filter((ServerRequest request, HandlerFunction next) -> { + AsyncServerResponse response = next.handle(request); + assertThat(response.block()).isSameAs(completed); + return response; + }) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/foo")) + .andExpect(status().isOk()) + .andExpect(content().string("hello")); + } + + @Test + void outerErrorHandlerStillHandlesNestedResponseWriteFailure() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .path("/api", nested -> nested.GET("/foo", req -> ServerResponse.ok().build((request, response) -> { + throw new IOException("write failed"); + })).filter((request, next) -> next.handle(request))) + .onError(IOException.class, + (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("fallback")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/api/foo")) + .andExpect(status().isBadGateway()) + .andExpect(content().string("fallback")); + } + + @Test + void nestedResponseHeaderMutationRetainsOuterWriteErrorHandling() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .path("/api", nested -> nested.GET("/foo", req -> ServerResponse.ok().build((request, response) -> { + throw new IOException("write failed"); + })).after(addResponseHeader("X-Test", "TestFoo"))) + .onError(IOException.class, + (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY) + .header("X-Fallback", "yes") + .body("fallback")) + .build(); + + MockMvcBuilders.routerFunctions(fooRoute) + .build() + .perform(get("/api/foo")) + .andExpect(status().isBadGateway()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(header().string("X-Fallback", "yes")) + .andExpect(content().string("fallback")); + } + + @Test + void firstRegisteredErrorHandlerKeepsPrecedenceForHandlerAndWriteFailures() throws Exception { + RouterFunction fooRoute = route("fooRoute").GET("/handler", req -> { + throw new IOException("handler failed"); + }).path("/api", nested -> nested.GET("/writer", req -> ServerResponse.ok().build((request, response) -> { + throw new IOException("write failed"); + })).filter((request, next) -> next.handle(request))) + .onError(IOException.class, + (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("first")) + .onError(IOException.class, + (failure, request) -> ServerResponse.status(HttpStatus.SERVICE_UNAVAILABLE).body("second")) + .after(addResponseHeader("X-Test", "TestFoo")) + .build(); + MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); + + for (String path : List.of("/handler", "/api/writer")) { + mockMvc.perform(get(path)) + .andExpect(status().isBadGateway()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("first")); + } + } + + @Test + void requestProcessorsKeepTheirOrderRelativeToFilters() throws Exception { + List calls = new ArrayList<>(); + RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> { + calls.add("handler"); + return ServerResponse.ok().build(); + }).before(request -> { + calls.add("beforeOuter"); + return request; + }).filter((request, next) -> { + calls.add("filterBefore"); + ServerResponse response = next.handle(request); + calls.add("filterAfter"); + return response; + }).before(request -> { + calls.add("beforeInner"); + return request; + }).build(); + + MockMvcBuilders.routerFunctions(fooRoute).build().perform(get("/foo")).andExpect(status().isOk()); + + assertThat(calls).containsExactly("beforeOuter", "filterBefore", "beforeInner", "handler", "filterAfter"); + } + + @Test + void buildingAgainDoesNotChangePreviouslyBuiltFilterChains() throws Exception { + RouterFunctions.Builder builder = route("fooRoute").GET("/foo", req -> ServerResponse.ok().build()) + .after(addResponseHeader("X-Test", "first")); + RouterFunction firstRoute = builder.build(); + RouterFunction secondRoute = builder.after(addResponseHeader("X-Test", "second")).build(); + + MockMvcBuilders.routerFunctions(firstRoute) + .build() + .perform(get("/foo")) + .andExpect(header().stringValues("X-Test", "first")); + MockMvcBuilders.routerFunctions(secondRoute) + .build() + .perform(get("/foo")) + .andExpect(header().stringValues("X-Test", "second", "first")); + } + + @Test + void outerErrorHandlerStillHandlesNestedAsyncResponseFailure() throws Exception { + CompletableFuture response = new CompletableFuture<>(); + RouterFunction fooRoute = route("fooRoute") + .path("/api", + nested -> nested.GET("/foo", req -> ServerResponse.async(response)) + .filter((request, next) -> next.handle(request))) + .onError(IOException.class, + (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("fallback")) + .build(); + MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); + + MvcResult result = mockMvc.perform(get("/api/foo")) + .andExpect(MockMvcResultMatchers.request().asyncStarted()) + .andReturn(); + response.completeExceptionally(new IOException("async failed")); + result.getAsyncResult(5000); + mockMvc.perform(asyncDispatch(result)) + .andExpect(status().isBadGateway()) + .andExpect(content().string("fallback")); + } + + @Test + void filterDoesNotReadUnresolvedAsyncResponseMetadata() throws Exception { + CompletableFuture response = new CompletableFuture<>(); + RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.async(response)) + .filter((request, next) -> next.handle(request)) + .build(); + MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); + + MvcResult result = mockMvc.perform(get("/foo")) + .andExpect(MockMvcResultMatchers.request().asyncStarted()) + .andReturn(); + response.complete(ServerResponse.ok().body("hello")); + result.getAsyncResult(5000); + mockMvc.perform(asyncDispatch(result)).andExpect(status().isOk()).andExpect(content().string("hello")); + } + + @Test + void responseHeaderFilterPreservesPublisherBody() throws Exception { + RouterFunction fooRoute = route("fooRoute") + .GET("/foo", req -> ServerResponse.ok().body(Flux.just("hello", "world"))) + .after(addResponseHeader("X-Test", "TestFoo")) + .build(); + MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); + + MvcResult result = mockMvc.perform(get("/foo")) + .andExpect(MockMvcResultMatchers.request().asyncStarted()) + .andReturn(); + result.getAsyncResult(5000); + mockMvc.perform(asyncDispatch(result)) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("helloworld")); + } + + @Test + void responseHeaderFilterPreservesCompletionStageBody() throws Exception { + CompletableFuture body = new CompletableFuture<>(); + RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.ok().body(body)) + .after(addResponseHeader("X-Test", "TestFoo")) + .build(); + MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); + + MvcResult result = mockMvc.perform(get("/foo")) + .andExpect(MockMvcResultMatchers.request().asyncStarted()) + .andReturn(); + body.complete("hello"); + result.getAsyncResult(5000); + mockMvc.perform(asyncDispatch(result)) + .andExpect(status().isOk()) + .andExpect(header().string("X-Test", "TestFoo")) + .andExpect(content().string("hello")); + } + @Test void addResponseHeaderOnServerResponseBuild() { assertThatThrownBy(() -> apply(addResponseHeader("X-Test", "TestFoo"), ServerResponse.ok().build())) @@ -89,7 +517,7 @@ void addResponseHeaderOnGatewayServerResponseBuild() { @Test void addResponseHeaderAfterRedirectToFilter() throws Exception { RouterFunction redirectRoute = route("redirectRoute") - .GET(path("/redirect"), req -> GatewayServerResponse.ok().build()) + .GET(path("/redirect"), req -> ServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRedirect")) .filter(redirectTo(HttpStatus.FOUND, URI.create("https://example.org"))) .build(); @@ -118,6 +546,26 @@ void addResponseHeaderAfterFnHandler() { }); } + @Test + void addResponseHeaderAfterFnHandlerPreservesPublisherBody() { + new WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ContextFunctionCatalogAutoConfiguration.class)) + .withUserConfiguration(UpperFunctionConfiguration.class) + .run(context -> { + RouterFunction fnRoute = route("fnRoute").POST(path("/fn"), fn("upperFlux")) + .after(addResponseHeader("X-Test", "TestFn")) + .build(); + ServerRequest request = request("POST", "/fn", context, "hello"); + + ServerResponse response = fnRoute.route(request).orElseThrow().handle(request); + MockHttpServletResponse servletResponse = write(response, request); + + assertThat(servletResponse.getStatus()).isEqualTo(HttpStatus.OK.value()); + assertThat(servletResponse.getHeader("X-Test")).isEqualTo("TestFn"); + assertThat(servletResponse.getContentAsString()).isEqualTo("HELLO"); + }); + } + @Test void addResponseHeaderAfterFnHandlerFunctionNotFound() { new WebApplicationContextRunner().withBean(FunctionCatalog.class, () -> mock(FunctionCatalog.class)) @@ -215,7 +663,7 @@ void addResponseHeaderAfterStreamHandler() { void addResponseHeaderAfterRateLimitRejectsEmptyKey() { rateLimitContextRunner().run(context -> { RouterFunction rateLimitRoute = route("rateLimitRoute") - .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) + .GET(path("/limited"), req -> ServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRateLimit")) .filter(rateLimit(1, Duration.ofMinutes(1), req -> "")) .build(); @@ -231,7 +679,7 @@ void addResponseHeaderAfterRateLimitRejectsEmptyKey() { void addResponseHeaderAfterRateLimitExceeded() { rateLimitContextRunner().run(context -> { RouterFunction rateLimitRoute = route("rateLimitRoute") - .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) + .GET(path("/limited"), req -> ServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRateLimit")) .filter(rateLimit(1, Duration.ofMinutes(1), req -> "client")) .build(); @@ -281,7 +729,18 @@ private static WebApplicationContextRunner rateLimitContextRunner() { private static ServerResponse handle(RouterFunction routerFunction, ServerRequest request) throws Exception { - return routerFunction.route(request).orElseThrow().handle(request); + ServerResponse response = routerFunction.route(request).orElseThrow().handle(request); + MockHttpServletResponse servletResponse = write(response, request); + assertThat(servletResponse.getStatus()).isEqualTo(response.statusCode().value()); + response.headers() + .forEach((name, values) -> assertThat(servletResponse.getHeaders(name)).containsExactlyElementsOf(values)); + return response; + } + + private static MockHttpServletResponse write(ServerResponse response, ServerRequest request) throws Exception { + MockHttpServletResponse servletResponse = new MockHttpServletResponse(); + response.writeTo(request.servletRequest(), servletResponse, request::messageConverters); + return servletResponse; } private static ServerResponse apply(BiFunction filter, @@ -296,6 +755,7 @@ private static ServerRequest request(String method, String path, @Nullable WebAp private static ServerRequest request(String method, String path, @Nullable WebApplicationContext context, @Nullable String body) { MockHttpServletRequest servletRequest = new MockHttpServletRequest(method, path); + servletRequest.setAsyncSupported(true); if (context != null) { servletRequest.setAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE, context); } @@ -315,6 +775,11 @@ Function upper() { return String::toUpperCase; } + @Bean + Function> upperFlux() { + return value -> Flux.just(value.toUpperCase(Locale.ROOT)); + } + } @Configuration(proxyBeanMethods = false) diff --git a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java new file mode 100644 index 0000000000..0a9cd5839e --- /dev/null +++ b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java @@ -0,0 +1,569 @@ +/* + * Copyright 2026-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.server.mvc.handler; + +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import org.springframework.core.io.ByteArrayResource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.converter.ByteArrayHttpMessageConverter; +import org.springframework.http.converter.ResourceHttpMessageConverter; +import org.springframework.http.converter.ResourceRegionHttpMessageConverter; +import org.springframework.http.converter.StringHttpMessageConverter; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.web.context.request.async.WebAsyncUtils; +import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.servlet.function.EntityResponse; +import org.springframework.web.servlet.function.RenderingResponse; +import org.springframework.web.servlet.function.ServerResponse; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class DelegatingServerResponseTests { + + private static final long LAST_MODIFIED = 1445412480000L; + + private final ServerResponse.Context context = () -> List.of(new StringHttpMessageConverter(), + new ByteArrayHttpMessageConverter(), new ResourceHttpMessageConverter(), + new ResourceRegionHttpMessageConverter()); + + @Test + void adaptationIsIdempotentAndDoesNotAddGatewayStatusMutability() { + ServerResponse delegate = ServerResponse.ok().body("body"); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + ServerResponse gateway = GatewayServerResponse.ok().body("gateway"); + + assertThat(adapted).isNotInstanceOf(GatewayServerResponse.class); + assertThat(DelegatingServerResponse.adapt(adapted)).isSameAs(adapted); + assertThat(DelegatingServerResponse.adapt(gateway)).isSameAs(gateway); + } + + @Test + void unaccessedHeadersPassTheOriginalServletObjectsToTheDelegate() throws Exception { + AtomicReference writtenRequest = new AtomicReference<>(); + AtomicReference writtenResponse = new AtomicReference<>(); + ServerResponse delegate = ServerResponse.ok().build((request, response) -> { + writtenRequest.set(request); + writtenResponse.set(response); + return null; + }); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + DelegatingServerResponse.adapt(delegate).writeTo(request, response, this.context); + + assertThat(writtenRequest.get()).isSameAs(request); + assertThat(writtenResponse.get()).isSameAs(response); + } + + @Test + void unchangedHeadersPassTheOriginalServletResponseToTheDelegate() throws Exception { + AtomicReference writtenResponse = new AtomicReference<>(); + ServerResponse delegate = ServerResponse.ok().header("X-Test", "original").build((request, response) -> { + writtenResponse.set(response); + return null; + }); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + adapted.headers().set("x-test", "temporary"); + adapted.headers().set("X-TEST", "original"); + MockHttpServletResponse response = write(adapted); + + assertThat(writtenResponse.get()).isSameAs(response); + assertThat(response.getHeaders("X-Test")).containsExactly("original"); + } + + @Test + void headersAreMutableDeepCopies() { + ServerResponse delegate = ServerResponse.ok().header("X-Test", "one", "two").body("body"); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + + adapted.headers().get("X-Test").remove(0); + adapted.headers().get("X-Test").add("three"); + + assertThat(adapted.headers().get("X-Test")).containsExactly("two", "three"); + assertThat(delegate.headers().get("X-Test")).containsExactly("one", "two"); + assertThat(adapted.headers()).isSameAs(adapted.headers()); + } + + @Test + void addingHeadersDoesNotDuplicateOriginalValuesAndPreservesBody() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().header("X-Test", "one").body("original body")); + adapted.headers().add("X-Test", "two"); + adapted.headers().add("X-New", "three"); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.getHeaders("X-Test")).containsExactly("one", "two"); + assertThat(response.getHeaders("X-New")).containsExactly("three"); + assertThat(response.getContentAsString()).isEqualTo("original body"); + assertThat(response.getContentType()).startsWith("text/plain"); + assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("13"); + } + + @Test + void settingHeadersReplacesAllOriginalValuesIgnoringCase() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().header("X-Test", "one", "two").body("body")); + adapted.headers().set("x-TEST", "replacement"); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.getHeaders("X-Test")).containsExactly("replacement"); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void removingHeadersPreventsTheDelegateFromRestoringThem() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().header("X-Test", "one").body("body")); + adapted.headers().remove("x-test"); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.containsHeader("X-Test")).isFalse(); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void deduplicatingHeadersDoesNotReintroduceOriginalDuplicates() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().header("X-Test", "one", "one", "two").body("body")); + adapted.headers().put("x-test", List.copyOf(new LinkedHashSet<>(adapted.headers().get("X-Test")))); + + assertThat(write(adapted).getHeaders("X-Test")).containsExactly("one", "two"); + } + + @Test + void changedHeadersReplaceValuesAlreadyPresentOnTheServletResponse() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().header("X-Test", "original").header("X-Remove", "original").body("body")); + adapted.headers().set("X-Test", "replacement"); + adapted.headers().remove("X-Remove"); + MockHttpServletResponse response = new MockHttpServletResponse(); + response.addHeader("X-Test", "earlier"); + response.addHeader("X-Remove", "earlier"); + + adapted.writeTo(new MockHttpServletRequest("GET", "/"), response, this.context); + + assertThat(response.getHeaders("X-Test")).containsExactly("replacement"); + assertThat(response.containsHeader("X-Remove")).isFalse(); + } + + @Test + void cookiesAndStatusAreDelegatedRatherThanFrozen() throws Exception { + Cookie originalCookie = new Cookie("original", "one"); + Cookie dynamicCookie = new Cookie("dynamic", "two"); + ServerResponse delegate = ServerResponse.accepted().cookie(originalCookie).build((request, response) -> { + response.setStatus(HttpStatus.PARTIAL_CONTENT.value()); + response.addCookie(dynamicCookie); + return null; + }); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + adapted.headers().add("X-Test", "value"); + + MockHttpServletResponse response = write(adapted); + + assertThat(adapted.statusCode()).isSameAs(delegate.statusCode()); + assertThat(adapted.cookies()).isSameAs(delegate.cookies()); + assertThat(response.getStatus()).isEqualTo(HttpStatus.PARTIAL_CONTENT.value()); + assertThat(response.getCookies()).containsExactly(originalCookie, dynamicCookie); + } + + @Test + void contentLengthChangesSuppressBothServletPropertySetters() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().contentLength(111).build((request, response) -> { + response.setContentLength(222); + response.setContentLengthLong(333); + response.getOutputStream().write("body".getBytes(StandardCharsets.UTF_8)); + return null; + })); + adapted.headers().setContentLength(4); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.getHeaders(HttpHeaders.CONTENT_LENGTH)).containsExactly("4"); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void removingContentLengthSuppressesOriginalAndConverterLengths() throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().contentLength(111).body("body")); + adapted.headers().remove(HttpHeaders.CONTENT_LENGTH); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.containsHeader(HttpHeaders.CONTENT_LENGTH)).isFalse(); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void contentTypeAndCharsetChangesAreNotOverwrittenByServletPropertySetters() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().contentType(MediaType.TEXT_PLAIN).build((request, response) -> { + response.setContentType("text/plain;charset=ISO-8859-1"); + response.setCharacterEncoding("ISO-8859-1"); + response.setCharacterEncoding(StandardCharsets.US_ASCII); + response.getWriter().write("你好"); + return null; + })); + adapted.headers().setContentType(new MediaType("text", "html", StandardCharsets.UTF_8)); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.getContentType()).isEqualTo("text/html;charset=UTF-8"); + assertThat(response.getCharacterEncoding()).isEqualTo("UTF-8"); + assertThat(response.getContentAsString()).isEqualTo("你好"); + } + + @Test + void removingContentTypeSuppressesConverterDefaults() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().contentType(MediaType.TEXT_PLAIN).body("body")); + adapted.headers().remove(HttpHeaders.CONTENT_TYPE); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.getContentType()).isNull(); + assertThat(response.containsHeader(HttpHeaders.CONTENT_TYPE)).isFalse(); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void changedHeadersSuppressDateAndIntSettersButLeaveUnchangedHeadersDynamic() throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() + .header("X-Date", "original") + .header("X-Int", "original") + .build((request, response) -> { + response.addDateHeader("x-DATE", LAST_MODIFIED); + response.setDateHeader("X-Date", LAST_MODIFIED); + response.addIntHeader("x-INT", 1); + response.setIntHeader("X-Int", 2); + response.setDateHeader("X-Dynamic-Date", LAST_MODIFIED); + response.addIntHeader("X-Dynamic-Int", 3); + response.setContentType("text/plain;charset=UTF-8"); + response.setContentLength(4); + response.setCharacterEncoding(StandardCharsets.UTF_8); + return null; + })); + adapted.headers().remove("X-Date"); + adapted.headers().set("X-Int", "replacement"); + + MockHttpServletResponse response = write(adapted); + + assertThat(response.containsHeader("X-Date")).isFalse(); + assertThat(response.getHeaders("X-Int")).containsExactly("replacement"); + assertThat(response.getHeader("X-Dynamic-Date")).isEqualTo("Wed, 21 Oct 2015 07:28:00 GMT"); + assertThat(response.getHeader("X-Dynamic-Int")).isEqualTo("3"); + assertThat(response.getContentType()).isEqualTo("text/plain;charset=UTF-8"); + assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("4"); + } + + @ParameterizedTest + @ValueSource(strings = { "GET", "HEAD" }) + void changedEtagDoesNotReturnNotModifiedForTheOriginalEtag(String method) throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().eTag("original").lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); + adapted.headers().setETag("\"replacement\""); + MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"replacement\""); + assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).hasSize(1); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @ParameterizedTest + @ValueSource(strings = { "GET", "HEAD" }) + void changedEtagReturnsNotModifiedForTheEffectiveEtagAndKeepsCookies(String method) throws Exception { + Cookie cookie = new Cookie("session", "value"); + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() + .eTag("original") + .lastModified(Instant.ofEpochMilli(LAST_MODIFIED)) + .header("X-Test", "value") + .cookie(cookie) + .body("body")); + adapted.headers().setETag("\"replacement\""); + MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"replacement\""); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(304); + assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"replacement\""); + assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).hasSize(1); + assertThat(response.getHeaders("X-Test")).containsExactly("value"); + assertThat(response.getCookies()).containsExactly(cookie); + assertThat(response.getContentAsByteArray()).isEmpty(); + } + + @ParameterizedTest + @ValueSource(strings = { "GET", "HEAD" }) + void removedEtagDoesNotReturnNotModifiedForTheOriginalEtag(String method) throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); + adapted.headers().remove("etag"); + MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.containsHeader(HttpHeaders.ETAG)).isFalse(); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void changedLastModifiedDoesNotReturnNotModifiedForTheOriginalDate() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().eTag("original").lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); + adapted.headers().setLastModified(LAST_MODIFIED + 1000); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"original\""); + assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).containsExactly("Wed, 21 Oct 2015 07:28:01 GMT"); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void changedLastModifiedReturnsNotModifiedForTheEffectiveDate() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); + adapted.headers().setLastModified(LAST_MODIFIED + 1000); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:01 GMT"); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(304); + assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).containsExactly("Wed, 21 Oct 2015 07:28:01 GMT"); + assertThat(response.getContentAsByteArray()).isEmpty(); + } + + @Test + void removedLastModifiedDoesNotReturnNotModifiedForTheOriginalDate() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); + adapted.headers().remove("last-modified"); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.containsHeader(HttpHeaders.LAST_MODIFIED)).isFalse(); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void unchangedValidatorsStillUseTheOriginalConditionalHandling() throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); + adapted.headers().add("X-Test", "value"); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(304); + assertThat(response.getHeaders("X-Test")).containsExactly("value"); + assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"original\""); + assertThat(response.getContentAsByteArray()).isEmpty(); + } + + @Test + void changedValidatorsDoNotApplySafeMethodConditionalHandlingToPost() throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); + adapted.headers().setETag("\"replacement\""); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/"); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"replacement\""); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getContentAsString()).isEqualTo("body"); + } + + @Test + void conditionalRequestHeaderViewsHideOldConditionsButRetainRangeAndIfRange() throws Exception { + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().eTag("original").build((request, response) -> { + for (String name : List.of(HttpHeaders.IF_MATCH, HttpHeaders.IF_NONE_MATCH, + HttpHeaders.IF_MODIFIED_SINCE, HttpHeaders.IF_UNMODIFIED_SINCE)) { + assertThat(request.getHeader(name.toLowerCase(Locale.ROOT))).isNull(); + assertThat(Collections.list(request.getHeaders(name))).isEmpty(); + assertThat(request.getDateHeader(name)).isEqualTo(-1); + assertThat(request.getIntHeader(name)).isEqualTo(-1); + assertThat(Collections.list(request.getHeaderNames())).doesNotContain(name); + } + assertThat(request.getHeader(HttpHeaders.RANGE)).isEqualTo("bytes=0-3"); + assertThat(request.getHeader(HttpHeaders.IF_RANGE)).isEqualTo("\"original\""); + assertThat(Collections.list(request.getHeaders(HttpHeaders.IF_RANGE))).containsExactly("\"original\""); + assertThat(Collections.list(request.getHeaderNames())).contains(HttpHeaders.RANGE, + HttpHeaders.IF_RANGE); + return null; + })); + adapted.headers().remove(HttpHeaders.ETAG); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.IF_MATCH, "\"original\""); + request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); + request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); + request.addHeader(HttpHeaders.IF_UNMODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); + request.addHeader(HttpHeaders.RANGE, "bytes=0-3"); + request.addHeader(HttpHeaders.IF_RANGE, "\"original\""); + + assertThat(write(adapted, request).getStatus()).isEqualTo(200); + } + + @Test + void entityAdaptationPreservesTheEntityInterfaceAndOriginalEntity() throws Exception { + byte[] entity = "body".getBytes(StandardCharsets.UTF_8); + EntityResponse delegate = EntityResponse.fromObject(entity).build(); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + + assertThat(adapted).isInstanceOf(EntityResponse.class); + assertThat(((EntityResponse) adapted).entity()).isSameAs(entity); + adapted.headers().add("X-Test", "value"); + assertThat(write(adapted).getContentAsByteArray()).isEqualTo(entity); + } + + @Test + void renderingAdaptationPreservesTheRenderingInterfaceAndModelAndView() throws Exception { + Object modelValue = new Object(); + RenderingResponse delegate = RenderingResponse.create("template").modelAttribute("value", modelValue).build(); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + + assertThat(adapted).isInstanceOf(RenderingResponse.class); + RenderingResponse renderingResponse = (RenderingResponse) adapted; + assertThat(renderingResponse.name()).isEqualTo(delegate.name()); + assertThat(renderingResponse.model()).isSameAs(delegate.model()); + adapted.headers().add("X-Test", "value"); + MockHttpServletResponse response = new MockHttpServletResponse(); + ModelAndView result = adapted.writeTo(new MockHttpServletRequest("GET", "/"), response, this.context); + + assertThat(result).isNotNull(); + assertThat(result.getViewName()).isEqualTo("template"); + assertThat(result.getModel()).containsEntry("value", modelValue); + assertThat(response.getHeaders("X-Test")).containsExactly("value"); + } + + @Test + void arbitraryWriteFunctionsReturnTheOriginalModelAndView() throws Exception { + ModelAndView modelAndView = new ModelAndView("template"); + ServerResponse adapted = DelegatingServerResponse + .adapt(ServerResponse.ok().build((request, response) -> modelAndView)); + adapted.headers().add("X-Test", "value"); + + assertThat(adapted.writeTo(new MockHttpServletRequest("GET", "/"), new MockHttpServletResponse(), this.context)) + .isSameAs(modelAndView); + } + + @Test + void resourceRangeStatusAndDynamicHeadersRemainIntact() throws Exception { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() + .contentType(MediaType.APPLICATION_OCTET_STREAM) + .body(new ByteArrayResource("abcdef".getBytes(StandardCharsets.UTF_8)))); + adapted.headers().add("X-Test", "value"); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addHeader(HttpHeaders.RANGE, "bytes=1-3"); + + MockHttpServletResponse response = write(adapted, request); + + assertThat(response.getStatus()).isEqualTo(206); + assertThat(response.getHeader(HttpHeaders.ACCEPT_RANGES)).isEqualTo("bytes"); + assertThat(response.getHeader(HttpHeaders.CONTENT_RANGE)).isEqualTo("bytes 1-3/6"); + assertThat(response.getContentAsString()).isEqualTo("bcd"); + assertThat(response.getHeaders("X-Test")).containsExactly("value"); + } + + @Test + void unfinishedWholeAsyncResponseCanBeAdaptedAndWrittenWithoutReadingHeaders() throws Exception { + CompletableFuture future = new CompletableFuture<>(); + ServerResponse delegate = ServerResponse.async(future); + ServerResponse adapted = DelegatingServerResponse.adapt(delegate); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.setAsyncSupported(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + adapted.writeTo(request, response, this.context); + + assertThat(request.isAsyncStarted()).isTrue(); + ServerResponse completed = ServerResponse.ok().body("async"); + future.complete(completed); + assertThat(WebAsyncUtils.getAsyncManager(request).getConcurrentResult()).isSameAs(completed); + } + + @Test + void unfinishedWholeAsyncMetadataRetainsTheOriginalFailureInsteadOfBlocking() { + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.async(new CompletableFuture<>())); + + assertThatThrownBy(adapted::headers).isInstanceOf(IllegalStateException.class) + .hasMessage("Future ServerResponse has not yet completed"); + } + + @Test + void asynchronousBodyRetainsTheChangedHeaderWrapper() throws Exception { + CompletableFuture body = new CompletableFuture<>(); + ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().header("X-Test", "one").body(body)); + adapted.headers().add("X-Test", "two"); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.setAsyncSupported(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + adapted.writeTo(request, response, this.context); + body.complete("async body"); + + assertThat(response.getHeaders("X-Test")).containsExactly("one", "two"); + assertThat(response.getContentAsString()).isEqualTo("async body"); + assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("10"); + } + + private MockHttpServletResponse write(ServerResponse response) throws Exception { + return write(response, new MockHttpServletRequest("GET", "/")); + } + + private MockHttpServletResponse write(ServerResponse response, MockHttpServletRequest request) throws Exception { + MockHttpServletResponse servletResponse = new MockHttpServletResponse(); + response.writeTo(request, servletResponse, this.context); + return servletResponse; + } + +} From 79026dc40be6b0f5f7618087eaae85c4fbe8fbab Mon Sep 17 00:00:00 2001 From: joecqupt Date: Thu, 22 Oct 2026 12:05:44 +0800 Subject: [PATCH 3/3] Revert "Preserve native MVC response writing when adapting response headers" Restore the original response-builder fix while the delegation approach and its error-handling compatibility are reconsidered. This reverts commit 3c6e32e165562e9ca9597e6361d58a3dfcc2bde6. Signed-off-by: joecqupt --- .../config/RouterFunctionHolderFactory.java | 3 +- .../mvc/filter/Bucket4jFilterFunctions.java | 5 +- .../filter/CircuitBreakerFilterFunctions.java | 2 +- .../server/mvc/filter/FilterFunctions.java | 6 +- .../mvc/handler/DelegatingServerResponse.java | 332 ---------- ...unctionHandlerRequestProcessingHelper.java | 8 +- .../mvc/handler/GatewayRouterFunctions.java | 3 +- .../GatewayRouterFunctionsBuilder.java | 66 +- .../mvc/handler/GatewayServerResponse.java | 4 - .../server/mvc/handler/HandlerFunctions.java | 6 +- ...erFilterFunctionsResponseHeadersTests.java | 473 +-------------- .../DelegatingServerResponseTests.java | 569 ------------------ 12 files changed, 34 insertions(+), 1443 deletions(-) delete mode 100644 spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java delete mode 100644 spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java index a7fee09813..5646c24cbc 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/RouterFunctionHolderFactory.java @@ -48,6 +48,7 @@ import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; import org.springframework.cloud.gateway.server.mvc.filter.FilterBeanFactoryDiscoverer; import org.springframework.cloud.gateway.server.mvc.filter.FilterDiscoverer; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.cloud.gateway.server.mvc.handler.HandlerDiscoverer; import org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctionDefinition; import org.springframework.cloud.gateway.server.mvc.invoke.InvocationContext; @@ -101,7 +102,7 @@ public String toString() { }; private static final RouterFunction NEVER_ROUTE = RouterFunctions.route(neverPredicate, - request -> ServerResponse.notFound().build()); + request -> GatewayServerResponse.notFound().build()); private final Log log = LogFactory.getLog(getClass()); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java index 8df51990ed..5b5f85ef88 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/Bucket4jFilterFunctions.java @@ -32,6 +32,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.core.task.TaskExecutor; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; @@ -78,7 +79,7 @@ public static HandlerFilterFunction rateLimit( String key = config.getKeyResolver() != null ? config.getKeyResolver().apply(request) : ""; if (!StringUtils.hasText(key)) { // TODO: configurable empty key status code - return ServerResponse.status(HttpStatus.FORBIDDEN).build(); + return GatewayServerResponse.status(HttpStatus.FORBIDDEN).build(); } TaskExecutor taskExecutor = MvcUtils.getApplicationContext(request) .getBean(APPLICATION_TASK_EXECUTOR_BEAN_NAME, TaskExecutor.class); @@ -100,7 +101,7 @@ public static HandlerFilterFunction rateLimit( serverResponse.headers().add(config.getHeaderName(), String.valueOf(remainingTokens)); return serverResponse; } - return ServerResponse.status(config.getStatusCode()) + return GatewayServerResponse.status(config.getStatusCode()) .header(config.getHeaderName(), String.valueOf(remainingTokens)) .build(); }; diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java index d2f218ccf6..e001c73a37 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/CircuitBreakerFilterFunctions.java @@ -114,7 +114,7 @@ public static HandlerFilterFunction circuitBreak } // if resume without error, return ok response if (config.isResumeWithoutError()) { - return ServerResponse.ok().build(); + return GatewayServerResponse.ok().build(); } throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, throwable != null ? throwable.getMessage() : null, throwable); diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java index d21ce90102..2e5c1390b8 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/FilterFunctions.java @@ -24,6 +24,7 @@ import org.springframework.cloud.gateway.server.mvc.common.Shortcut; import org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.DedupeStrategy; import org.springframework.cloud.gateway.server.mvc.filter.BeforeFilterFunctions.FallbackHeadersConfig; +import org.springframework.cloud.gateway.server.mvc.handler.GatewayServerResponse; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatusCode; import org.springframework.util.Assert; @@ -129,8 +130,9 @@ static HandlerFilterFunction redirectTo(HttpStat static HandlerFilterFunction redirectTo(HttpStatusHolder status, URI uri) { Assert.isTrue(status.is3xxRedirection(), "status must be a 3xx code, but was " + status); - return (request, - next) -> ServerResponse.status(status.resolve()).header(HttpHeaders.LOCATION, uri.toString()).build(); + return (request, next) -> GatewayServerResponse.status(status.resolve()) + .header(HttpHeaders.LOCATION, uri.toString()) + .build(); } @Shortcut diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java deleted file mode 100644 index 85435909e3..0000000000 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponse.java +++ /dev/null @@ -1,332 +0,0 @@ -/* - * Copyright 2026-present 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.gateway.server.mvc.handler; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.util.Collection; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.TreeSet; - -import jakarta.servlet.ServletException; -import jakarta.servlet.http.Cookie; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; -import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpServletResponseWrapper; -import org.jspecify.annotations.Nullable; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatusCode; -import org.springframework.util.MultiValueMap; -import org.springframework.web.context.request.ServletWebRequest; -import org.springframework.web.servlet.ModelAndView; -import org.springframework.web.servlet.function.AsyncServerResponse; -import org.springframework.web.servlet.function.EntityResponse; -import org.springframework.web.servlet.function.RenderingResponse; -import org.springframework.web.servlet.function.ServerResponse; - -class DelegatingServerResponse implements ServerResponse { - - private final ServerResponse delegate; - - private @Nullable HttpHeaders originalHeaders; - - private @Nullable HttpHeaders headers; - - private DelegatingServerResponse(ServerResponse delegate) { - this.delegate = delegate; - } - - static ServerResponse adapt(ServerResponse response) { - if (response instanceof GatewayServerResponse || response instanceof DelegatingServerResponse) { - return response; - } - if (response instanceof EntityResponse entityResponse) { - return new DelegatingEntityResponse<>(entityResponse); - } - if (response instanceof RenderingResponse renderingResponse) { - return new DelegatingRenderingResponse(renderingResponse); - } - if (response instanceof AsyncServerResponse asyncResponse) { - return new DelegatingAsyncServerResponse(asyncResponse); - } - return new DelegatingServerResponse(response); - } - - ServerResponse delegate() { - return this.delegate; - } - - @Override - public HttpStatusCode statusCode() { - return this.delegate.statusCode(); - } - - @Override - public HttpHeaders headers() { - if (this.headers == null) { - // An unfinished AsyncServerResponse cannot expose its metadata yet. - this.originalHeaders = HttpHeaders.copyOf(this.delegate.headers()); - this.headers = HttpHeaders.copyOf(this.originalHeaders); - } - return this.headers; - } - - @Override - public MultiValueMap cookies() { - return this.delegate.cookies(); - } - - @Override - public @Nullable ModelAndView writeTo(HttpServletRequest request, HttpServletResponse response, Context context) - throws ServletException, IOException { - Set changedNames = changedNames(); - if (changedNames.isEmpty()) { - return this.delegate.writeTo(request, response, context); - } - HttpHeaders effectiveHeaders = headers(); - for (String name : changedNames) { - List values = effectiveHeaders.getOrEmpty(name); - response.setHeader(name, values.isEmpty() ? null : values.get(0)); - for (int i = 1; i < values.size(); i++) { - response.addHeader(name, values.get(i)); - } - } - if ((changedNames.contains(HttpHeaders.ETAG) || changedNames.contains(HttpHeaders.LAST_MODIFIED)) - && ("GET".equals(request.getMethod()) || "HEAD".equals(request.getMethod()))) { - response.setStatus(statusCode().value()); - // Avoid writing validators twice when the delegate replays its headers. - HttpServletResponse conditionalResponse = new ChangedHeadersResponseWrapper(response, - Set.of(HttpHeaders.ETAG, HttpHeaders.LAST_MODIFIED)); - if (new ServletWebRequest(request, conditionalResponse).checkNotModified(effectiveHeaders.getETag(), - effectiveHeaders.getLastModified())) { - effectiveHeaders.forEach((name, values) -> { - if (!changedNames.contains(name)) { - values.forEach(value -> response.addHeader(name, value)); - } - }); - cookies().values().stream().flatMap(Collection::stream).forEach(response::addCookie); - return null; - } - request = new ConditionalHeadersRequestWrapper(request); - } - return this.delegate.writeTo(request, new ChangedHeadersResponseWrapper(response, changedNames), context); - } - - private Set changedNames() { - Set names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - if (this.headers != null && this.originalHeaders != null) { - names.addAll(this.originalHeaders.headerNames()); - names.addAll(this.headers.headerNames()); - names.removeIf(name -> Objects.equals(this.originalHeaders.get(name), this.headers.get(name))); - } - return names; - } - - private static final class ChangedHeadersResponseWrapper extends HttpServletResponseWrapper { - - private final Set changedNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - - private ChangedHeadersResponseWrapper(HttpServletResponse response, Set changedNames) { - super(response); - this.changedNames.addAll(changedNames); - } - - @Override - public void addHeader(String name, @Nullable String value) { - if (!this.changedNames.contains(name)) { - super.addHeader(name, value); - } - } - - @Override - public void setHeader(String name, @Nullable String value) { - if (!this.changedNames.contains(name)) { - super.setHeader(name, value); - } - } - - @Override - public void addDateHeader(String name, long date) { - if (!this.changedNames.contains(name)) { - super.addDateHeader(name, date); - } - } - - @Override - public void setDateHeader(String name, long date) { - if (!this.changedNames.contains(name)) { - super.setDateHeader(name, date); - } - } - - @Override - public void addIntHeader(String name, int value) { - if (!this.changedNames.contains(name)) { - super.addIntHeader(name, value); - } - } - - @Override - public void setIntHeader(String name, int value) { - if (!this.changedNames.contains(name)) { - super.setIntHeader(name, value); - } - } - - @Override - public void setContentType(@Nullable String type) { - if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { - super.setContentType(type); - } - } - - @Override - public void setContentLength(int length) { - if (!this.changedNames.contains(HttpHeaders.CONTENT_LENGTH)) { - super.setContentLength(length); - } - } - - @Override - public void setContentLengthLong(long length) { - if (!this.changedNames.contains(HttpHeaders.CONTENT_LENGTH)) { - super.setContentLengthLong(length); - } - } - - @Override - public void setCharacterEncoding(@Nullable String charset) { - // Servlet charset setters also modify Content-Type. - if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { - super.setCharacterEncoding(charset); - } - } - - @Override - public void setCharacterEncoding(Charset charset) { - if (!this.changedNames.contains(HttpHeaders.CONTENT_TYPE)) { - super.setCharacterEncoding(charset); - } - } - - } - - private static final class ConditionalHeadersRequestWrapper extends HttpServletRequestWrapper { - - private static final Set CONDITIONAL_HEADERS = Set.of(HttpHeaders.IF_MATCH, HttpHeaders.IF_NONE_MATCH, - HttpHeaders.IF_MODIFIED_SINCE, HttpHeaders.IF_UNMODIFIED_SINCE); - - private ConditionalHeadersRequestWrapper(HttpServletRequest request) { - super(request); - } - - private boolean isConditional(String name) { - return CONDITIONAL_HEADERS.stream().anyMatch(name::equalsIgnoreCase); - } - - @Override - public @Nullable String getHeader(String name) { - return isConditional(name) ? null : super.getHeader(name); - } - - @Override - public Enumeration getHeaders(String name) { - return isConditional(name) ? Collections.emptyEnumeration() : super.getHeaders(name); - } - - @Override - public Enumeration getHeaderNames() { - return Collections.enumeration( - Collections.list(super.getHeaderNames()).stream().filter(name -> !isConditional(name)).toList()); - } - - @Override - public long getDateHeader(String name) { - return isConditional(name) ? -1 : super.getDateHeader(name); - } - - @Override - public int getIntHeader(String name) { - return isConditional(name) ? -1 : super.getIntHeader(name); - } - - } - - private static final class DelegatingAsyncServerResponse extends DelegatingServerResponse - implements AsyncServerResponse { - - private final AsyncServerResponse delegate; - - private DelegatingAsyncServerResponse(AsyncServerResponse delegate) { - super(delegate); - this.delegate = delegate; - } - - @Override - public ServerResponse block() { - return this.delegate.block(); - } - - } - - private static final class DelegatingEntityResponse extends DelegatingServerResponse - implements EntityResponse { - - private final EntityResponse delegate; - - private DelegatingEntityResponse(EntityResponse delegate) { - super(delegate); - this.delegate = delegate; - } - - @Override - public T entity() { - return this.delegate.entity(); - } - - } - - private static final class DelegatingRenderingResponse extends DelegatingServerResponse - implements RenderingResponse { - - private final RenderingResponse delegate; - - private DelegatingRenderingResponse(RenderingResponse delegate) { - super(delegate); - this.delegate = delegate; - } - - @Override - public String name() { - return this.delegate.name(); - } - - @Override - public Map model() { - return this.delegate.model(); - } - - } - -} diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java index fd48f6c031..40b0e1c5ff 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/FunctionHandlerRequestProcessingHelper.java @@ -68,7 +68,7 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } if (function == null) { - return ServerResponse.notFound().build(); + return GatewayServerResponse.notFound().build(); } HttpHeaders headers = request.headers().asHttpHeaders(); @@ -95,13 +95,13 @@ static ServerResponse processRequest(ServerRequest request, @Nullable FunctionIn } Object result = function.apply(inputMessage); if (function.isConsumer()) { - return HttpMethod.DELETE.equals(request.method()) ? ServerResponse.ok().build() - : ServerResponse.accepted() + return HttpMethod.DELETE.equals(request.method()) ? GatewayServerResponse.ok().build() + : GatewayServerResponse.accepted() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))) .build(); } - BodyBuilder responseOkBuilder = ServerResponse.ok() + BodyBuilder responseOkBuilder = GatewayServerResponse.ok() .headers(h -> h.addAll(sanitize(headers, ignoredHeaders, requestOnlyHeaders))); if (result instanceof Iterable i) { List aggregatedResult = (List) StreamSupport.stream(i.spliterator(), false).map(m -> { diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java index d8509d0f0c..ad0c82334f 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctions.java @@ -35,7 +35,8 @@ public static Builder route() { } public static Builder route(String routeId) { - return new GatewayRouterFunctionsBuilder(RouterFunctions.route(), routeId).before(routeId(routeId)); + Builder builder = RouterFunctions.route().before(routeId(routeId)); + return new GatewayRouterFunctionsBuilder(builder, routeId); } public static RouterFunction route(RequestPredicate predicate, diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java index 40033a3c30..6a602d25e2 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayRouterFunctionsBuilder.java @@ -16,8 +16,6 @@ package org.springframework.cloud.gateway.server.mvc.handler; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.BiConsumer; @@ -26,12 +24,10 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; -import java.util.stream.Stream; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; -import org.springframework.util.Assert; import org.springframework.web.servlet.function.HandlerFilterFunction; import org.springframework.web.servlet.function.HandlerFunction; import org.springframework.web.servlet.function.RequestPredicate; @@ -46,10 +42,6 @@ public class GatewayRouterFunctionsBuilder implements RouterFunctions.Builder { private final String routeId; - private final List> filters = new ArrayList<>(); - - private final List> errorHandlers = new ArrayList<>(); - public GatewayRouterFunctionsBuilder(RouterFunctions.Builder builder, String routeId) { this.builder = builder; this.routeId = routeId; @@ -58,12 +50,6 @@ public GatewayRouterFunctionsBuilder(RouterFunctions.Builder builder, String rou @Override public RouterFunction build() { RouterFunction routerFunction = builder.build(); - Optional> filter = Stream - .concat(this.filters.stream(), this.errorHandlers.stream()) - .reduce(HandlerFilterFunction::andThen); - if (filter.isPresent()) { - routerFunction = routerFunction.filter(filter.get()); - } RouterFunction withAttributes = routerFunction.withAttribute(MvcUtils.GATEWAY_ROUTE_ID_ATTR, this.routeId); return new GatewayDelegatingRouterFunction<>(withAttributes, this.routeId); @@ -269,15 +255,13 @@ public RouterFunctions.Builder add(RouterFunction @Override public RouterFunctions.Builder resource(RequestPredicate predicate, Resource resource) { - builder.resource(predicate, resource); - return this; + return builder.resource(predicate, resource); } @Override public RouterFunctions.Builder resource(RequestPredicate predicate, Resource resource, BiConsumer headersConsumer) { - builder.resource(predicate, resource, headersConsumer); - return this; + return builder.resource(predicate, resource, headersConsumer); } @Override @@ -315,13 +299,7 @@ public RouterFunctions.Builder nest(RequestPredicate @Override public RouterFunctions.Builder nest(RequestPredicate predicate, Consumer builderConsumer) { - Assert.notNull(builderConsumer, "BuilderConsumer must not be null"); - builder.nest(predicate, () -> { - GatewayRouterFunctionsBuilder nested = new GatewayRouterFunctionsBuilder(RouterFunctions.route(), - this.routeId); - builderConsumer.accept(nested); - return nested.build(); - }); + builder.nest(predicate, builderConsumer); return this; } @@ -334,64 +312,42 @@ public RouterFunctions.Builder path(String pattern, @Override public RouterFunctions.Builder path(String pattern, Consumer builderConsumer) { - Assert.notNull(builderConsumer, "BuilderConsumer must not be null"); - builder.path(pattern, () -> { - GatewayRouterFunctionsBuilder nested = new GatewayRouterFunctionsBuilder(RouterFunctions.route(), - this.routeId); - builderConsumer.accept(nested); - return nested.build(); - }); + builder.path(pattern, builderConsumer); return this; } @Override - @SuppressWarnings("unchecked") public RouterFunctions.Builder filter( HandlerFilterFunction filterFunction) { - Assert.notNull(filterFunction, "FilterFunction must not be null"); - HandlerFilterFunction adaptingFilter = (request, next) -> { - HandlerFunction adaptingNext = downstreamRequest -> (T) GatewayServerResponse - .adapt(next.handle(downstreamRequest)); - return GatewayServerResponse.adapt(filterFunction.filter(request, adaptingNext)); - }; - this.filters.add(adaptingFilter); + builder.filter(filterFunction); return this; } @Override public RouterFunctions.Builder before(Function requestProcessor) { - this.filters.add(HandlerFilterFunction.ofRequestProcessor(requestProcessor)); + builder.before(requestProcessor); return this; } @Override public RouterFunctions.Builder after( BiFunction responseProcessor) { - return filter(HandlerFilterFunction.ofResponseProcessor(responseProcessor)); + builder.after(responseProcessor); + return this; } @Override public RouterFunctions.Builder onError(Predicate predicate, BiFunction responseProvider) { - Assert.notNull(responseProvider, "ResponseProvider must not be null"); - HandlerFilterFunction errorFilter = HandlerFilterFunction - .ofErrorHandler(predicate, (failure, request) -> responseProvider.apply(failure, request)); - this.errorHandlers.add(0, (request, next) -> errorFilter.filter(request, downstreamRequest -> { - ServerResponse response = next.handle(downstreamRequest); - if (response instanceof DelegatingServerResponse delegatingResponse) { - // Spring registers write errors only on its own response base class. - errorFilter.filter(downstreamRequest, ignored -> delegatingResponse.delegate()); - } - return response; - })); + builder.onError(predicate, responseProvider); return this; } @Override public RouterFunctions.Builder onError(Class exceptionType, BiFunction responseProvider) { - Assert.notNull(exceptionType, "ExceptionType must not be null"); - return onError(exceptionType::isInstance, responseProvider); + builder.onError(exceptionType, responseProvider); + return this; } @Override diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java index c0607fa55c..b54add76ce 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/GatewayServerResponse.java @@ -30,10 +30,6 @@ public interface GatewayServerResponse extends ServerResponse { void setStatusCode(HttpStatusCode statusCode); - static ServerResponse adapt(ServerResponse response) { - return DelegatingServerResponse.adapt(response); - } - // Static methods /** diff --git a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java index 3bd102d583..128279d373 100644 --- a/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java +++ b/spring-cloud-gateway-server-webmvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/HandlerFunctions.java @@ -96,7 +96,7 @@ public static HandlerFunction fn(String functionName) { return processRequest(request, function, body, false, Collections.emptyList(), Collections.emptyList(), additionalRequestHeaders); } - return ServerResponse.notFound().build(); + return GatewayServerResponse.notFound().build(); }; } @@ -111,9 +111,9 @@ public static HandlerFunction stream(String bindingName) { .fromHttp(FunctionHandlerHeaderUtils.sanitize(request.headers().asHttpHeaders())); boolean send = streamOps.send(expandedBindingName, MessageBuilder.createMessage(body, messageHeaders)); if (send) { - return ServerResponse.accepted().build(); + return GatewayServerResponse.accepted().build(); } - return ServerResponse.badRequest().build(); + return GatewayServerResponse.badRequest().build(); }; } diff --git a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java index fb2c669d5d..1970897571 100644 --- a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java +++ b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/AfterFilterFunctionsResponseHeadersTests.java @@ -16,14 +16,10 @@ package org.springframework.cloud.gateway.server.mvc.filter; -import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.ArrayList; import java.util.List; -import java.util.Locale; -import java.util.concurrent.CompletableFuture; import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; @@ -35,7 +31,6 @@ import io.github.bucket4j.distributed.remote.RemoteBucketState; import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; -import reactor.core.publisher.Flux; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; @@ -47,7 +42,6 @@ import org.springframework.cloud.stream.function.StreamOperations; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.core.io.ByteArrayResource; import org.springframework.core.task.SyncTaskExecutor; import org.springframework.core.task.TaskExecutor; import org.springframework.http.HttpStatus; @@ -55,18 +49,9 @@ import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.test.web.servlet.MockMvc; -import org.springframework.test.web.servlet.MvcResult; -import org.springframework.test.web.servlet.result.MockMvcResultMatchers; -import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.servlet.DispatcherServlet; -import org.springframework.web.servlet.function.AsyncServerResponse; -import org.springframework.web.servlet.function.EntityResponse; -import org.springframework.web.servlet.function.HandlerFunction; import org.springframework.web.servlet.function.RouterFunction; -import org.springframework.web.servlet.function.RouterFunctions; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.servlet.function.ServerResponse; @@ -78,429 +63,16 @@ import static org.mockito.Mockito.mock; import static org.springframework.boot.autoconfigure.task.TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME; import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.addResponseHeader; -import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.dedupeResponseHeader; -import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.removeResponseHeader; -import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.rewriteLocationResponseHeader; -import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.rewriteResponseHeader; -import static org.springframework.cloud.gateway.server.mvc.filter.AfterFilterFunctions.setResponseHeader; import static org.springframework.cloud.gateway.server.mvc.filter.Bucket4jFilterFunctions.rateLimit; import static org.springframework.cloud.gateway.server.mvc.filter.CircuitBreakerFilterFunctions.circuitBreaker; import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.redirectTo; import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route; import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.fn; import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.stream; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.web.servlet.function.RequestPredicates.path; class AfterFilterFunctionsResponseHeadersTests { - @Test - void addResponseHeaderOnCustomHandlerRoute() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET(path("/foo"), req -> ServerResponse.ok().build()) - .after(addResponseHeader("X-Test", "TestFoo")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("")); - } - - @Test - void responseHeaderFiltersPreserveEntityBody() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", - req -> ServerResponse.ok() - .header("X-Source", "old") - .header("X-Remove", "old") - .header("X-Duplicate", "same") - .header("X-Rewrite", "old") - .body("hello")) - .after(dedupeResponseHeader("X-Duplicate")) - .after(addResponseHeader("X-Duplicate", "same")) - .after(rewriteResponseHeader("X-Rewrite", "old", "new")) - .after(setResponseHeader("X-Source", "new")) - .after(removeResponseHeader("X-Remove")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(header().stringValues("X-Source", "new")) - .andExpect(header().doesNotExist("X-Remove")) - .andExpect(header().stringValues("X-Duplicate", "same")) - .andExpect(header().string("X-Rewrite", "new")) - .andExpect(content().string("hello")); - } - - @Test - void locationResponseHeaderFilterPreservesBody() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.ok().header("Location", "https://upstream.example/foo").body("hello")) - .after(rewriteLocationResponseHeader()) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo").header("Host", "gateway.example")) - .andExpect(status().isOk()) - .andExpect(header().string("Location", "https://gateway.example/foo")) - .andExpect(content().string("hello")); - } - - @Test - void customFilterCanMutateDownstreamResponse() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.ok().body("hello")) - .filter((request, next) -> { - ServerResponse response = next.handle(request); - response.headers().add("X-Custom", "yes"); - return response; - }) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(header().string("X-Custom", "yes")) - .andExpect(content().string("hello")); - } - - @Test - void customFilterShortCircuitResponseIsAdapted() throws Exception { - RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> { - throw new AssertionError("Handler must not be called"); - }) - .after(addResponseHeader("X-Test", "TestFoo")) - .filter((request, next) -> ServerResponse.status(HttpStatus.FORBIDDEN).body("denied")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isForbidden()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("denied")); - } - - @Test - void responseReplacedByAfterFilterIsAdapted() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.ok().body("original")) - .after(addResponseHeader("X-Test", "TestFoo")) - .after((request, response) -> ServerResponse.accepted().body("replacement")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isAccepted()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("replacement")); - } - - @Test - void responseFiltersKeepTheirExecutionOrder() throws Exception { - RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.ok().build()) - .after(addResponseHeader("X-Order", "outer")) - .after(addResponseHeader("X-Order", "inner")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(header().stringValues("X-Order", "inner", "outer")); - } - - @Test - void pathNestedResponseFiltersAreAdapted() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .path("/api", - nested -> nested.GET("/foo", req -> ServerResponse.ok().body("hello")) - .after(addResponseHeader("X-Test", "TestFoo"))) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/api/foo")) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("hello")); - } - - @Test - void predicateNestedResponseFiltersAreAdapted() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .nest(path("/api"), - nested -> nested.GET("/foo", req -> ServerResponse.ok().body("hello")) - .after(addResponseHeader("X-Test", "TestFoo"))) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/api/foo")) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("hello")); - } - - @Test - void resourceFluentResponseFiltersAreAdapted() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .resource(path("/foo"), new ByteArrayResource("hello".getBytes(StandardCharsets.UTF_8))) - .after(addResponseHeader("X-Test", "TestFoo")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("hello")); - } - - @Test - void typedEntityResponseProcessorStillReceivesEntityResponse() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> EntityResponse.fromObject("hello").build()) - .after((ServerRequest request, EntityResponse response) -> { - assertThat(response.entity()).isEqualTo("hello"); - response.headers().add("X-Typed", "yes"); - return response; - }) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(header().string("X-Typed", "yes")) - .andExpect(content().string("hello")); - } - - @Test - void responseHeaderFilterPreservesCustomBodyWriter() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.ok().header("X-Source", "old").build((request, response) -> { - response.getWriter().write("custom"); - response.flushBuffer(); - return null; - })) - .after(setResponseHeader("X-Source", "new")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(header().stringValues("X-Source", "new")) - .andExpect(content().string("custom")); - } - - @Test - void typedAsyncResponseFilterStillReceivesAsyncServerResponse() throws Exception { - ServerResponse completed = ServerResponse.ok().body("hello"); - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.async(CompletableFuture.completedFuture(completed))) - .filter((ServerRequest request, HandlerFunction next) -> { - AsyncServerResponse response = next.handle(request); - assertThat(response.block()).isSameAs(completed); - return response; - }) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/foo")) - .andExpect(status().isOk()) - .andExpect(content().string("hello")); - } - - @Test - void outerErrorHandlerStillHandlesNestedResponseWriteFailure() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .path("/api", nested -> nested.GET("/foo", req -> ServerResponse.ok().build((request, response) -> { - throw new IOException("write failed"); - })).filter((request, next) -> next.handle(request))) - .onError(IOException.class, - (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("fallback")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/api/foo")) - .andExpect(status().isBadGateway()) - .andExpect(content().string("fallback")); - } - - @Test - void nestedResponseHeaderMutationRetainsOuterWriteErrorHandling() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .path("/api", nested -> nested.GET("/foo", req -> ServerResponse.ok().build((request, response) -> { - throw new IOException("write failed"); - })).after(addResponseHeader("X-Test", "TestFoo"))) - .onError(IOException.class, - (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY) - .header("X-Fallback", "yes") - .body("fallback")) - .build(); - - MockMvcBuilders.routerFunctions(fooRoute) - .build() - .perform(get("/api/foo")) - .andExpect(status().isBadGateway()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(header().string("X-Fallback", "yes")) - .andExpect(content().string("fallback")); - } - - @Test - void firstRegisteredErrorHandlerKeepsPrecedenceForHandlerAndWriteFailures() throws Exception { - RouterFunction fooRoute = route("fooRoute").GET("/handler", req -> { - throw new IOException("handler failed"); - }).path("/api", nested -> nested.GET("/writer", req -> ServerResponse.ok().build((request, response) -> { - throw new IOException("write failed"); - })).filter((request, next) -> next.handle(request))) - .onError(IOException.class, - (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("first")) - .onError(IOException.class, - (failure, request) -> ServerResponse.status(HttpStatus.SERVICE_UNAVAILABLE).body("second")) - .after(addResponseHeader("X-Test", "TestFoo")) - .build(); - MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); - - for (String path : List.of("/handler", "/api/writer")) { - mockMvc.perform(get(path)) - .andExpect(status().isBadGateway()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("first")); - } - } - - @Test - void requestProcessorsKeepTheirOrderRelativeToFilters() throws Exception { - List calls = new ArrayList<>(); - RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> { - calls.add("handler"); - return ServerResponse.ok().build(); - }).before(request -> { - calls.add("beforeOuter"); - return request; - }).filter((request, next) -> { - calls.add("filterBefore"); - ServerResponse response = next.handle(request); - calls.add("filterAfter"); - return response; - }).before(request -> { - calls.add("beforeInner"); - return request; - }).build(); - - MockMvcBuilders.routerFunctions(fooRoute).build().perform(get("/foo")).andExpect(status().isOk()); - - assertThat(calls).containsExactly("beforeOuter", "filterBefore", "beforeInner", "handler", "filterAfter"); - } - - @Test - void buildingAgainDoesNotChangePreviouslyBuiltFilterChains() throws Exception { - RouterFunctions.Builder builder = route("fooRoute").GET("/foo", req -> ServerResponse.ok().build()) - .after(addResponseHeader("X-Test", "first")); - RouterFunction firstRoute = builder.build(); - RouterFunction secondRoute = builder.after(addResponseHeader("X-Test", "second")).build(); - - MockMvcBuilders.routerFunctions(firstRoute) - .build() - .perform(get("/foo")) - .andExpect(header().stringValues("X-Test", "first")); - MockMvcBuilders.routerFunctions(secondRoute) - .build() - .perform(get("/foo")) - .andExpect(header().stringValues("X-Test", "second", "first")); - } - - @Test - void outerErrorHandlerStillHandlesNestedAsyncResponseFailure() throws Exception { - CompletableFuture response = new CompletableFuture<>(); - RouterFunction fooRoute = route("fooRoute") - .path("/api", - nested -> nested.GET("/foo", req -> ServerResponse.async(response)) - .filter((request, next) -> next.handle(request))) - .onError(IOException.class, - (failure, request) -> ServerResponse.status(HttpStatus.BAD_GATEWAY).body("fallback")) - .build(); - MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); - - MvcResult result = mockMvc.perform(get("/api/foo")) - .andExpect(MockMvcResultMatchers.request().asyncStarted()) - .andReturn(); - response.completeExceptionally(new IOException("async failed")); - result.getAsyncResult(5000); - mockMvc.perform(asyncDispatch(result)) - .andExpect(status().isBadGateway()) - .andExpect(content().string("fallback")); - } - - @Test - void filterDoesNotReadUnresolvedAsyncResponseMetadata() throws Exception { - CompletableFuture response = new CompletableFuture<>(); - RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.async(response)) - .filter((request, next) -> next.handle(request)) - .build(); - MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); - - MvcResult result = mockMvc.perform(get("/foo")) - .andExpect(MockMvcResultMatchers.request().asyncStarted()) - .andReturn(); - response.complete(ServerResponse.ok().body("hello")); - result.getAsyncResult(5000); - mockMvc.perform(asyncDispatch(result)).andExpect(status().isOk()).andExpect(content().string("hello")); - } - - @Test - void responseHeaderFilterPreservesPublisherBody() throws Exception { - RouterFunction fooRoute = route("fooRoute") - .GET("/foo", req -> ServerResponse.ok().body(Flux.just("hello", "world"))) - .after(addResponseHeader("X-Test", "TestFoo")) - .build(); - MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); - - MvcResult result = mockMvc.perform(get("/foo")) - .andExpect(MockMvcResultMatchers.request().asyncStarted()) - .andReturn(); - result.getAsyncResult(5000); - mockMvc.perform(asyncDispatch(result)) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("helloworld")); - } - - @Test - void responseHeaderFilterPreservesCompletionStageBody() throws Exception { - CompletableFuture body = new CompletableFuture<>(); - RouterFunction fooRoute = route("fooRoute").GET("/foo", req -> ServerResponse.ok().body(body)) - .after(addResponseHeader("X-Test", "TestFoo")) - .build(); - MockMvc mockMvc = MockMvcBuilders.routerFunctions(fooRoute).build(); - - MvcResult result = mockMvc.perform(get("/foo")) - .andExpect(MockMvcResultMatchers.request().asyncStarted()) - .andReturn(); - body.complete("hello"); - result.getAsyncResult(5000); - mockMvc.perform(asyncDispatch(result)) - .andExpect(status().isOk()) - .andExpect(header().string("X-Test", "TestFoo")) - .andExpect(content().string("hello")); - } - @Test void addResponseHeaderOnServerResponseBuild() { assertThatThrownBy(() -> apply(addResponseHeader("X-Test", "TestFoo"), ServerResponse.ok().build())) @@ -517,7 +89,7 @@ void addResponseHeaderOnGatewayServerResponseBuild() { @Test void addResponseHeaderAfterRedirectToFilter() throws Exception { RouterFunction redirectRoute = route("redirectRoute") - .GET(path("/redirect"), req -> ServerResponse.ok().build()) + .GET(path("/redirect"), req -> GatewayServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRedirect")) .filter(redirectTo(HttpStatus.FOUND, URI.create("https://example.org"))) .build(); @@ -546,26 +118,6 @@ void addResponseHeaderAfterFnHandler() { }); } - @Test - void addResponseHeaderAfterFnHandlerPreservesPublisherBody() { - new WebApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(ContextFunctionCatalogAutoConfiguration.class)) - .withUserConfiguration(UpperFunctionConfiguration.class) - .run(context -> { - RouterFunction fnRoute = route("fnRoute").POST(path("/fn"), fn("upperFlux")) - .after(addResponseHeader("X-Test", "TestFn")) - .build(); - ServerRequest request = request("POST", "/fn", context, "hello"); - - ServerResponse response = fnRoute.route(request).orElseThrow().handle(request); - MockHttpServletResponse servletResponse = write(response, request); - - assertThat(servletResponse.getStatus()).isEqualTo(HttpStatus.OK.value()); - assertThat(servletResponse.getHeader("X-Test")).isEqualTo("TestFn"); - assertThat(servletResponse.getContentAsString()).isEqualTo("HELLO"); - }); - } - @Test void addResponseHeaderAfterFnHandlerFunctionNotFound() { new WebApplicationContextRunner().withBean(FunctionCatalog.class, () -> mock(FunctionCatalog.class)) @@ -663,7 +215,7 @@ void addResponseHeaderAfterStreamHandler() { void addResponseHeaderAfterRateLimitRejectsEmptyKey() { rateLimitContextRunner().run(context -> { RouterFunction rateLimitRoute = route("rateLimitRoute") - .GET(path("/limited"), req -> ServerResponse.ok().build()) + .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRateLimit")) .filter(rateLimit(1, Duration.ofMinutes(1), req -> "")) .build(); @@ -679,7 +231,7 @@ void addResponseHeaderAfterRateLimitRejectsEmptyKey() { void addResponseHeaderAfterRateLimitExceeded() { rateLimitContextRunner().run(context -> { RouterFunction rateLimitRoute = route("rateLimitRoute") - .GET(path("/limited"), req -> ServerResponse.ok().build()) + .GET(path("/limited"), req -> GatewayServerResponse.ok().build()) .after(addResponseHeader("X-Test", "TestRateLimit")) .filter(rateLimit(1, Duration.ofMinutes(1), req -> "client")) .build(); @@ -729,18 +281,7 @@ private static WebApplicationContextRunner rateLimitContextRunner() { private static ServerResponse handle(RouterFunction routerFunction, ServerRequest request) throws Exception { - ServerResponse response = routerFunction.route(request).orElseThrow().handle(request); - MockHttpServletResponse servletResponse = write(response, request); - assertThat(servletResponse.getStatus()).isEqualTo(response.statusCode().value()); - response.headers() - .forEach((name, values) -> assertThat(servletResponse.getHeaders(name)).containsExactlyElementsOf(values)); - return response; - } - - private static MockHttpServletResponse write(ServerResponse response, ServerRequest request) throws Exception { - MockHttpServletResponse servletResponse = new MockHttpServletResponse(); - response.writeTo(request.servletRequest(), servletResponse, request::messageConverters); - return servletResponse; + return routerFunction.route(request).orElseThrow().handle(request); } private static ServerResponse apply(BiFunction filter, @@ -755,7 +296,6 @@ private static ServerRequest request(String method, String path, @Nullable WebAp private static ServerRequest request(String method, String path, @Nullable WebApplicationContext context, @Nullable String body) { MockHttpServletRequest servletRequest = new MockHttpServletRequest(method, path); - servletRequest.setAsyncSupported(true); if (context != null) { servletRequest.setAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE, context); } @@ -775,11 +315,6 @@ Function upper() { return String::toUpperCase; } - @Bean - Function> upperFlux() { - return value -> Flux.just(value.toUpperCase(Locale.ROOT)); - } - } @Configuration(proxyBeanMethods = false) diff --git a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java b/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java deleted file mode 100644 index 0a9cd5839e..0000000000 --- a/spring-cloud-gateway-server-webmvc/src/test/java/org/springframework/cloud/gateway/server/mvc/handler/DelegatingServerResponseTests.java +++ /dev/null @@ -1,569 +0,0 @@ -/* - * Copyright 2026-present 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.gateway.server.mvc.handler; - -import java.nio.charset.StandardCharsets; -import java.time.Instant; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Locale; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicReference; - -import jakarta.servlet.http.Cookie; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import org.springframework.core.io.ByteArrayResource; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; -import org.springframework.http.converter.ByteArrayHttpMessageConverter; -import org.springframework.http.converter.ResourceHttpMessageConverter; -import org.springframework.http.converter.ResourceRegionHttpMessageConverter; -import org.springframework.http.converter.StringHttpMessageConverter; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.web.context.request.async.WebAsyncUtils; -import org.springframework.web.servlet.ModelAndView; -import org.springframework.web.servlet.function.EntityResponse; -import org.springframework.web.servlet.function.RenderingResponse; -import org.springframework.web.servlet.function.ServerResponse; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -class DelegatingServerResponseTests { - - private static final long LAST_MODIFIED = 1445412480000L; - - private final ServerResponse.Context context = () -> List.of(new StringHttpMessageConverter(), - new ByteArrayHttpMessageConverter(), new ResourceHttpMessageConverter(), - new ResourceRegionHttpMessageConverter()); - - @Test - void adaptationIsIdempotentAndDoesNotAddGatewayStatusMutability() { - ServerResponse delegate = ServerResponse.ok().body("body"); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - ServerResponse gateway = GatewayServerResponse.ok().body("gateway"); - - assertThat(adapted).isNotInstanceOf(GatewayServerResponse.class); - assertThat(DelegatingServerResponse.adapt(adapted)).isSameAs(adapted); - assertThat(DelegatingServerResponse.adapt(gateway)).isSameAs(gateway); - } - - @Test - void unaccessedHeadersPassTheOriginalServletObjectsToTheDelegate() throws Exception { - AtomicReference writtenRequest = new AtomicReference<>(); - AtomicReference writtenResponse = new AtomicReference<>(); - ServerResponse delegate = ServerResponse.ok().build((request, response) -> { - writtenRequest.set(request); - writtenResponse.set(response); - return null; - }); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - MockHttpServletResponse response = new MockHttpServletResponse(); - - DelegatingServerResponse.adapt(delegate).writeTo(request, response, this.context); - - assertThat(writtenRequest.get()).isSameAs(request); - assertThat(writtenResponse.get()).isSameAs(response); - } - - @Test - void unchangedHeadersPassTheOriginalServletResponseToTheDelegate() throws Exception { - AtomicReference writtenResponse = new AtomicReference<>(); - ServerResponse delegate = ServerResponse.ok().header("X-Test", "original").build((request, response) -> { - writtenResponse.set(response); - return null; - }); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - adapted.headers().set("x-test", "temporary"); - adapted.headers().set("X-TEST", "original"); - MockHttpServletResponse response = write(adapted); - - assertThat(writtenResponse.get()).isSameAs(response); - assertThat(response.getHeaders("X-Test")).containsExactly("original"); - } - - @Test - void headersAreMutableDeepCopies() { - ServerResponse delegate = ServerResponse.ok().header("X-Test", "one", "two").body("body"); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - - adapted.headers().get("X-Test").remove(0); - adapted.headers().get("X-Test").add("three"); - - assertThat(adapted.headers().get("X-Test")).containsExactly("two", "three"); - assertThat(delegate.headers().get("X-Test")).containsExactly("one", "two"); - assertThat(adapted.headers()).isSameAs(adapted.headers()); - } - - @Test - void addingHeadersDoesNotDuplicateOriginalValuesAndPreservesBody() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().header("X-Test", "one").body("original body")); - adapted.headers().add("X-Test", "two"); - adapted.headers().add("X-New", "three"); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.getHeaders("X-Test")).containsExactly("one", "two"); - assertThat(response.getHeaders("X-New")).containsExactly("three"); - assertThat(response.getContentAsString()).isEqualTo("original body"); - assertThat(response.getContentType()).startsWith("text/plain"); - assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("13"); - } - - @Test - void settingHeadersReplacesAllOriginalValuesIgnoringCase() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().header("X-Test", "one", "two").body("body")); - adapted.headers().set("x-TEST", "replacement"); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.getHeaders("X-Test")).containsExactly("replacement"); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void removingHeadersPreventsTheDelegateFromRestoringThem() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().header("X-Test", "one").body("body")); - adapted.headers().remove("x-test"); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.containsHeader("X-Test")).isFalse(); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void deduplicatingHeadersDoesNotReintroduceOriginalDuplicates() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().header("X-Test", "one", "one", "two").body("body")); - adapted.headers().put("x-test", List.copyOf(new LinkedHashSet<>(adapted.headers().get("X-Test")))); - - assertThat(write(adapted).getHeaders("X-Test")).containsExactly("one", "two"); - } - - @Test - void changedHeadersReplaceValuesAlreadyPresentOnTheServletResponse() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().header("X-Test", "original").header("X-Remove", "original").body("body")); - adapted.headers().set("X-Test", "replacement"); - adapted.headers().remove("X-Remove"); - MockHttpServletResponse response = new MockHttpServletResponse(); - response.addHeader("X-Test", "earlier"); - response.addHeader("X-Remove", "earlier"); - - adapted.writeTo(new MockHttpServletRequest("GET", "/"), response, this.context); - - assertThat(response.getHeaders("X-Test")).containsExactly("replacement"); - assertThat(response.containsHeader("X-Remove")).isFalse(); - } - - @Test - void cookiesAndStatusAreDelegatedRatherThanFrozen() throws Exception { - Cookie originalCookie = new Cookie("original", "one"); - Cookie dynamicCookie = new Cookie("dynamic", "two"); - ServerResponse delegate = ServerResponse.accepted().cookie(originalCookie).build((request, response) -> { - response.setStatus(HttpStatus.PARTIAL_CONTENT.value()); - response.addCookie(dynamicCookie); - return null; - }); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - adapted.headers().add("X-Test", "value"); - - MockHttpServletResponse response = write(adapted); - - assertThat(adapted.statusCode()).isSameAs(delegate.statusCode()); - assertThat(adapted.cookies()).isSameAs(delegate.cookies()); - assertThat(response.getStatus()).isEqualTo(HttpStatus.PARTIAL_CONTENT.value()); - assertThat(response.getCookies()).containsExactly(originalCookie, dynamicCookie); - } - - @Test - void contentLengthChangesSuppressBothServletPropertySetters() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().contentLength(111).build((request, response) -> { - response.setContentLength(222); - response.setContentLengthLong(333); - response.getOutputStream().write("body".getBytes(StandardCharsets.UTF_8)); - return null; - })); - adapted.headers().setContentLength(4); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.getHeaders(HttpHeaders.CONTENT_LENGTH)).containsExactly("4"); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void removingContentLengthSuppressesOriginalAndConverterLengths() throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().contentLength(111).body("body")); - adapted.headers().remove(HttpHeaders.CONTENT_LENGTH); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.containsHeader(HttpHeaders.CONTENT_LENGTH)).isFalse(); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void contentTypeAndCharsetChangesAreNotOverwrittenByServletPropertySetters() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().contentType(MediaType.TEXT_PLAIN).build((request, response) -> { - response.setContentType("text/plain;charset=ISO-8859-1"); - response.setCharacterEncoding("ISO-8859-1"); - response.setCharacterEncoding(StandardCharsets.US_ASCII); - response.getWriter().write("你好"); - return null; - })); - adapted.headers().setContentType(new MediaType("text", "html", StandardCharsets.UTF_8)); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.getContentType()).isEqualTo("text/html;charset=UTF-8"); - assertThat(response.getCharacterEncoding()).isEqualTo("UTF-8"); - assertThat(response.getContentAsString()).isEqualTo("你好"); - } - - @Test - void removingContentTypeSuppressesConverterDefaults() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().contentType(MediaType.TEXT_PLAIN).body("body")); - adapted.headers().remove(HttpHeaders.CONTENT_TYPE); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.getContentType()).isNull(); - assertThat(response.containsHeader(HttpHeaders.CONTENT_TYPE)).isFalse(); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void changedHeadersSuppressDateAndIntSettersButLeaveUnchangedHeadersDynamic() throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() - .header("X-Date", "original") - .header("X-Int", "original") - .build((request, response) -> { - response.addDateHeader("x-DATE", LAST_MODIFIED); - response.setDateHeader("X-Date", LAST_MODIFIED); - response.addIntHeader("x-INT", 1); - response.setIntHeader("X-Int", 2); - response.setDateHeader("X-Dynamic-Date", LAST_MODIFIED); - response.addIntHeader("X-Dynamic-Int", 3); - response.setContentType("text/plain;charset=UTF-8"); - response.setContentLength(4); - response.setCharacterEncoding(StandardCharsets.UTF_8); - return null; - })); - adapted.headers().remove("X-Date"); - adapted.headers().set("X-Int", "replacement"); - - MockHttpServletResponse response = write(adapted); - - assertThat(response.containsHeader("X-Date")).isFalse(); - assertThat(response.getHeaders("X-Int")).containsExactly("replacement"); - assertThat(response.getHeader("X-Dynamic-Date")).isEqualTo("Wed, 21 Oct 2015 07:28:00 GMT"); - assertThat(response.getHeader("X-Dynamic-Int")).isEqualTo("3"); - assertThat(response.getContentType()).isEqualTo("text/plain;charset=UTF-8"); - assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("4"); - } - - @ParameterizedTest - @ValueSource(strings = { "GET", "HEAD" }) - void changedEtagDoesNotReturnNotModifiedForTheOriginalEtag(String method) throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().eTag("original").lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); - adapted.headers().setETag("\"replacement\""); - MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"replacement\""); - assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).hasSize(1); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @ParameterizedTest - @ValueSource(strings = { "GET", "HEAD" }) - void changedEtagReturnsNotModifiedForTheEffectiveEtagAndKeepsCookies(String method) throws Exception { - Cookie cookie = new Cookie("session", "value"); - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() - .eTag("original") - .lastModified(Instant.ofEpochMilli(LAST_MODIFIED)) - .header("X-Test", "value") - .cookie(cookie) - .body("body")); - adapted.headers().setETag("\"replacement\""); - MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"replacement\""); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(304); - assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"replacement\""); - assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).hasSize(1); - assertThat(response.getHeaders("X-Test")).containsExactly("value"); - assertThat(response.getCookies()).containsExactly(cookie); - assertThat(response.getContentAsByteArray()).isEmpty(); - } - - @ParameterizedTest - @ValueSource(strings = { "GET", "HEAD" }) - void removedEtagDoesNotReturnNotModifiedForTheOriginalEtag(String method) throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); - adapted.headers().remove("etag"); - MockHttpServletRequest request = new MockHttpServletRequest(method, "/"); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.containsHeader(HttpHeaders.ETAG)).isFalse(); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void changedLastModifiedDoesNotReturnNotModifiedForTheOriginalDate() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().eTag("original").lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); - adapted.headers().setLastModified(LAST_MODIFIED + 1000); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"original\""); - assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).containsExactly("Wed, 21 Oct 2015 07:28:01 GMT"); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void changedLastModifiedReturnsNotModifiedForTheEffectiveDate() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); - adapted.headers().setLastModified(LAST_MODIFIED + 1000); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:01 GMT"); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(304); - assertThat(response.getHeaders(HttpHeaders.LAST_MODIFIED)).containsExactly("Wed, 21 Oct 2015 07:28:01 GMT"); - assertThat(response.getContentAsByteArray()).isEmpty(); - } - - @Test - void removedLastModifiedDoesNotReturnNotModifiedForTheOriginalDate() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().lastModified(Instant.ofEpochMilli(LAST_MODIFIED)).body("body")); - adapted.headers().remove("last-modified"); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.containsHeader(HttpHeaders.LAST_MODIFIED)).isFalse(); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void unchangedValidatorsStillUseTheOriginalConditionalHandling() throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); - adapted.headers().add("X-Test", "value"); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(304); - assertThat(response.getHeaders("X-Test")).containsExactly("value"); - assertThat(response.getHeaders(HttpHeaders.ETAG)).containsExactly("\"original\""); - assertThat(response.getContentAsByteArray()).isEmpty(); - } - - @Test - void changedValidatorsDoNotApplySafeMethodConditionalHandlingToPost() throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().eTag("original").body("body")); - adapted.headers().setETag("\"replacement\""); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/"); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"replacement\""); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.getContentAsString()).isEqualTo("body"); - } - - @Test - void conditionalRequestHeaderViewsHideOldConditionsButRetainRangeAndIfRange() throws Exception { - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().eTag("original").build((request, response) -> { - for (String name : List.of(HttpHeaders.IF_MATCH, HttpHeaders.IF_NONE_MATCH, - HttpHeaders.IF_MODIFIED_SINCE, HttpHeaders.IF_UNMODIFIED_SINCE)) { - assertThat(request.getHeader(name.toLowerCase(Locale.ROOT))).isNull(); - assertThat(Collections.list(request.getHeaders(name))).isEmpty(); - assertThat(request.getDateHeader(name)).isEqualTo(-1); - assertThat(request.getIntHeader(name)).isEqualTo(-1); - assertThat(Collections.list(request.getHeaderNames())).doesNotContain(name); - } - assertThat(request.getHeader(HttpHeaders.RANGE)).isEqualTo("bytes=0-3"); - assertThat(request.getHeader(HttpHeaders.IF_RANGE)).isEqualTo("\"original\""); - assertThat(Collections.list(request.getHeaders(HttpHeaders.IF_RANGE))).containsExactly("\"original\""); - assertThat(Collections.list(request.getHeaderNames())).contains(HttpHeaders.RANGE, - HttpHeaders.IF_RANGE); - return null; - })); - adapted.headers().remove(HttpHeaders.ETAG); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.IF_MATCH, "\"original\""); - request.addHeader(HttpHeaders.IF_NONE_MATCH, "\"original\""); - request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); - request.addHeader(HttpHeaders.IF_UNMODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"); - request.addHeader(HttpHeaders.RANGE, "bytes=0-3"); - request.addHeader(HttpHeaders.IF_RANGE, "\"original\""); - - assertThat(write(adapted, request).getStatus()).isEqualTo(200); - } - - @Test - void entityAdaptationPreservesTheEntityInterfaceAndOriginalEntity() throws Exception { - byte[] entity = "body".getBytes(StandardCharsets.UTF_8); - EntityResponse delegate = EntityResponse.fromObject(entity).build(); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - - assertThat(adapted).isInstanceOf(EntityResponse.class); - assertThat(((EntityResponse) adapted).entity()).isSameAs(entity); - adapted.headers().add("X-Test", "value"); - assertThat(write(adapted).getContentAsByteArray()).isEqualTo(entity); - } - - @Test - void renderingAdaptationPreservesTheRenderingInterfaceAndModelAndView() throws Exception { - Object modelValue = new Object(); - RenderingResponse delegate = RenderingResponse.create("template").modelAttribute("value", modelValue).build(); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - - assertThat(adapted).isInstanceOf(RenderingResponse.class); - RenderingResponse renderingResponse = (RenderingResponse) adapted; - assertThat(renderingResponse.name()).isEqualTo(delegate.name()); - assertThat(renderingResponse.model()).isSameAs(delegate.model()); - adapted.headers().add("X-Test", "value"); - MockHttpServletResponse response = new MockHttpServletResponse(); - ModelAndView result = adapted.writeTo(new MockHttpServletRequest("GET", "/"), response, this.context); - - assertThat(result).isNotNull(); - assertThat(result.getViewName()).isEqualTo("template"); - assertThat(result.getModel()).containsEntry("value", modelValue); - assertThat(response.getHeaders("X-Test")).containsExactly("value"); - } - - @Test - void arbitraryWriteFunctionsReturnTheOriginalModelAndView() throws Exception { - ModelAndView modelAndView = new ModelAndView("template"); - ServerResponse adapted = DelegatingServerResponse - .adapt(ServerResponse.ok().build((request, response) -> modelAndView)); - adapted.headers().add("X-Test", "value"); - - assertThat(adapted.writeTo(new MockHttpServletRequest("GET", "/"), new MockHttpServletResponse(), this.context)) - .isSameAs(modelAndView); - } - - @Test - void resourceRangeStatusAndDynamicHeadersRemainIntact() throws Exception { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok() - .contentType(MediaType.APPLICATION_OCTET_STREAM) - .body(new ByteArrayResource("abcdef".getBytes(StandardCharsets.UTF_8)))); - adapted.headers().add("X-Test", "value"); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.addHeader(HttpHeaders.RANGE, "bytes=1-3"); - - MockHttpServletResponse response = write(adapted, request); - - assertThat(response.getStatus()).isEqualTo(206); - assertThat(response.getHeader(HttpHeaders.ACCEPT_RANGES)).isEqualTo("bytes"); - assertThat(response.getHeader(HttpHeaders.CONTENT_RANGE)).isEqualTo("bytes 1-3/6"); - assertThat(response.getContentAsString()).isEqualTo("bcd"); - assertThat(response.getHeaders("X-Test")).containsExactly("value"); - } - - @Test - void unfinishedWholeAsyncResponseCanBeAdaptedAndWrittenWithoutReadingHeaders() throws Exception { - CompletableFuture future = new CompletableFuture<>(); - ServerResponse delegate = ServerResponse.async(future); - ServerResponse adapted = DelegatingServerResponse.adapt(delegate); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.setAsyncSupported(true); - MockHttpServletResponse response = new MockHttpServletResponse(); - - adapted.writeTo(request, response, this.context); - - assertThat(request.isAsyncStarted()).isTrue(); - ServerResponse completed = ServerResponse.ok().body("async"); - future.complete(completed); - assertThat(WebAsyncUtils.getAsyncManager(request).getConcurrentResult()).isSameAs(completed); - } - - @Test - void unfinishedWholeAsyncMetadataRetainsTheOriginalFailureInsteadOfBlocking() { - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.async(new CompletableFuture<>())); - - assertThatThrownBy(adapted::headers).isInstanceOf(IllegalStateException.class) - .hasMessage("Future ServerResponse has not yet completed"); - } - - @Test - void asynchronousBodyRetainsTheChangedHeaderWrapper() throws Exception { - CompletableFuture body = new CompletableFuture<>(); - ServerResponse adapted = DelegatingServerResponse.adapt(ServerResponse.ok().header("X-Test", "one").body(body)); - adapted.headers().add("X-Test", "two"); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.setAsyncSupported(true); - MockHttpServletResponse response = new MockHttpServletResponse(); - - adapted.writeTo(request, response, this.context); - body.complete("async body"); - - assertThat(response.getHeaders("X-Test")).containsExactly("one", "two"); - assertThat(response.getContentAsString()).isEqualTo("async body"); - assertThat(response.getHeader(HttpHeaders.CONTENT_LENGTH)).isEqualTo("10"); - } - - private MockHttpServletResponse write(ServerResponse response) throws Exception { - return write(response, new MockHttpServletRequest("GET", "/")); - } - - private MockHttpServletResponse write(ServerResponse response, MockHttpServletRequest request) throws Exception { - MockHttpServletResponse servletResponse = new MockHttpServletResponse(); - response.writeTo(request, servletResponse, this.context); - return servletResponse; - } - -}