Skip to content

Commit cca440e

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 Closes gh-32317
1 parent 497aa3c commit cca440e

File tree

3 files changed

+78
-66
lines changed

3 files changed

+78
-66
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
@@ -60,9 +60,6 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
6060
@Nullable
6161
private Integer contentLength;
6262

63-
@Nullable
64-
private String contentType;
65-
6663

6764
/**
6865
* Create a new ContentCachingResponseWrapper for the given servlet response.
@@ -150,28 +147,11 @@ public void setContentLengthLong(long len) {
150147
setContentLength((int) len);
151148
}
152149

153-
@Override
154-
public void setContentType(@Nullable String type) {
155-
this.contentType = type;
156-
}
157-
158-
@Override
159-
@Nullable
160-
public String getContentType() {
161-
if (this.contentType != null) {
162-
return this.contentType;
163-
}
164-
return super.getContentType();
165-
}
166-
167150
@Override
168151
public boolean containsHeader(String name) {
169152
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
170153
return true;
171154
}
172-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
173-
return true;
174-
}
175155
else {
176156
return super.containsHeader(name);
177157
}
@@ -182,9 +162,6 @@ public void setHeader(String name, String value) {
182162
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
183163
this.contentLength = Integer.valueOf(value);
184164
}
185-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
186-
this.contentType = value;
187-
}
188165
else {
189166
super.setHeader(name, value);
190167
}
@@ -195,9 +172,6 @@ public void addHeader(String name, String value) {
195172
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
196173
this.contentLength = Integer.valueOf(value);
197174
}
198-
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
199-
this.contentType = value;
200-
}
201175
else {
202176
super.addHeader(name, value);
203177
}
@@ -229,9 +203,6 @@ public String getHeader(String name) {
229203
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
230204
return this.contentLength.toString();
231205
}
232-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
233-
return this.contentType;
234-
}
235206
else {
236207
return super.getHeader(name);
237208
}
@@ -242,9 +213,6 @@ public Collection<String> getHeaders(String name) {
242213
if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
243214
return Collections.singleton(this.contentLength.toString());
244215
}
245-
else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
246-
return Collections.singleton(this.contentType);
247-
}
248216
else {
249217
return super.getHeaders(name);
250218
}
@@ -253,14 +221,9 @@ else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(n
253221
@Override
254222
public Collection<String> getHeaderNames() {
255223
Collection<String> headerNames = super.getHeaderNames();
256-
if (this.contentLength != null || this.contentType != null) {
224+
if (this.contentLength != null) {
257225
Set<String> result = new LinkedHashSet<>(headerNames);
258-
if (this.contentLength != null) {
259-
result.add(HttpHeaders.CONTENT_LENGTH);
260-
}
261-
if (this.contentType != null) {
262-
result.add(HttpHeaders.CONTENT_TYPE);
263-
}
226+
result.add(HttpHeaders.CONTENT_LENGTH);
264227
return result;
265228
}
266229
else {
@@ -333,10 +296,6 @@ protected void copyBodyToResponse(boolean complete) throws IOException {
333296
}
334297
this.contentLength = null;
335298
}
336-
if (this.contentType != null) {
337-
rawResponse.setContentType(this.contentType);
338-
this.contentType = null;
339-
}
340299
}
341300
this.content.writeTo(rawResponse.getOutputStream());
342301
this.content.reset();

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

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,12 @@
1616

1717
package org.springframework.web.filter;
1818

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

2221
import jakarta.servlet.http.HttpServletResponse;
22+
import org.junit.jupiter.api.Named;
2323
import org.junit.jupiter.api.Test;
2424
import org.junit.jupiter.params.ParameterizedTest;
25-
import org.junit.jupiter.params.provider.Arguments;
2625
import org.junit.jupiter.params.provider.MethodSource;
2726

2827
import org.springframework.http.MediaType;
@@ -33,7 +32,6 @@
3332
import static java.nio.charset.StandardCharsets.UTF_8;
3433
import static org.assertj.core.api.Assertions.assertThat;
3534
import static org.junit.jupiter.api.Named.named;
36-
import static org.junit.jupiter.params.provider.Arguments.arguments;
3735
import static org.springframework.http.HttpHeaders.CONTENT_LENGTH;
3836
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
3937
import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING;
@@ -120,39 +118,83 @@ void copyBodyToResponseWithPresetHeaders() throws Exception {
120118
}
121119

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

132128
MockHttpServletResponse response = new MockHttpServletResponse();
133129
response.setContentLength(originalContentLength);
134-
response.setContentType(originalContentType);
135130

136131
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
137-
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
138132
responseWrapper.setContentLength(overridingContentLength);
139-
setContentType.accept(responseWrapper, overridingContentType);
140133

141-
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
134+
setContentLength.invoke(responseWrapper, overridingContentLength);
135+
142136
assertThat(responseWrapper.getContentSize()).isZero();
143-
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
137+
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_LENGTH);
144138

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

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

153196
responseWrapper.copyBodyToResponse();
154197

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

@@ -161,24 +203,19 @@ void copyBodyToResponseWithOverridingHeaders(BiConsumer<HttpServletResponse, Str
161203
assertContentTypeHeader(response, overridingContentType);
162204
assertContentTypeHeader(responseWrapper, overridingContentType);
163205

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

170-
private static Stream<Arguments> setContentTypeFunctions() {
211+
private static Stream<Named<SetContentType>> setContentTypeFunctions() {
171212
return Stream.of(
172-
namedArguments("setContentType()", HttpServletResponse::setContentType),
173-
namedArguments("setHeader()", (response, contentType) -> response.setHeader(CONTENT_TYPE, contentType)),
174-
namedArguments("addHeader()", (response, contentType) -> response.addHeader(CONTENT_TYPE, contentType))
213+
named("setContentType()", HttpServletResponse::setContentType),
214+
named("setHeader()", (response, contentType) -> response.setHeader(CONTENT_TYPE, contentType)),
215+
named("addHeader()", (response, contentType) -> response.addHeader(CONTENT_TYPE, contentType))
175216
);
176217
}
177218

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

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

‎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
public 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 @@ public 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)