Skip to content

Commit e190f26

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

File tree

2 files changed

+132
-5
lines changed

2 files changed

+132
-5
lines changed

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

+23-4
Original file line numberDiff line numberDiff line change
@@ -296,14 +296,33 @@ private void addContentDispositionHeader(ServletServerHttpRequest request,
296296
pathParams = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, pathParams);
297297
String extInPathParams = StringUtils.getFilenameExtension(pathParams);
298298

299-
if (!isSafeExtension(ext) || !isSafeExtension(extInPathParams)) {
299+
if (!safeExtension(servletRequest, ext) || !safeExtension(servletRequest, extInPathParams)) {
300300
headers.add(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=f.txt");
301301
}
302302
}
303303

304-
private boolean isSafeExtension(String extension) {
305-
return (!StringUtils.hasText(extension) ||
306-
this.safeExtensions.contains(extension.toLowerCase(Locale.ENGLISH)));
304+
@SuppressWarnings("unchecked")
305+
private boolean safeExtension(HttpServletRequest request, String extension) {
306+
if (!StringUtils.hasText(extension)) {
307+
return true;
308+
}
309+
extension = extension.toLowerCase(Locale.ENGLISH);
310+
if (this.safeExtensions.contains(extension)) {
311+
return true;
312+
}
313+
if (extension.equals("html")) {
314+
String name = HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE;
315+
String pattern = (String) request.getAttribute(name);
316+
if (pattern != null && pattern.endsWith(".html")) {
317+
return true;
318+
}
319+
name = HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE;
320+
Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(name);
321+
if (!CollectionUtils.isEmpty(mediaTypes) && mediaTypes.contains(MediaType.TEXT_HTML)) {
322+
return true;
323+
}
324+
}
325+
return false;
307326
}
308327

309328
}

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

+109-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.lang.reflect.Method;
2929
import java.net.URI;
3030
import java.net.URISyntaxException;
31+
import java.nio.charset.Charset;
3132
import java.security.Principal;
3233
import java.text.SimpleDateFormat;
3334
import java.util.ArrayList;
@@ -105,6 +106,8 @@
105106
import org.springframework.validation.Errors;
106107
import org.springframework.validation.FieldError;
107108
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
109+
import org.springframework.web.accept.ContentNegotiationManager;
110+
import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
108111
import org.springframework.web.bind.WebDataBinder;
109112
import org.springframework.web.bind.annotation.CookieValue;
110113
import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -137,7 +140,14 @@
137140
import org.springframework.web.servlet.support.RequestContextUtils;
138141
import org.springframework.web.servlet.view.InternalResourceViewResolver;
139142

140-
import static org.junit.Assert.*;
143+
import static org.junit.Assert.assertArrayEquals;
144+
import static org.junit.Assert.assertEquals;
145+
import static org.junit.Assert.assertFalse;
146+
import static org.junit.Assert.assertNotNull;
147+
import static org.junit.Assert.assertNull;
148+
import static org.junit.Assert.assertSame;
149+
import static org.junit.Assert.assertTrue;
150+
import static org.junit.Assert.fail;
141151

142152
/**
143153
* The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}.
@@ -1599,6 +1609,84 @@ public void responseAsHttpHeadersNoHeader() throws Exception {
15991609
assertEquals("Expected an empty content", 0, response.getContentLength());
16001610
}
16011611

1612+
@Test
1613+
public void responseBodyAsHtml() throws Exception {
1614+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1615+
@Override
1616+
public void initialize(GenericWebApplicationContext wac) {
1617+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1618+
factoryBean.afterPropertiesSet();
1619+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1620+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1621+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1622+
}
1623+
}, TextRestController.class);
1624+
1625+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1626+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a1.html");
1627+
request.setContent(content);
1628+
MockHttpServletResponse response = new MockHttpServletResponse();
1629+
1630+
getServlet().service(request, response);
1631+
1632+
assertEquals(200, response.getStatus());
1633+
assertEquals("text/html", response.getContentType());
1634+
assertEquals("attachment;filename=f.txt", response.getHeader("Content-Disposition"));
1635+
assertArrayEquals(content, response.getContentAsByteArray());
1636+
}
1637+
1638+
@Test
1639+
public void responseBodyAsHtmlWithSuffixPresent() throws Exception {
1640+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1641+
@Override
1642+
public void initialize(GenericWebApplicationContext wac) {
1643+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1644+
factoryBean.afterPropertiesSet();
1645+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1646+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1647+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1648+
}
1649+
}, TextRestController.class);
1650+
1651+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1652+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a2.html");
1653+
request.setContent(content);
1654+
MockHttpServletResponse response = new MockHttpServletResponse();
1655+
1656+
getServlet().service(request, response);
1657+
1658+
assertEquals(200, response.getStatus());
1659+
assertEquals("text/html", response.getContentType());
1660+
assertNull(response.getHeader("Content-Disposition"));
1661+
assertArrayEquals(content, response.getContentAsByteArray());
1662+
}
1663+
1664+
@Test
1665+
public void responseBodyAsHtmlWithProducesCondition() throws Exception {
1666+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1667+
@Override
1668+
public void initialize(GenericWebApplicationContext wac) {
1669+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1670+
factoryBean.afterPropertiesSet();
1671+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1672+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1673+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1674+
}
1675+
}, TextRestController.class);
1676+
1677+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1678+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a3.html");
1679+
request.setContent(content);
1680+
MockHttpServletResponse response = new MockHttpServletResponse();
1681+
1682+
getServlet().service(request, response);
1683+
1684+
assertEquals(200, response.getStatus());
1685+
assertEquals("text/html", response.getContentType());
1686+
assertNull(response.getHeader("Content-Disposition"));
1687+
assertArrayEquals(content, response.getContentAsByteArray());
1688+
}
1689+
16021690
/*
16031691
* Controllers
16041692
*/
@@ -3037,6 +3125,26 @@ public HttpHeaders createNoHeader() throws URISyntaxException {
30373125

30383126
}
30393127

3128+
@RestController
3129+
public static class TextRestController {
3130+
3131+
@RequestMapping(value = "/a1", method = RequestMethod.GET)
3132+
public String a1(@RequestBody String body) {
3133+
return body;
3134+
}
3135+
3136+
@RequestMapping(value = "/a2.html", method = RequestMethod.GET)
3137+
public String a2(@RequestBody String body) {
3138+
return body;
3139+
}
3140+
3141+
@RequestMapping(value = "/a3", method = RequestMethod.GET, produces = "text/html")
3142+
public String a3(@RequestBody String body) throws IOException {
3143+
return body;
3144+
}
3145+
}
3146+
3147+
30403148

30413149
// Test cases deleted from the original ServletAnnotationControllerTests:
30423150

0 commit comments

Comments
 (0)