From 6a9329cb761257cacd45a0d14eaa48e289a25630 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 2 Nov 2015 13:07:48 -0500 Subject: [PATCH] No Content-Disposition if HTML in the request mapping Issue: SPR-13629 --- ...stractMessageConverterMethodProcessor.java | 27 +++- ...nnotationControllerHandlerMethodTests.java | 119 +++++++++++++++++- 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index a67ebe08138..7ae3b829b28 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -253,14 +253,33 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe pathParams = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, pathParams); String extInPathParams = StringUtils.getFilenameExtension(pathParams); - if (!isSafeExtension(ext) || !isSafeExtension(extInPathParams)) { + if (!safeExtension(servletRequest, ext) || !safeExtension(servletRequest, extInPathParams)) { headers.add("Content-Disposition", "attachment;filename=f.txt"); } } - private boolean isSafeExtension(String extension) { - return (!StringUtils.hasText(extension) || - this.safeExtensions.contains(extension.toLowerCase(Locale.ENGLISH))); + @SuppressWarnings("unchecked") + private boolean safeExtension(HttpServletRequest request, String extension) { + if (!StringUtils.hasText(extension)) { + return true; + } + extension = extension.toLowerCase(Locale.ENGLISH); + if (this.safeExtensions.contains(extension)) { + return true; + } + if (extension.equals("html")) { + String name = HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE; + String pattern = (String) request.getAttribute(name); + if (pattern != null && pattern.endsWith(".html")) { + return true; + } + name = HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE; + Set mediaTypes = (Set) request.getAttribute(name); + if (!CollectionUtils.isEmpty(mediaTypes) && mediaTypes.contains(MediaType.TEXT_HTML)) { + return true; + } + } + return false; } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 6c5c7327e08..583525216e1 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -34,6 +34,9 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.Charset; import java.security.Principal; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -111,6 +114,8 @@ import org.springframework.validation.BindingResult; import org.springframework.validation.Errors; import org.springframework.validation.FieldError; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; +import org.springframework.web.accept.ContentNegotiationManager; +import org.springframework.web.accept.ContentNegotiationManagerFactoryBean; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.CookieValue; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -142,6 +147,15 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes; import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.servlet.view.InternalResourceViewResolver; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + /** * The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}. * @@ -1569,6 +1583,85 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertEquals("count:3", response.getContentAsString()); } + @Test + public void responseBodyAsHtml() throws Exception { + initServlet(new ApplicationContextInitializer() { + @Override + public void initialize(GenericWebApplicationContext wac) { + ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean(); + factoryBean.afterPropertiesSet(); + RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class); + adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject()); + wac.registerBeanDefinition("handlerAdapter", adapterDef); + } + }, TextRestController.class); + + byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1")); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a1.html"); + request.setContent(content); + MockHttpServletResponse response = new MockHttpServletResponse(); + + getServlet().service(request, response); + + assertEquals(200, response.getStatus()); + assertEquals("text/html", response.getContentType()); + assertEquals("attachment;filename=f.txt", response.getHeader("Content-Disposition")); + assertArrayEquals(content, response.getContentAsByteArray()); + } + + @Test + public void responseBodyAsHtmlWithSuffixPresent() throws Exception { + initServlet(new ApplicationContextInitializer() { + @Override + public void initialize(GenericWebApplicationContext wac) { + ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean(); + factoryBean.afterPropertiesSet(); + RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class); + adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject()); + wac.registerBeanDefinition("handlerAdapter", adapterDef); + } + }, TextRestController.class); + + byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1")); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a2.html"); + request.setContent(content); + MockHttpServletResponse response = new MockHttpServletResponse(); + + getServlet().service(request, response); + + assertEquals(200, response.getStatus()); + assertEquals("text/html", response.getContentType()); + assertNull(response.getHeader("Content-Disposition")); + assertArrayEquals(content, response.getContentAsByteArray()); + } + + @Test + public void responseBodyAsHtmlWithProducesCondition() throws Exception { + initServlet(new ApplicationContextInitializer() { + @Override + public void initialize(GenericWebApplicationContext wac) { + ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean(); + factoryBean.afterPropertiesSet(); + RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class); + adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject()); + wac.registerBeanDefinition("handlerAdapter", adapterDef); + } + }, TextRestController.class); + + byte[] content = "alert('boo')".getBytes(Charset.forName("ISO-8859-1")); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a3.html"); + request.setContent(content); + MockHttpServletResponse response = new MockHttpServletResponse(); + + getServlet().service(request, response); + + assertEquals(200, response.getStatus()); + assertEquals("text/html", response.getContentType()); + assertNull(response.getHeader("Content-Disposition")); + assertArrayEquals(content, response.getContentAsByteArray()); + } + + /* * Controllers */ @@ -2979,7 +3072,31 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } -// Test cases deleted from the original SevletAnnotationControllerTests: + @Controller + public static class TextRestController { + + @RequestMapping(value = "/a1", method = RequestMethod.GET) + @ResponseBody + public String a1(@RequestBody String body) { + return body; + } + + @RequestMapping(value = "/a2.html", method = RequestMethod.GET) + @ResponseBody + public String a2(@RequestBody String body) { + return body; + } + + @RequestMapping(value = "/a3", method = RequestMethod.GET, produces = "text/html") + @ResponseBody + public String a3(@RequestBody String body) throws IOException { + return body; + } + } + + + +// Test cases deleted from the original ServletAnnotationControllerTests: // @Ignore("Controller interface => no method-level @RequestMapping annotation") // public void standardHandleMethod() throws Exception {