Skip to content

Commit 6a9329c

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

File tree

2 files changed

+141
-5
lines changed

2 files changed

+141
-5
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
@@ -253,14 +253,33 @@ private void addContentDispositionHeader(ServletServerHttpRequest request,
253253
pathParams = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, pathParams);
254254
String extInPathParams = StringUtils.getFilenameExtension(pathParams);
255255

256-
if (!isSafeExtension(ext) || !isSafeExtension(extInPathParams)) {
256+
if (!safeExtension(servletRequest, ext) || !safeExtension(servletRequest, extInPathParams)) {
257257
headers.add("Content-Disposition", "attachment;filename=f.txt");
258258
}
259259
}
260260

261-
private boolean isSafeExtension(String extension) {
262-
return (!StringUtils.hasText(extension) ||
263-
this.safeExtensions.contains(extension.toLowerCase(Locale.ENGLISH)));
261+
@SuppressWarnings("unchecked")
262+
private boolean safeExtension(HttpServletRequest request, String extension) {
263+
if (!StringUtils.hasText(extension)) {
264+
return true;
265+
}
266+
extension = extension.toLowerCase(Locale.ENGLISH);
267+
if (this.safeExtensions.contains(extension)) {
268+
return true;
269+
}
270+
if (extension.equals("html")) {
271+
String name = HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE;
272+
String pattern = (String) request.getAttribute(name);
273+
if (pattern != null && pattern.endsWith(".html")) {
274+
return true;
275+
}
276+
name = HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE;
277+
Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(name);
278+
if (!CollectionUtils.isEmpty(mediaTypes) && mediaTypes.contains(MediaType.TEXT_HTML)) {
279+
return true;
280+
}
281+
}
282+
return false;
264283
}
265284

266285
}

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

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
import java.lang.annotation.RetentionPolicy;
3535
import java.lang.annotation.Target;
3636
import java.lang.reflect.Method;
37+
import java.net.URI;
38+
import java.net.URISyntaxException;
39+
import java.nio.charset.Charset;
3740
import java.security.Principal;
3841
import java.text.SimpleDateFormat;
3942
import java.util.ArrayList;
@@ -111,6 +114,8 @@
111114
import org.springframework.validation.Errors;
112115
import org.springframework.validation.FieldError;
113116
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
117+
import org.springframework.web.accept.ContentNegotiationManager;
118+
import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
114119
import org.springframework.web.bind.WebDataBinder;
115120
import org.springframework.web.bind.annotation.CookieValue;
116121
import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -142,6 +147,15 @@
142147
import org.springframework.web.servlet.support.RequestContextUtils;
143148
import org.springframework.web.servlet.view.InternalResourceViewResolver;
144149

150+
import static org.junit.Assert.assertArrayEquals;
151+
import static org.junit.Assert.assertEquals;
152+
import static org.junit.Assert.assertFalse;
153+
import static org.junit.Assert.assertNotNull;
154+
import static org.junit.Assert.assertNull;
155+
import static org.junit.Assert.assertSame;
156+
import static org.junit.Assert.assertTrue;
157+
import static org.junit.Assert.fail;
158+
145159
/**
146160
* The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}.
147161
*
@@ -1569,6 +1583,85 @@ public void initialize(GenericWebApplicationContext context) {
15691583
assertEquals("count:3", response.getContentAsString());
15701584
}
15711585

1586+
@Test
1587+
public void responseBodyAsHtml() throws Exception {
1588+
initServlet(new ApplicationContextInitializer<GenericWebApplicationContext>() {
1589+
@Override
1590+
public void initialize(GenericWebApplicationContext wac) {
1591+
ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
1592+
factoryBean.afterPropertiesSet();
1593+
RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
1594+
adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
1595+
wac.registerBeanDefinition("handlerAdapter", adapterDef);
1596+
}
1597+
}, TextRestController.class);
1598+
1599+
byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1"));
1600+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a1.html");
1601+
request.setContent(content);
1602+
MockHttpServletResponse response = new MockHttpServletResponse();
1603+
1604+
getServlet().service(request, response);
1605+
1606+
assertEquals(200, response.getStatus());
1607+
assertEquals("text/html", response.getContentType());
1608+
assertEquals("attachment;filename=f.txt", response.getHeader("Content-Disposition"));
1609+
assertArrayEquals(content, response.getContentAsByteArray());
1610+
}
1611+
1612+
@Test
1613+
public void responseBodyAsHtmlWithSuffixPresent() 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", "/a2.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+
assertNull(response.getHeader("Content-Disposition"));
1635+
assertArrayEquals(content, response.getContentAsByteArray());
1636+
}
1637+
1638+
@Test
1639+
public void responseBodyAsHtmlWithProducesCondition() 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", "/a3.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+
15721665
/*
15731666
* Controllers
15741667
*/
@@ -2979,7 +3072,31 @@ public void message(int param, Writer writer) throws IOException {
29793072
}
29803073
}
29813074

2982-
// Test cases deleted from the original SevletAnnotationControllerTests:
3075+
@Controller
3076+
public static class TextRestController {
3077+
3078+
@RequestMapping(value = "/a1", method = RequestMethod.GET)
3079+
@ResponseBody
3080+
public String a1(@RequestBody String body) {
3081+
return body;
3082+
}
3083+
3084+
@RequestMapping(value = "/a2.html", method = RequestMethod.GET)
3085+
@ResponseBody
3086+
public String a2(@RequestBody String body) {
3087+
return body;
3088+
}
3089+
3090+
@RequestMapping(value = "/a3", method = RequestMethod.GET, produces = "text/html")
3091+
@ResponseBody
3092+
public String a3(@RequestBody String body) throws IOException {
3093+
return body;
3094+
}
3095+
}
3096+
3097+
3098+
3099+
// Test cases deleted from the original ServletAnnotationControllerTests:
29833100

29843101
// @Ignore("Controller interface => no method-level @RequestMapping annotation")
29853102
// public void standardHandleMethod() throws Exception {

0 commit comments

Comments
 (0)