Skip to content

Commit 380f5d5

Browse files
sbrannensbrannen
committed
Honor Content-[Type|Length] headers from wrapped response again
Commit 375e0e6 introduced a regression in ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer honors Content-Type and Content-Length headers that have been set in the wrapped response and now incorrectly returns null for those header values if they have not been set directly in the CCRW. This commit fixes this regression as follows. - The Content-Type and Content-Length headers set in the wrapped response are honored in getContentType(), containsHeader(), getHeader(), and getHeaders() unless those headers have been set directly in the CCRW. - In copyBodyToResponse(), the Content-Type in the wrapped response is only overridden if the Content-Type has been set directly in the CCRW. Furthermore, prior to this commit, getHeaderNames() returned duplicates for the Content-Type and Content-Length headers if they were set in the wrapped response as well as in CCRW. This commit fixes that by returning a unique set from getHeaderNames(). This commit also updates ContentCachingResponseWrapperTests to verify the expected behavior for Content-Type and Content-Length headers that are set in the wrapped response as well as in CCRW. See gh-32039 See gh-32317 Closes gh-32322
1 parent 1acc1c3 commit 380f5d5

File tree

2 files changed

+158
-17
lines changed

2 files changed

+158
-17
lines changed

‎spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
import java.io.OutputStreamWriter;
2222
import java.io.PrintWriter;
2323
import java.io.UnsupportedEncodingException;
24-
import java.util.ArrayList;
2524
import java.util.Collection;
2625
import java.util.Collections;
27-
import java.util.List;
26+
import java.util.LinkedHashSet;
27+
import java.util.Set;
2828

2929
import javax.servlet.ServletOutputStream;
3030
import javax.servlet.WriteListener;
@@ -44,6 +44,7 @@
4444
* Note: As of Spring Framework 5.0, this wrapper is built on the Servlet 3.1 API.
4545
*
4646
* @author Juergen Hoeller
47+
* @author Sam Brannen
4748
* @since 4.1.3
4849
* @see ContentCachingRequestWrapper
4950
*/
@@ -160,16 +161,19 @@ public void setContentType(@Nullable String type) {
160161
@Override
161162
@Nullable
162163
public String getContentType() {
163-
return this.contentType;
164+
if (this.contentType != null) {
165+
return this.contentType;
166+
}
167+
return super.getContentType();
164168
}
165169

166170
@Override
167171
public boolean containsHeader(String name) {
168-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
169-
return this.contentLength != null;
172+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
173+
return true;
170174
}
171-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
172-
return this.contentType != null;
175+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
176+
return true;
173177
}
174178
else {
175179
return super.containsHeader(name);
@@ -225,10 +229,10 @@ public void addIntHeader(String name, int value) {
225229
@Override
226230
@Nullable
227231
public String getHeader(String name) {
228-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
229-
return (this.contentLength != null) ? this.contentLength.toString() : null;
232+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
233+
return this.contentLength.toString();
230234
}
231-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
235+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
232236
return this.contentType;
233237
}
234238
else {
@@ -238,12 +242,11 @@ else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
238242

239243
@Override
240244
public Collection<String> getHeaders(String name) {
241-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
242-
return this.contentLength != null ? Collections.singleton(this.contentLength.toString()) :
243-
Collections.emptySet();
245+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
246+
return Collections.singleton(this.contentLength.toString());
244247
}
245-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
246-
return this.contentType != null ? Collections.singleton(this.contentType) : Collections.emptySet();
248+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
249+
return Collections.singleton(this.contentType);
247250
}
248251
else {
249252
return super.getHeaders(name);
@@ -254,7 +257,7 @@ else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
254257
public Collection<String> getHeaderNames() {
255258
Collection<String> headerNames = super.getHeaderNames();
256259
if (this.contentLength != null || this.contentType != null) {
257-
List<String> result = new ArrayList<>(headerNames);
260+
Set<String> result = new LinkedHashSet<>(headerNames);
258261
if (this.contentLength != null) {
259262
result.add(HttpHeaders.CONTENT_LENGTH);
260263
}
@@ -342,7 +345,7 @@ protected void copyBodyToResponse(boolean complete) throws IOException {
342345
}
343346
this.contentLength = null;
344347
}
345-
if (complete || this.contentType != null) {
348+
if (this.contentType != null) {
346349
rawResponse.setContentType(this.contentType);
347350
this.contentType = null;
348351
}

‎spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,34 @@
1616

1717
package org.springframework.web.filter;
1818

19+
import java.util.function.BiConsumer;
20+
import java.util.stream.Stream;
21+
1922
import javax.servlet.http.HttpServletResponse;
2023

2124
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.Arguments;
27+
import org.junit.jupiter.params.provider.MethodSource;
2228

29+
import org.springframework.http.MediaType;
2330
import org.springframework.util.FileCopyUtils;
2431
import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
2532
import org.springframework.web.util.ContentCachingResponseWrapper;
2633

2734
import static java.nio.charset.StandardCharsets.UTF_8;
2835
import static org.assertj.core.api.Assertions.assertThat;
36+
import static org.junit.jupiter.api.Named.named;
37+
import static org.junit.jupiter.params.provider.Arguments.arguments;
2938
import static org.springframework.http.HttpHeaders.CONTENT_LENGTH;
39+
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
3040
import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING;
3141

3242
/**
3343
* Unit tests for {@link ContentCachingResponseWrapper}.
3444
*
3545
* @author Rossen Stoyanchev
46+
* @author Sam Brannen
3647
*/
3748
public class ContentCachingResponseWrapperTests {
3849

@@ -51,6 +62,124 @@ void copyBodyToResponse() throws Exception {
5162
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
5263
}
5364

65+
@Test
66+
void copyBodyToResponseWithPresetHeaders() throws Exception {
67+
String PUZZLE = "puzzle";
68+
String ENIGMA = "enigma";
69+
String NUMBER = "number";
70+
String MAGIC = "42";
71+
72+
byte[] responseBody = "Hello World".getBytes(UTF_8);
73+
int responseLength = responseBody.length;
74+
int originalContentLength = 999;
75+
String contentType = MediaType.APPLICATION_JSON_VALUE;
76+
77+
MockHttpServletResponse response = new MockHttpServletResponse();
78+
response.setContentType(contentType);
79+
response.setContentLength(originalContentLength);
80+
response.setHeader(PUZZLE, ENIGMA);
81+
response.setIntHeader(NUMBER, 42);
82+
83+
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
84+
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
85+
86+
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
87+
assertThat(responseWrapper.getContentSize()).isZero();
88+
assertThat(responseWrapper.getHeaderNames())
89+
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
90+
91+
assertHeader(responseWrapper, PUZZLE, ENIGMA);
92+
assertHeader(responseWrapper, NUMBER, MAGIC);
93+
assertHeader(responseWrapper, CONTENT_LENGTH, originalContentLength);
94+
assertContentTypeHeader(responseWrapper, contentType);
95+
96+
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
97+
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
98+
99+
responseWrapper.copyBodyToResponse();
100+
101+
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
102+
assertThat(responseWrapper.getContentSize()).isZero();
103+
assertThat(responseWrapper.getHeaderNames())
104+
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
105+
106+
assertHeader(responseWrapper, PUZZLE, ENIGMA);
107+
assertHeader(responseWrapper, NUMBER, MAGIC);
108+
assertHeader(responseWrapper, CONTENT_LENGTH, responseLength);
109+
assertContentTypeHeader(responseWrapper, contentType);
110+
111+
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
112+
assertThat(response.getContentLength()).isEqualTo(responseLength);
113+
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
114+
assertThat(response.getHeaderNames())
115+
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
116+
117+
assertHeader(response, PUZZLE, ENIGMA);
118+
assertHeader(response, NUMBER, MAGIC);
119+
assertHeader(response, CONTENT_LENGTH, responseLength);
120+
assertContentTypeHeader(response, contentType);
121+
}
122+
123+
@ParameterizedTest(name = "[{index}] {0}")
124+
@MethodSource("setContentTypeFunctions")
125+
void copyBodyToResponseWithOverridingHeaders(BiConsumer<HttpServletResponse, String> setContentType) throws Exception {
126+
byte[] responseBody = "Hello World".getBytes(UTF_8);
127+
int responseLength = responseBody.length;
128+
int originalContentLength = 11;
129+
int overridingContentLength = 22;
130+
String originalContentType = MediaType.TEXT_PLAIN_VALUE;
131+
String overridingContentType = MediaType.APPLICATION_JSON_VALUE;
132+
133+
MockHttpServletResponse response = new MockHttpServletResponse();
134+
response.setContentLength(originalContentLength);
135+
response.setContentType(originalContentType);
136+
137+
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
138+
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
139+
responseWrapper.setContentLength(overridingContentLength);
140+
setContentType.accept(responseWrapper, overridingContentType);
141+
142+
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
143+
assertThat(responseWrapper.getContentSize()).isZero();
144+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
145+
146+
assertHeader(response, CONTENT_LENGTH, originalContentLength);
147+
assertHeader(responseWrapper, CONTENT_LENGTH, overridingContentLength);
148+
assertContentTypeHeader(response, originalContentType);
149+
assertContentTypeHeader(responseWrapper, overridingContentType);
150+
151+
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
152+
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
153+
154+
responseWrapper.copyBodyToResponse();
155+
156+
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
157+
assertThat(responseWrapper.getContentSize()).isZero();
158+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
159+
160+
assertHeader(response, CONTENT_LENGTH, responseLength);
161+
assertHeader(responseWrapper, CONTENT_LENGTH, responseLength);
162+
assertContentTypeHeader(response, overridingContentType);
163+
assertContentTypeHeader(responseWrapper, overridingContentType);
164+
165+
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
166+
assertThat(response.getContentLength()).isEqualTo(responseLength);
167+
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
168+
assertThat(response.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
169+
}
170+
171+
private static Stream<Arguments> setContentTypeFunctions() {
172+
return Stream.of(
173+
namedArguments("setContentType()", HttpServletResponse::setContentType),
174+
namedArguments("setHeader()", (response, contentType) -> response.setHeader(CONTENT_TYPE, contentType)),
175+
namedArguments("addHeader()", (response, contentType) -> response.addHeader(CONTENT_TYPE, contentType))
176+
);
177+
}
178+
179+
private static Arguments namedArguments(String name, BiConsumer<HttpServletResponse, String> setContentTypeFunction) {
180+
return arguments(named(name, setContentTypeFunction));
181+
}
182+
54183
@Test
55184
void copyBodyToResponseWithTransferEncoding() throws Exception {
56185
byte[] responseBody = "6\r\nHello 5\r\nWorld0\r\n\r\n".getBytes(UTF_8);
@@ -68,6 +197,10 @@ void copyBodyToResponseWithTransferEncoding() throws Exception {
68197
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
69198
}
70199

200+
private void assertHeader(HttpServletResponse response, String header, int value) {
201+
assertHeader(response, header, Integer.toString(value));
202+
}
203+
71204
private void assertHeader(HttpServletResponse response, String header, String value) {
72205
if (value == null) {
73206
assertThat(response.containsHeader(header)).as(header).isFalse();
@@ -81,4 +214,9 @@ private void assertHeader(HttpServletResponse response, String header, String va
81214
}
82215
}
83216

217+
private void assertContentTypeHeader(HttpServletResponse response, String contentType) {
218+
assertHeader(response, CONTENT_TYPE, contentType);
219+
assertThat(response.getContentType()).as(CONTENT_TYPE).isEqualTo(contentType);
220+
}
221+
84222
}

0 commit comments

Comments
 (0)