From 7add5243b9db13a9f8e765c8ab8545c8e8fe606b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Sun, 3 May 2026 11:11:40 +0200 Subject: [PATCH] Prevent special prefixes in default view name resolution This commit updates the default view name generation logic in both Spring WebMVC and Spring WebFlux to prevent "redirect:" and "forward:" (for MVC) prefixes from the incoming request path. Closes gh-36793 --- .../view/ViewResolutionResultHandler.java | 116 ++++++++++-------- .../ViewResolutionResultHandlerTests.java | 15 +++ .../DefaultRequestToViewNameTranslator.java | 15 ++- ...faultRequestToViewNameTranslatorTests.java | 20 +++ 4 files changed, 112 insertions(+), 54 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index d4fc2e86c57..5147d2c59c6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -44,6 +44,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.MediaType; import org.springframework.http.codec.ServerSentEvent; @@ -61,6 +62,7 @@ import org.springframework.web.reactive.HandlerResultHandler; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.result.HandlerResultHandlerSupport; import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; /** @@ -89,6 +91,7 @@ import org.springframework.web.server.ServerWebExchange; * presence of annotations, for example, for {@code @ResponseBody}. * * @author Rossen Stoyanchev + * @author Sebastien Deleuze * @since 5.0 */ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport implements HandlerResultHandler, Ordered { @@ -256,65 +259,70 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp clazz = FragmentsRendering.class; } - if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) { - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { - viewsMono = resolveViews(returnValue.toString(), locale); - } - else if (Rendering.class.isAssignableFrom(clazz)) { - Rendering render = (Rendering) returnValue; - HttpStatusCode status = render.status(); - if (status != null) { - exchange.getResponse().setStatusCode(status); + try { + if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) { + viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - exchange.getResponse().getHeaders().putAll(render.headers()); - model.addAllAttributes(render.modelAttributes()); - Object view = render.view(); - if (view == null) { - view = getDefaultViewName(exchange); + else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { + viewsMono = resolveViews(returnValue.toString(), locale); } - viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) : - Mono.just(Collections.singletonList((View) view))); - } - else if (FragmentsRendering.class.isAssignableFrom(clazz)) { - ServerHttpResponse response = exchange.getResponse(); - FragmentsRendering render = (FragmentsRendering) returnValue; - HttpStatusCode status = render.status(); - if (status != null) { - response.setStatusCode(status); + else if (Rendering.class.isAssignableFrom(clazz)) { + Rendering render = (Rendering) returnValue; + HttpStatusCode status = render.status(); + if (status != null) { + exchange.getResponse().setStatusCode(status); + } + exchange.getResponse().getHeaders().putAll(render.headers()); + model.addAllAttributes(render.modelAttributes()); + Object view = render.view(); + if (view == null) { + view = getDefaultViewName(exchange); + } + viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) : + Mono.just(Collections.singletonList((View) view))); } - response.getHeaders().putAll(render.headers()); - bindingContext.updateModel(exchange); + else if (FragmentsRendering.class.isAssignableFrom(clazz)) { + ServerHttpResponse response = exchange.getResponse(); + FragmentsRendering render = (FragmentsRendering) returnValue; + HttpStatusCode status = render.status(); + if (status != null) { + response.setStatusCode(status); + } + response.getHeaders().putAll(render.headers()); + bindingContext.updateModel(exchange); - StreamHandler streamHandler = - (this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null); + StreamHandler streamHandler = + (this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null); - if (streamHandler != null) { - streamHandler.updateResponse(exchange); + if (streamHandler != null) { + streamHandler.updateResponse(exchange); + } + + Flux> renderFlux = render.fragments() + .concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange)) + .doOnDiscard(DataBuffer.class, DataBufferUtils::release); + + return response.writeAndFlushWith(renderFlux); + } + else if (Model.class.isAssignableFrom(clazz)) { + model.addAllAttributes(((Model) returnValue).asMap()); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { + model.addAllAttributes((Map) returnValue); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (View.class.isAssignableFrom(clazz)) { + viewsMono = Mono.just(Collections.singletonList((View) returnValue)); + } + else { + String name = getNameForReturnValue(parameter); + model.addAttribute(name, returnValue); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - - Flux> renderFlux = render.fragments() - .concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange)) - .doOnDiscard(DataBuffer.class, DataBufferUtils::release); - - return response.writeAndFlushWith(renderFlux); } - else if (Model.class.isAssignableFrom(clazz)) { - model.addAllAttributes(((Model) returnValue).asMap()); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { - model.addAllAttributes((Map) returnValue); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (View.class.isAssignableFrom(clazz)) { - viewsMono = Mono.just(Collections.singletonList((View) returnValue)); - } - else { - String name = getNameForReturnValue(parameter); - model.addAttribute(name, returnValue); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); + catch (ResponseStatusException ex) { + return Mono.error(ex); } bindingContext.updateModel(exchange); return viewsMono.flatMap(views -> render(views, model.asMap(), null, bindingContext, exchange)); @@ -324,12 +332,16 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp /** * Select a default view name when a controller did not specify it. * Use the request path the leading and trailing slash stripped. + * @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" prefix */ private String getDefaultViewName(ServerWebExchange exchange) { String path = exchange.getRequest().getPath().pathWithinApplication().value(); if (path.startsWith("/")) { path = path.substring(1); } + if (path.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix"); + } if (path.endsWith("/")) { path = path.substring(0, path.length() - 1); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 97199c88451..6e88b61ab96 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -52,6 +52,7 @@ import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.accept.HeaderContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -256,6 +257,20 @@ class ViewResolutionResultHandlerTests { assertResponseBody(exchange, "account: {id=123}"); } + @Test + void defaultViewNameWithRedirectPrefixFails() { + MethodParameter returnType = on(Handler.class).resolveReturnType(Mono.class, String.class); + HandlerResult result = new HandlerResult(new Object(), Mono.empty(), returnType, this.bindingContext); + ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account")); + + MockServerWebExchange exchange = MockServerWebExchange.from(get("/redirect:account")); + Mono mono = handler.handleResult(exchange, result); + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ResponseStatusException.class) + .verify(); + } + @Test void unresolvedViewName() { String returnValue = "account"; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java index 7d7161ce4aa..756f9da5ad6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java @@ -20,7 +20,9 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import org.jspecify.annotations.Nullable; +import org.springframework.http.HttpStatus; import org.springframework.util.StringUtils; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.RequestToViewNameTranslator; import org.springframework.web.util.ServletRequestPathUtils; @@ -50,6 +52,7 @@ import org.springframework.web.util.ServletRequestPathUtils; * * @author Rob Harrop * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 2.0 * @see org.springframework.web.servlet.RequestToViewNameTranslator * @see org.springframework.web.servlet.ViewResolver @@ -127,13 +130,21 @@ public class DefaultRequestToViewNameTranslator implements RequestToViewNameTran * into the view name based on the configured parameters. * @throws IllegalArgumentException if neither a parsed RequestPath, nor a * String lookupPath have been resolved and cached as a request attribute. + * @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" or a "forward:" prefix * @see ServletRequestPathUtils#getCachedPath(ServletRequest) * @see #transformPath */ @Override public String getViewName(HttpServletRequest request) { String path = ServletRequestPathUtils.getCachedPathValue(request); - return (this.prefix + transformPath(path) + this.suffix); + String viewName = this.prefix + transformPath(path) + this.suffix; + if (viewName.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix"); + } + if (viewName.startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'forward:' prefix"); + } + return viewName; } /** @@ -144,7 +155,7 @@ public class DefaultRequestToViewNameTranslator implements RequestToViewNameTran * @return the transformed path, with slashes and extensions stripped * if desired */ - protected @Nullable String transformPath(String lookupPath) { + protected String transformPath(String lookupPath) { String path = lookupPath; if (this.stripLeadingSlash && path.startsWith(SLASH)) { path = path.substring(1); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java index ec1d0fe4b48..a2e3c6e356d 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java @@ -21,15 +21,19 @@ import java.util.stream.Stream; import org.junit.jupiter.api.Named; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.handler.PathPatternsParameterizedTest; import org.springframework.web.servlet.handler.PathPatternsTestUtils; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Rick Evans * @author Juergen Hoeller + * @author Sebastien Deleuze */ class DefaultRequestToViewNameTranslatorTests { @@ -121,6 +125,22 @@ class DefaultRequestToViewNameTranslatorTests { assertViewName(request, VIEW_NAME); } + @PathPatternsParameterizedTest + void getViewNameWithRedirectPrefixFails(Function requestFactory) { + MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.REDIRECT_URL_PREFIX + VIEW_NAME); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> this.translator.getViewName(request)) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + + @PathPatternsParameterizedTest + void getViewNameWithForwardPrefixFails(Function requestFactory) { + MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.FORWARD_URL_PREFIX + VIEW_NAME); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> this.translator.getViewName(request)) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + private void assertViewName(MockHttpServletRequest request, String expectedViewName) { String actualViewName = this.translator.getViewName(request);