Skip to content

Commit 464fa7e

Browse files
sbrannensbrannen
committed
Do not cache Content-Type in ContentCachingResponseWrapper
Based on feedback from several members of the community, we have decided to revert the caching of the Content-Type header that was introduced in ContentCachingResponseWrapper in 375e0e6. This commit therefore completely removes Content-Type caching in ContentCachingResponseWrapper and updates the existing tests accordingly. To provide guards against future regressions in this area, this commit also introduces explicit tests for the 6 ways to set the content length in ContentCachingResponseWrapper and modifies a test in ShallowEtagHeaderFilterTests to ensure that a Content-Type header set directly on ContentCachingResponseWrapper is propagated to the underlying response even if content caching is disabled for the ShallowEtagHeaderFilter. See gh-32039 See gh-32317 Closes gh-32322
1 parent 6e1f583 commit 464fa7e

File tree

3 files changed

+79
-67
lines changed

3 files changed

+79
-67
lines changed

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

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
6161
@Nullable
6262
private Integer contentLength;
6363

64-
@Nullable
65-
private String contentType;
66-
6764

6865
/**
6966
* Create a new ContentCachingResponseWrapper for the given servlet response.
@@ -153,28 +150,11 @@ public void setContentLengthLong(long len) {
153150
setContentLength((int) len);
154151
}
155152

156-
@Override
157-
public void setContentType(@Nullable String type) {
158-
this.contentType = type;
159-
}
160-
161-
@Override
162-
@Nullable
163-
public String getContentType() {
164-
if (this.contentType != null) {
165-
return this.contentType;
166-
}
167-
return super.getContentType();
168-
}
169-
170153
@Override
171154
public boolean containsHeader(String name) {
172155
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
173156
return true;
174157
}
175-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
176-
return true;
177-
}
178158
else {
179159
return super.containsHeader(name);
180160
}
@@ -185,9 +165,6 @@ public void setHeader(String name, String value) {
185165
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
186166
this.contentLength = Integer.valueOf(value);
187167
}
188-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
189-
this.contentType = value;
190-
}
191168
else {
192169
super.setHeader(name, value);
193170
}
@@ -198,9 +175,6 @@ public void addHeader(String name, String value) {
198175
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
199176
this.contentLength = Integer.valueOf(value);
200177
}
201-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
202-
this.contentType = value;
203-
}
204178
else {
205179
super.addHeader(name, value);
206180
}
@@ -232,9 +206,6 @@ public String getHeader(String name) {
232206
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
233207
return this.contentLength.toString();
234208
}
235-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
236-
return this.contentType;
237-
}
238209
else {
239210
return super.getHeader(name);
240211
}
@@ -245,9 +216,6 @@ public Collection<String> getHeaders(String name) {
245216
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
246217
return Collections.singleton(this.contentLength.toString());
247218
}
248-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
249-
return Collections.singleton(this.contentType);
250-
}
251219
else {
252220
return super.getHeaders(name);
253221
}
@@ -256,14 +224,9 @@ else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(n
256224
@Override
257225
public Collection<String> getHeaderNames() {
258226
Collection<String> headerNames = super.getHeaderNames();
259-
if (this.contentLength != null || this.contentType != null) {
227+
if (this.contentLength != null) {
260228
Set<String> result = new LinkedHashSet<>(headerNames);
261-
if (this.contentLength != null) {
262-
result.add(HttpHeaders.CONTENT_LENGTH);
263-
}
264-
if (this.contentType != null) {
265-
result.add(HttpHeaders.CONTENT_TYPE);
266-
}
229+
result.add(HttpHeaders.CONTENT_LENGTH);
267230
return result;
268231
}
269232
else {
@@ -345,10 +308,6 @@ protected void copyBodyToResponse(boolean complete) throws IOException {
345308
}
346309
this.contentLength = null;
347310
}
348-
if (this.contentType != null) {
349-
rawResponse.setContentType(this.contentType);
350-
this.contentType = null;
351-
}
352311
}
353312
this.content.writeTo(rawResponse.getOutputStream());
354313
this.content.reset();

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

Lines changed: 71 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,13 @@
1616

1717
package org.springframework.web.filter;
1818

19-
import java.util.function.BiConsumer;
2019
import java.util.stream.Stream;
2120

2221
import javax.servlet.http.HttpServletResponse;
2322

23+
import org.junit.jupiter.api.Named;
2424
import org.junit.jupiter.api.Test;
2525
import org.junit.jupiter.params.ParameterizedTest;
26-
import org.junit.jupiter.params.provider.Arguments;
2726
import org.junit.jupiter.params.provider.MethodSource;
2827

2928
import org.springframework.http.MediaType;
@@ -34,7 +33,6 @@
3433
import static java.nio.charset.StandardCharsets.UTF_8;
3534
import static org.assertj.core.api.Assertions.assertThat;
3635
import static org.junit.jupiter.api.Named.named;
37-
import static org.junit.jupiter.params.provider.Arguments.arguments;
3836
import static org.springframework.http.HttpHeaders.CONTENT_LENGTH;
3937
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
4038
import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING;
@@ -45,7 +43,7 @@
4543
* @author Rossen Stoyanchev
4644
* @author Sam Brannen
4745
*/
48-
public class ContentCachingResponseWrapperTests {
46+
class ContentCachingResponseWrapperTests {
4947

5048
@Test
5149
void copyBodyToResponse() throws Exception {
@@ -121,39 +119,83 @@ void copyBodyToResponseWithPresetHeaders() throws Exception {
121119
}
122120

123121
@ParameterizedTest(name = "[{index}] {0}")
124-
@MethodSource("setContentTypeFunctions")
125-
void copyBodyToResponseWithOverridingHeaders(BiConsumer<HttpServletResponse, String> setContentType) throws Exception {
122+
@MethodSource("setContentLengthFunctions")
123+
void copyBodyToResponseWithOverridingContentLength(SetContentLength setContentLength) throws Exception {
126124
byte[] responseBody = "Hello World".getBytes(UTF_8);
127125
int responseLength = responseBody.length;
128126
int originalContentLength = 11;
129127
int overridingContentLength = 22;
130-
String originalContentType = MediaType.TEXT_PLAIN_VALUE;
131-
String overridingContentType = MediaType.APPLICATION_JSON_VALUE;
132128

133129
MockHttpServletResponse response = new MockHttpServletResponse();
134130
response.setContentLength(originalContentLength);
135-
response.setContentType(originalContentType);
136131

137132
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
138-
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
139133
responseWrapper.setContentLength(overridingContentLength);
140-
setContentType.accept(responseWrapper, overridingContentType);
141134

142-
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
135+
setContentLength.invoke(responseWrapper, overridingContentLength);
136+
143137
assertThat(responseWrapper.getContentSize()).isZero();
144-
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
138+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_LENGTH);
145139

146140
assertHeader(response, CONTENT_LENGTH, originalContentLength);
147141
assertHeader(responseWrapper, CONTENT_LENGTH, overridingContentLength);
142+
143+
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
144+
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
145+
146+
responseWrapper.copyBodyToResponse();
147+
148+
assertThat(responseWrapper.getContentSize()).isZero();
149+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_LENGTH);
150+
151+
assertHeader(response, CONTENT_LENGTH, responseLength);
152+
assertHeader(responseWrapper, CONTENT_LENGTH, responseLength);
153+
154+
assertThat(response.getContentLength()).isEqualTo(responseLength);
155+
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
156+
assertThat(response.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_LENGTH);
157+
}
158+
159+
private static Stream<Named<SetContentLength>> setContentLengthFunctions() {
160+
return Stream.of(
161+
named("setContentLength()", HttpServletResponse::setContentLength),
162+
named("setContentLengthLong()", HttpServletResponse::setContentLengthLong),
163+
named("setIntHeader()", (response, contentLength) -> response.setIntHeader(CONTENT_LENGTH, contentLength)),
164+
named("addIntHeader()", (response, contentLength) -> response.addIntHeader(CONTENT_LENGTH, contentLength)),
165+
named("setHeader()", (response, contentLength) -> response.setHeader(CONTENT_LENGTH, "" + contentLength)),
166+
named("addHeader()", (response, contentLength) -> response.addHeader(CONTENT_LENGTH, "" + contentLength))
167+
);
168+
}
169+
170+
@ParameterizedTest(name = "[{index}] {0}")
171+
@MethodSource("setContentTypeFunctions")
172+
void copyBodyToResponseWithOverridingContentType(SetContentType setContentType) throws Exception {
173+
byte[] responseBody = "Hello World".getBytes(UTF_8);
174+
int responseLength = responseBody.length;
175+
String originalContentType = MediaType.TEXT_PLAIN_VALUE;
176+
String overridingContentType = MediaType.APPLICATION_JSON_VALUE;
177+
178+
MockHttpServletResponse response = new MockHttpServletResponse();
179+
response.setContentType(originalContentType);
180+
181+
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
182+
148183
assertContentTypeHeader(response, originalContentType);
184+
assertContentTypeHeader(responseWrapper, originalContentType);
185+
186+
setContentType.invoke(responseWrapper, overridingContentType);
187+
188+
assertThat(responseWrapper.getContentSize()).isZero();
189+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE);
190+
191+
assertContentTypeHeader(response, overridingContentType);
149192
assertContentTypeHeader(responseWrapper, overridingContentType);
150193

151194
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
152195
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
153196

154197
responseWrapper.copyBodyToResponse();
155198

156-
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
157199
assertThat(responseWrapper.getContentSize()).isZero();
158200
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
159201

@@ -162,24 +204,19 @@ void copyBodyToResponseWithOverridingHeaders(BiConsumer<HttpServletResponse, Str
162204
assertContentTypeHeader(response, overridingContentType);
163205
assertContentTypeHeader(responseWrapper, overridingContentType);
164206

165-
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
166207
assertThat(response.getContentLength()).isEqualTo(responseLength);
167208
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
168209
assertThat(response.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
169210
}
170211

171-
private static Stream<Arguments> setContentTypeFunctions() {
212+
private static Stream<Named<SetContentType>> setContentTypeFunctions() {
172213
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))
214+
named("setContentType()", HttpServletResponse::setContentType),
215+
named("setHeader()", (response, contentType) -> response.setHeader(CONTENT_TYPE, contentType)),
216+
named("addHeader()", (response, contentType) -> response.addHeader(CONTENT_TYPE, contentType))
176217
);
177218
}
178219

179-
private static Arguments namedArguments(String name, BiConsumer<HttpServletResponse, String> setContentTypeFunction) {
180-
return arguments(named(name, setContentTypeFunction));
181-
}
182-
183220
@Test
184221
void copyBodyToResponseWithTransferEncoding() throws Exception {
185222
byte[] responseBody = "6\r\nHello 5\r\nWorld0\r\n\r\n".getBytes(UTF_8);
@@ -219,4 +256,15 @@ private void assertContentTypeHeader(HttpServletResponse response, String conten
219256
assertThat(response.getContentType()).as(CONTENT_TYPE).isEqualTo(contentType);
220257
}
221258

259+
260+
@FunctionalInterface
261+
private interface SetContentLength {
262+
void invoke(HttpServletResponse response, int contentLength);
263+
}
264+
265+
@FunctionalInterface
266+
private interface SetContentType {
267+
void invoke(HttpServletResponse response, String contentType);
268+
}
269+
222270
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import static java.nio.charset.StandardCharsets.UTF_8;
3030
import static org.assertj.core.api.Assertions.assertThat;
31+
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
3132
import static org.springframework.http.MediaType.TEXT_PLAIN_VALUE;
3233

3334
/**
@@ -36,6 +37,7 @@
3637
* @author Arjen Poutsma
3738
* @author Brian Clozel
3839
* @author Juergen Hoeller
40+
* @author Sam Brannen
3941
*/
4042
class ShallowEtagHeaderFilterTests {
4143

@@ -123,7 +125,7 @@ void filterMatch() throws Exception {
123125
assertThat(response.getStatus()).as("Invalid status").isEqualTo(304);
124126
assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\"");
125127
assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse();
126-
assertThat(response.containsHeader("Content-Type")).as("Response has Content-Type header").isFalse();
128+
assertThat(response.getContentType()).as("Invalid Content-Type header").isEqualTo(TEXT_PLAIN_VALUE);
127129
assertThat(response.getContentAsByteArray()).as("Invalid content").isEmpty();
128130
}
129131

@@ -173,11 +175,13 @@ void filterWriter() throws Exception {
173175
void filterWriterWithDisabledCaching() throws Exception {
174176
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels");
175177
MockHttpServletResponse response = new MockHttpServletResponse();
178+
response.setContentType(TEXT_PLAIN_VALUE);
176179

177180
byte[] responseBody = "Hello World".getBytes(UTF_8);
178181
FilterChain filterChain = (filterRequest, filterResponse) -> {
179182
assertThat(filterRequest).as("Invalid request passed").isEqualTo(request);
180183
((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK);
184+
filterResponse.setContentType(APPLICATION_JSON_VALUE);
181185
FileCopyUtils.copy(responseBody, filterResponse.getOutputStream());
182186
};
183187

@@ -186,6 +190,7 @@ void filterWriterWithDisabledCaching() throws Exception {
186190

187191
assertThat(response.getStatus()).isEqualTo(200);
188192
assertThat(response.getHeader("ETag")).isNull();
193+
assertThat(response.getContentType()).as("Invalid Content-Type header").isEqualTo(APPLICATION_JSON_VALUE);
189194
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
190195
}
191196

0 commit comments

Comments
 (0)