No Content-Disposition if HTML in the request mapping

Issue: SPR-13629

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<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(name);
+			if (!CollectionUtils.isEmpty(mediaTypes) && mediaTypes.contains(MediaType.TEXT_HTML)) {
+				return true;
+			}
+		}
+		return false;
 	}
 
 }

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<GenericWebApplicationContext>() {
+			@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<GenericWebApplicationContext>() {
+			@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<GenericWebApplicationContext>() {
+			@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 {