Skip to content

Commit ca602ef

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-32321
1 parent e9bf5f5 commit ca602ef

File tree

2 files changed

+158
-17
lines changed

2 files changed

+158
-17
lines changed

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

+20-17
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 jakarta.servlet.ServletOutputStream;
3030
import jakarta.servlet.WriteListener;
@@ -43,6 +43,7 @@
4343
* <p>Used e.g. by {@link org.springframework.web.filter.ShallowEtagHeaderFilter}.
4444
*
4545
* @author Juergen Hoeller
46+
* @author Sam Brannen
4647
* @since 4.1.3
4748
* @see ContentCachingRequestWrapper
4849
*/
@@ -157,16 +158,19 @@ public void setContentType(@Nullable String type) {
157158
@Override
158159
@Nullable
159160
public String getContentType() {
160-
return this.contentType;
161+
if (this.contentType != null) {
162+
return this.contentType;
163+
}
164+
return super.getContentType();
161165
}
162166

163167
@Override
164168
public boolean containsHeader(String name) {
165-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
166-
return this.contentLength != null;
169+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
170+
return true;
167171
}
168-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
169-
return this.contentType != null;
172+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
173+
return true;
170174
}
171175
else {
172176
return super.containsHeader(name);
@@ -222,10 +226,10 @@ public void addIntHeader(String name, int value) {
222226
@Override
223227
@Nullable
224228
public String getHeader(String name) {
225-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
226-
return (this.contentLength != null) ? this.contentLength.toString() : null;
229+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
230+
return this.contentLength.toString();
227231
}
228-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
232+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
229233
return this.contentType;
230234
}
231235
else {
@@ -235,12 +239,11 @@ else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
235239

236240
@Override
237241
public Collection<String> getHeaders(String name) {
238-
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
239-
return this.contentLength != null ? Collections.singleton(this.contentLength.toString()) :
240-
Collections.emptySet();
242+
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
243+
return Collections.singleton(this.contentLength.toString());
241244
}
242-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
243-
return this.contentType != null ? Collections.singleton(this.contentType) : Collections.emptySet();
245+
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
246+
return Collections.singleton(this.contentType);
244247
}
245248
else {
246249
return super.getHeaders(name);
@@ -251,7 +254,7 @@ else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
251254
public Collection<String> getHeaderNames() {
252255
Collection<String> headerNames = super.getHeaderNames();
253256
if (this.contentLength != null || this.contentType != null) {
254-
List<String> result = new ArrayList<>(headerNames);
257+
Set<String> result = new LinkedHashSet<>(headerNames);
255258
if (this.contentLength != null) {
256259
result.add(HttpHeaders.CONTENT_LENGTH);
257260
}
@@ -330,7 +333,7 @@ protected void copyBodyToResponse(boolean complete) throws IOException {
330333
}
331334
this.contentLength = null;
332335
}
333-
if (complete || this.contentType != null) {
336+
if (this.contentType != null) {
334337
rawResponse.setContentType(this.contentType);
335338
this.contentType = null;
336339
}

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

+138
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,32 @@
1616

1717
package org.springframework.web.filter;
1818

19+
import java.util.function.BiConsumer;
20+
import java.util.stream.Stream;
21+
1922
import jakarta.servlet.http.HttpServletResponse;
2023
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.params.ParameterizedTest;
25+
import org.junit.jupiter.params.provider.Arguments;
26+
import org.junit.jupiter.params.provider.MethodSource;
2127

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

2633
import static java.nio.charset.StandardCharsets.UTF_8;
2734
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.junit.jupiter.api.Named.named;
36+
import static org.junit.jupiter.params.provider.Arguments.arguments;
2837
import static org.springframework.http.HttpHeaders.CONTENT_LENGTH;
38+
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
2939
import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING;
3040

3141
/**
3242
* Unit tests for {@link ContentCachingResponseWrapper}.
3343
* @author Rossen Stoyanchev
44+
* @author Sam Brannen
3445
*/
3546
public class ContentCachingResponseWrapperTests {
3647

@@ -49,6 +60,124 @@ void copyBodyToResponse() throws Exception {
4960
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
5061
}
5162

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

198+
private void assertHeader(HttpServletResponse response, String header, int value) {
199+
assertHeader(response, header, Integer.toString(value));
200+
}
201+
69202
private void assertHeader(HttpServletResponse response, String header, String value) {
70203
if (value == null) {
71204
assertThat(response.containsHeader(header)).as(header).isFalse();
@@ -79,4 +212,9 @@ private void assertHeader(HttpServletResponse response, String header, String va
79212
}
80213
}
81214

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

0 commit comments

Comments
 (0)