From bdb71e91ad63d1a8da4bf8d6e9319c35afe0f5c8 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 | 113 +++++++++++++++++- 2 files changed, 133 insertions(+), 7 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 f23efe27e3..4aa6debc85 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 @@ -359,14 +359,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(HttpHeaders.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 efecd12ec9..a62a8d5ab9 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 @@ -16,8 +16,6 @@ package org.springframework.web.servlet.mvc.method.annotation; -import static org.junit.Assert.*; - import java.beans.PropertyEditorSupport; import java.io.IOException; import java.io.Serializable; @@ -30,6 +28,7 @@ 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; @@ -44,7 +43,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; - import javax.servlet.ServletConfig; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -108,6 +106,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; @@ -141,6 +141,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}. * @@ -1624,6 +1633,84 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertEquals("Expected an empty content", 0, response.getContentLength()); } + @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 */ @@ -3083,6 +3170,26 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } + @RestController + public static class TextRestController { + + @RequestMapping(path = "/a1", method = RequestMethod.GET) + public String a1(@RequestBody String body) { + return body; + } + + @RequestMapping(path = "/a2.html", method = RequestMethod.GET) + public String a2(@RequestBody String body) { + return body; + } + + @RequestMapping(path = "/a3", method = RequestMethod.GET, produces = "text/html") + public String a3(@RequestBody String body) throws IOException { + return body; + } + } + + // Test cases deleted from the original ServletAnnotationControllerTests: