Skip to content

Commit bdb71e9

Browse files
rstoyanchevrstoyanchev
committed
No Content-Disposition if HTML in the request mapping
Issue: SPR-13629
1 parent 1bc41bd commit bdb71e9

File tree

2 files changed

+133
-7
lines changed

2 files changed

+133
-7
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,33 @@ private void addContentDispositionHeader(ServletServerHttpRequest request,
359359
pathParams = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, pathParams);
360360
String extInPathParams = StringUtils.getFilenameExtension(pathParams);
361361

362-
if (!isSafeExtension(ext) || !isSafeExtension(extInPathParams)) {
362+
if (!safeExtension(servletRequest, ext) || !safeExtension(servletRequest, extInPathParams)) {
363363
headers.add(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=f.txt");
364364
}
365365
}
366366

367-
private boolean isSafeExtension(String extension) {
368-
return (!StringUtils.hasText(extension) ||
369-
this.safeExtensions.contains(extension.toLowerCase(Locale.ENGLISH)));
367+
@SuppressWarnings("unchecked")
368+
private boolean safeExtension(HttpServletRequest request, String extension) {
369+
if (!StringUtils.hasText(extension)) {
370+
return true;
371+
}
372+
extension = extension.toLowerCase(Locale.ENGLISH);
373+
if (this.safeExtensions.contains(extension)) {
374+
return true;
375+
}
376+
if (extension.equals("html")) {
377+
String name = HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE;
378+
String pattern = (String) request.getAttribute(name);
379+
if (pattern != null && pattern.endsWith(".html")) {
380+
return true;
381+
}
382+
name = HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE;
383+
Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(name);
384+
if (!CollectionUtils.isEmpty(mediaTypes) && mediaTypes.contains(MediaType.TEXT_HTML)) {
385+
return true;
386+
}
387+
}
388+
return false;
370389
}
371390

372391
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java‎

Lines changed: 110 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19-
import static org.junit.Assert.*;
20-
2119
import java.beans.PropertyEditorSupport;
2220
import java.io.IOException;
2321
import java.io.Serializable;
@@ -30,6 +28,7 @@
3028
import java.lang.reflect.Method;
3129
import java.net.URI;
3230
import java.net.URISyntaxException;
31+
import java.nio.charset.Charset;
3332
import java.security.Principal;
3433
import java.text.SimpleDateFormat;
3534
import java.util.ArrayList;
@@ -44,7 +43,6 @@
4443
import java.util.Locale;
4544
import java.util.Map;
4645
import java.util.Set;
47-
4846
import javax.servlet.ServletConfig;
4947
import javax.servlet.ServletContext;
5048
import javax.servlet.ServletException;
@@ -108,6 +106,8 @@
108106
import org.springframework.validation.Errors;
109107
import org.springframework.validation.FieldError;
110108
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
109+
import org.springframework.web.accept.ContentNegotiationManager;
110+
import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
111111
import org.springframework.web.bind.WebDataBinder;
112112
import org.springframework.web.bind.annotation.CookieValue;
113113
import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -141,6 +141,15 @@
141141
import org.springframework.web.servlet.support.RequestContextUtils;
142142
import org.springframework.web.servlet.view.InternalResourceViewResolver;
143143

144+
import static org.junit.Assert.assertArrayEquals;
145+
import static org.junit.Assert.assertEquals;
146+
import static org.junit.Assert.assertFalse;
147+
import static org.junit.Assert.assertNotNull;
148+
import static org.junit.Assert.assertNull;
149+
import static org.junit.Assert.assertSame;
150+
import static org.junit.Assert.assertTrue;
151+
import static org.junit.Assert.fail;
152+
144153
/**
145154
* The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}.
146155
*
@@ -1624,6 +1633,84 @@ public void responseAsHttpHeadersNoHeader() throws Exception {
16241633
assertEquals("Expected an empty content", 0, response.getContentLength());
16251634
}
16261635

1636+
@Test
1637+
public void responseBodyAsHtml() throws Exception {
1638+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1639+
@Override
1640+
public void initialize(GenericWebApplicationContext wac) {
1641+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1642+
factoryBean.afterPropertiesSet();
1643+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1644+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1645+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1646+
}
1647+
}, TextRestController.class);
1648+
1649+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1650+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a1.html");
1651+
request.setContent(content);
1652+
MockHttpServletResponse response = new MockHttpServletResponse();
1653+
1654+
getServlet().service(request, response);
1655+
1656+
assertEquals(200, response.getStatus());
1657+
assertEquals("text/html", response.getContentType());
1658+
assertEquals("attachment;filename=f.txt", response.getHeader("Content-Disposition"));
1659+
assertArrayEquals(content, response.getContentAsByteArray());
1660+
}
1661+
1662+
@Test
1663+
public void responseBodyAsHtmlWithSuffixPresent() throws Exception {
1664+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1665+
@Override
1666+
public void initialize(GenericWebApplicationContext wac) {
1667+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1668+
factoryBean.afterPropertiesSet();
1669+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1670+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1671+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1672+
}
1673+
}, TextRestController.class);
1674+
1675+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1676+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a2.html");
1677+
request.setContent(content);
1678+
MockHttpServletResponse response = new MockHttpServletResponse();
1679+
1680+
getServlet().service(request, response);
1681+
1682+
assertEquals(200, response.getStatus());
1683+
assertEquals("text/html", response.getContentType());
1684+
assertNull(response.getHeader("Content-Disposition"));
1685+
assertArrayEquals(content, response.getContentAsByteArray());
1686+
}
1687+
1688+
@Test
1689+
public void responseBodyAsHtmlWithProducesCondition() throws Exception {
1690+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1691+
@Override
1692+
public void initialize(GenericWebApplicationContext wac) {
1693+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1694+
factoryBean.afterPropertiesSet();
1695+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1696+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1697+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1698+
}
1699+
}, TextRestController.class);
1700+
1701+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1702+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a3.html");
1703+
request.setContent(content);
1704+
MockHttpServletResponse response = new MockHttpServletResponse();
1705+
1706+
getServlet().service(request, response);
1707+
1708+
assertEquals(200, response.getStatus());
1709+
assertEquals("text/html", response.getContentType());
1710+
assertNull(response.getHeader("Content-Disposition"));
1711+
assertArrayEquals(content, response.getContentAsByteArray());
1712+
}
1713+
16271714
/*
16281715
* Controllers
16291716
*/
@@ -3083,6 +3170,26 @@ public HttpHeaders createNoHeader() throws URISyntaxException {
30833170

30843171
}
30853172

3173+
@RestController
3174+
public static class TextRestController {
3175+
3176+
@RequestMapping(path = "/a1", method = RequestMethod.GET)
3177+
public String a1(@RequestBody String body) {
3178+
return body;
3179+
}
3180+
3181+
@RequestMapping(path = "/a2.html", method = RequestMethod.GET)
3182+
public String a2(@RequestBody String body) {
3183+
return body;
3184+
}
3185+
3186+
@RequestMapping(path = "/a3", method = RequestMethod.GET, produces = "text/html")
3187+
public String a3(@RequestBody String body) throws IOException {
3188+
return body;
3189+
}
3190+
}
3191+
3192+
30863193

30873194
// Test cases deleted from the original ServletAnnotationControllerTests:
30883195

0 commit comments

Comments
 (0)