Extend Content-Disposition "inline" to error responses

rstoyanchev · rstoyanchev · commit 16d125ce768b · 2020-09-14T15:28:53.000+01:00
We don't expect a browser to save error responses to a file but we
extend this protection anyway since "inline" is only a suggestion
that shouldn't have any side effects.
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
@@ -411,7 +411,7 @@ private void addContentDispositionHeader(ServletServerHttpRequest request, Servl
 
 		try {
 			int status = response.getServletResponse().getStatus();
-			if (status < 200 || status > 299) {
+			if (status < 200 || (status > 299 && status < 400)) {
 				return;
 			}
 		}
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java
@@ -400,6 +400,25 @@ public void addContentDispositionHeader() throws Exception {
 		this.servletRequest.removeAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE);
 	}
 
+	@Test
+	public void addContentDispositionHeaderToErrorResponse() throws Exception {
+		ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean();
+		factory.addMediaType("pdf", new MediaType("application", "pdf"));
+		factory.afterPropertiesSet();
+
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(
+				Collections.singletonList(new StringHttpMessageConverter()),
+				factory.getObject());
+
+		this.servletRequest.setRequestURI("/hello.dataless");
+		this.servletResponse.setStatus(400);
+
+		processor.handleReturnValue("body", this.returnTypeString, this.container, this.request);
+
+		String header = servletResponse.getHeader("Content-Disposition");
+		assertThat(header).isEqualTo("inline;filename=f.txt");
+	}
+
 	@Test
 	public void supportsReturnTypeResponseBodyOnType() throws Exception {
 		Method method = ResponseBodyController.class.getMethod("handle");
@@ -724,10 +743,14 @@ private void assertContentDisposition(RequestResponseBodyMethodProcessor process
 
 		String header = servletResponse.getHeader("Content-Disposition");
 		if (expectContentDisposition) {
-			assertThat(header).as("Expected 'Content-Disposition' header. Use case: '" + comment + "'").isEqualTo("inline;filename=f.txt");
+			assertThat(header)
+					.as("Expected 'Content-Disposition' header. Use case: '" + comment + "'")
+					.isEqualTo("inline;filename=f.txt");
 		}
 		else {
-			assertThat(header).as("Did not expect 'Content-Disposition' header. Use case: '" + comment + "'").isNull();
+			assertThat(header)
+					.as("Did not expect 'Content-Disposition' header. Use case: '" + comment + "'")
+					.isNull();
 		}
 
 		this.servletRequest = new MockHttpServletRequest();

Original file line number	Diff line number	Diff line change
`@@ -411,7 +411,7 @@ private void addContentDispositionHeader(ServletServerHttpRequest request, Servl`
`411`	`411`
`412`	`412`	`try {`
`413`	`413`	`int status = response.getServletResponse().getStatus();`
`414`		`- if (status < 200 \|\| status > 299) {`
	`414`	`+ if (status < 200 \|\| (status > 299 && status < 400)) {`
`415`	`415`	`return;`
`416`	`416`	`}`
`417`	`417`	`}`