Skip to content

Commit 3aaec98

Browse files
sdeleuzebclozel
sdeleuze
authored and
bclozel
committed
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-36794
1 parent ee4e790 commit 3aaec98

4 files changed

Lines changed: 113 additions & 56 deletions

File tree

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java‎

Lines changed: 66 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.springframework.core.io.buffer.DataBufferFactory;
4444
import org.springframework.core.io.buffer.DataBufferUtils;
4545
import org.springframework.http.HttpHeaders;
46+
import org.springframework.http.HttpStatus;
4647
import org.springframework.http.HttpStatusCode;
4748
import org.springframework.http.MediaType;
4849
import org.springframework.http.codec.ServerSentEvent;
@@ -61,6 +62,7 @@
6162
import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
6263
import org.springframework.web.reactive.result.HandlerResultHandlerSupport;
6364
import org.springframework.web.server.NotAcceptableStatusException;
65+
import org.springframework.web.server.ResponseStatusException;
6466
import org.springframework.web.server.ServerWebExchange;
6567

6668
/**
@@ -89,6 +91,7 @@
8991
* presence of annotations, for example, for {@code @ResponseBody}.
9092
*
9193
* @author Rossen Stoyanchev
94+
* @author Sebastien Deleuze
9295
* @since 5.0
9396
*/
9497
public class ViewResolutionResultHandler extends HandlerResultHandlerSupport implements HandlerResultHandler, Ordered {
@@ -256,65 +259,70 @@ public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result)
256259
clazz = FragmentsRendering.class;
257260
}
258261

259-
if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) {
260-
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
261-
}
262-
else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) {
263-
viewsMono = resolveViews(returnValue.toString(), locale);
264-
}
265-
else if (Rendering.class.isAssignableFrom(clazz)) {
266-
Rendering render = (Rendering) returnValue;
267-
HttpStatusCode status = render.status();
268-
if (status != null) {
269-
exchange.getResponse().setStatusCode(status);
262+
try {
263+
if (returnValue == NO_VALUE || ClassUtils.isVoidType(clazz)) {
264+
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
270265
}
271-
exchange.getResponse().getHeaders().putAll(render.headers());
272-
model.addAllAttributes(render.modelAttributes());
273-
Object view = render.view();
274-
if (view == null) {
275-
view = getDefaultViewName(exchange);
266+
else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) {
267+
viewsMono = resolveViews(returnValue.toString(), locale);
276268
}
277-
viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) :
278-
Mono.just(Collections.singletonList((View) view)));
279-
}
280-
else if (FragmentsRendering.class.isAssignableFrom(clazz)) {
281-
ServerHttpResponse response = exchange.getResponse();
282-
FragmentsRendering render = (FragmentsRendering) returnValue;
283-
HttpStatusCode status = render.status();
284-
if (status != null) {
285-
response.setStatusCode(status);
269+
else if (Rendering.class.isAssignableFrom(clazz)) {
270+
Rendering render = (Rendering) returnValue;
271+
HttpStatusCode status = render.status();
272+
if (status != null) {
273+
exchange.getResponse().setStatusCode(status);
274+
}
275+
exchange.getResponse().getHeaders().putAll(render.headers());
276+
model.addAllAttributes(render.modelAttributes());
277+
Object view = render.view();
278+
if (view == null) {
279+
view = getDefaultViewName(exchange);
280+
}
281+
viewsMono = (view instanceof String viewName ? resolveViews(viewName, locale) :
282+
Mono.just(Collections.singletonList((View) view)));
286283
}
287-
response.getHeaders().putAll(render.headers());
288-
bindingContext.updateModel(exchange);
289-
290-
StreamHandler streamHandler =
291-
(this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null);
292-
293-
if (streamHandler != null) {
294-
streamHandler.updateResponse(exchange);
284+
else if (FragmentsRendering.class.isAssignableFrom(clazz)) {
285+
ServerHttpResponse response = exchange.getResponse();
286+
FragmentsRendering render = (FragmentsRendering) returnValue;
287+
HttpStatusCode status = render.status();
288+
if (status != null) {
289+
response.setStatusCode(status);
290+
}
291+
response.getHeaders().putAll(render.headers());
292+
bindingContext.updateModel(exchange);
293+
294+
StreamHandler streamHandler =
295+
(this.sseHandler.supports(exchange.getRequest()) ? this.sseHandler : null);
296+
297+
if (streamHandler != null) {
298+
streamHandler.updateResponse(exchange);
299+
}
300+
301+
Flux<Flux<DataBuffer>> renderFlux = render.fragments()
302+
.concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange))
303+
.doOnDiscard(DataBuffer.class, DataBufferUtils::release);
304+
305+
return response.writeAndFlushWith(renderFlux);
306+
}
307+
else if (Model.class.isAssignableFrom(clazz)) {
308+
model.addAllAttributes(((Model) returnValue).asMap());
309+
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
310+
}
311+
else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) {
312+
model.addAllAttributes((Map<String, ?>) returnValue);
313+
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
314+
}
315+
else if (View.class.isAssignableFrom(clazz)) {
316+
viewsMono = Mono.just(Collections.singletonList((View) returnValue));
317+
}
318+
else {
319+
String name = getNameForReturnValue(parameter);
320+
model.addAttribute(name, returnValue);
321+
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
295322
}
296-
297-
Flux<Flux<DataBuffer>> renderFlux = render.fragments()
298-
.concatMap(fragment -> renderFragment(fragment, null, streamHandler, locale, bindingContext, exchange))
299-
.doOnDiscard(DataBuffer.class, DataBufferUtils::release);
300-
301-
return response.writeAndFlushWith(renderFlux);
302-
}
303-
else if (Model.class.isAssignableFrom(clazz)) {
304-
model.addAllAttributes(((Model) returnValue).asMap());
305-
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
306-
}
307-
else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) {
308-
model.addAllAttributes((Map<String, ?>) returnValue);
309-
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
310-
}
311-
else if (View.class.isAssignableFrom(clazz)) {
312-
viewsMono = Mono.just(Collections.singletonList((View) returnValue));
313323
}
314-
else {
315-
String name = getNameForReturnValue(parameter);
316-
model.addAttribute(name, returnValue);
317-
viewsMono = resolveViews(getDefaultViewName(exchange), locale);
324+
catch (ResponseStatusException ex) {
325+
return Mono.error(ex);
318326
}
319327
bindingContext.updateModel(exchange);
320328
return viewsMono.flatMap(views -> render(views, model.asMap(), null, bindingContext, exchange));
@@ -324,12 +332,16 @@ else if (View.class.isAssignableFrom(clazz)) {
324332
/**
325333
* Select a default view name when a controller did not specify it.
326334
* Use the request path the leading and trailing slash stripped.
335+
* @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" prefix
327336
*/
328337
private String getDefaultViewName(ServerWebExchange exchange) {
329338
String path = exchange.getRequest().getPath().pathWithinApplication().value();
330339
if (path.startsWith("/")) {
331340
path = path.substring(1);
332341
}
342+
if (path.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) {
343+
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix");
344+
}
333345
if (path.endsWith("/")) {
334346
path = path.substring(0, path.length() - 1);
335347
}

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.springframework.web.reactive.accept.HeaderContentTypeResolver;
5353
import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
5454
import org.springframework.web.server.NotAcceptableStatusException;
55+
import org.springframework.web.server.ResponseStatusException;
5556
import org.springframework.web.server.ServerWebExchange;
5657
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse;
5758
import org.springframework.web.testfixture.server.MockServerWebExchange;
@@ -255,6 +256,20 @@ private void testDefaultViewName(Object returnValue, MethodParameter returnType)
255256
assertResponseBody(exchange, "account: {id=123}");
256257
}
257258

259+
@Test
260+
void defaultViewNameWithRedirectPrefixFails() {
261+
MethodParameter returnType = on(Handler.class).resolveReturnType(Mono.class, String.class);
262+
HandlerResult result = new HandlerResult(new Object(), Mono.empty(), returnType, this.bindingContext);
263+
ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account"));
264+
265+
MockServerWebExchange exchange = MockServerWebExchange.from(get("/redirect:account"));
266+
Mono<Void> mono = handler.handleResult(exchange, result);
267+
StepVerifier.create(mono)
268+
.expectNextCount(0)
269+
.expectError(ResponseStatusException.class)
270+
.verify();
271+
}
272+
258273
@Test
259274
void unresolvedViewName() {
260275
String returnValue = "account";

‎spring-webmvc/src/main/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslator.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import jakarta.servlet.ServletRequest;
2020
import jakarta.servlet.http.HttpServletRequest;
2121

22+
import org.springframework.http.HttpStatus;
2223
import org.springframework.lang.Nullable;
2324
import org.springframework.util.StringUtils;
25+
import org.springframework.web.server.ResponseStatusException;
2426
import org.springframework.web.servlet.RequestToViewNameTranslator;
2527
import org.springframework.web.util.ServletRequestPathUtils;
2628

@@ -50,6 +52,7 @@
5052
*
5153
* @author Rob Harrop
5254
* @author Juergen Hoeller
55+
* @author Sebastien Deleuze
5356
* @since 2.0
5457
* @see org.springframework.web.servlet.RequestToViewNameTranslator
5558
* @see org.springframework.web.servlet.ViewResolver
@@ -127,13 +130,21 @@ public void setStripExtension(boolean stripExtension) {
127130
* into the view name based on the configured parameters.
128131
* @throws IllegalArgumentException if neither a parsed RequestPath, nor a
129132
* String lookupPath have been resolved and cached as a request attribute.
133+
* @throws ResponseStatusException with a 400 error code if the path contains a "redirect:" or a "forward:" prefix
130134
* @see ServletRequestPathUtils#getCachedPath(ServletRequest)
131135
* @see #transformPath
132136
*/
133137
@Override
134138
public String getViewName(HttpServletRequest request) {
135139
String path = ServletRequestPathUtils.getCachedPathValue(request);
136-
return (this.prefix + transformPath(path) + this.suffix);
140+
String viewName = this.prefix + transformPath(path) + this.suffix;
141+
if (viewName.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) {
142+
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'redirect:' prefix");
143+
}
144+
if (viewName.startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX)) {
145+
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Rejected path '" + path + "' with 'forward:' prefix");
146+
}
147+
return viewName;
137148
}
138149

139150
/**
@@ -144,7 +155,6 @@ public String getViewName(HttpServletRequest request) {
144155
* @return the transformed path, with slashes and extensions stripped
145156
* if desired
146157
*/
147-
@Nullable
148158
protected String transformPath(String lookupPath) {
149159
String path = lookupPath;
150160
if (this.stripLeadingSlash && path.startsWith(SLASH)) {

‎spring-webmvc/src/test/java/org/springframework/web/servlet/view/DefaultRequestToViewNameTranslatorTests.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,19 @@
2121

2222
import org.junit.jupiter.api.Named;
2323

24+
import org.springframework.http.HttpStatus;
25+
import org.springframework.web.server.ResponseStatusException;
2426
import org.springframework.web.servlet.handler.PathPatternsParameterizedTest;
2527
import org.springframework.web.servlet.handler.PathPatternsTestUtils;
2628
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
2729

2830
import static org.assertj.core.api.Assertions.assertThat;
31+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2932

3033
/**
3134
* @author Rick Evans
3235
* @author Juergen Hoeller
36+
* @author Sebastien Deleuze
3337
*/
3438
class DefaultRequestToViewNameTranslatorTests {
3539

@@ -121,6 +125,22 @@ void getViewNameWithNullSuffix(Function<String, MockHttpServletRequest> requestF
121125
assertViewName(request, VIEW_NAME);
122126
}
123127

128+
@PathPatternsParameterizedTest
129+
void getViewNameWithRedirectPrefixFails(Function<String, MockHttpServletRequest> requestFactory) {
130+
MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.REDIRECT_URL_PREFIX + VIEW_NAME);
131+
assertThatExceptionOfType(ResponseStatusException.class)
132+
.isThrownBy(() -> this.translator.getViewName(request))
133+
.satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST));
134+
}
135+
136+
@PathPatternsParameterizedTest
137+
void getViewNameWithForwardPrefixFails(Function<String, MockHttpServletRequest> requestFactory) {
138+
MockHttpServletRequest request = requestFactory.apply(UrlBasedViewResolver.FORWARD_URL_PREFIX + VIEW_NAME);
139+
assertThatExceptionOfType(ResponseStatusException.class)
140+
.isThrownBy(() -> this.translator.getViewName(request))
141+
.satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST));
142+
}
143+
124144

125145
private void assertViewName(MockHttpServletRequest request, String expectedViewName) {
126146
String actualViewName = this.translator.getViewName(request);

0 commit comments

Comments
 (0)