Browse Source

Honor Content-[Type|Length] headers from wrapped response again

Commit 375e0e6827 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
pull/32357/head
Sam Brannen 2 years ago
parent
commit
ca602ef874
  1. 37
      spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java
  2. 138
      spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java

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

@ -21,10 +21,10 @@ import java.io.InputStream;
import java.io.OutputStreamWriter; import java.io.OutputStreamWriter;
import java.io.PrintWriter; import java.io.PrintWriter;
import java.io.UnsupportedEncodingException; import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.LinkedHashSet;
import java.util.Set;
import jakarta.servlet.ServletOutputStream; import jakarta.servlet.ServletOutputStream;
import jakarta.servlet.WriteListener; import jakarta.servlet.WriteListener;
@ -43,6 +43,7 @@ import org.springframework.util.FastByteArrayOutputStream;
* <p>Used e.g. by {@link org.springframework.web.filter.ShallowEtagHeaderFilter}. * <p>Used e.g. by {@link org.springframework.web.filter.ShallowEtagHeaderFilter}.
* *
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Sam Brannen
* @since 4.1.3 * @since 4.1.3
* @see ContentCachingRequestWrapper * @see ContentCachingRequestWrapper
*/ */
@ -157,16 +158,19 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
@Override @Override
@Nullable @Nullable
public String getContentType() { public String getContentType() {
return this.contentType; if (this.contentType != null) {
return this.contentType;
}
return super.getContentType();
} }
@Override @Override
public boolean containsHeader(String name) { public boolean containsHeader(String name) {
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
return this.contentLength != null; return true;
} }
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) { else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
return this.contentType != null; return true;
} }
else { else {
return super.containsHeader(name); return super.containsHeader(name);
@ -222,10 +226,10 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
@Override @Override
@Nullable @Nullable
public String getHeader(String name) { public String getHeader(String name) {
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
return (this.contentLength != null) ? this.contentLength.toString() : null; return this.contentLength.toString();
} }
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) { else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
return this.contentType; return this.contentType;
} }
else { else {
@ -235,12 +239,11 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
@Override @Override
public Collection<String> getHeaders(String name) { public Collection<String> getHeaders(String name) {
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { if (this.contentLength != null && HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) {
return this.contentLength != null ? Collections.singleton(this.contentLength.toString()) : return Collections.singleton(this.contentLength.toString());
Collections.emptySet();
} }
else if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) { else if (this.contentType != null && HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
return this.contentType != null ? Collections.singleton(this.contentType) : Collections.emptySet(); return Collections.singleton(this.contentType);
} }
else { else {
return super.getHeaders(name); return super.getHeaders(name);
@ -251,7 +254,7 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
public Collection<String> getHeaderNames() { public Collection<String> getHeaderNames() {
Collection<String> headerNames = super.getHeaderNames(); Collection<String> headerNames = super.getHeaderNames();
if (this.contentLength != null || this.contentType != null) { if (this.contentLength != null || this.contentType != null) {
List<String> result = new ArrayList<>(headerNames); Set<String> result = new LinkedHashSet<>(headerNames);
if (this.contentLength != null) { if (this.contentLength != null) {
result.add(HttpHeaders.CONTENT_LENGTH); result.add(HttpHeaders.CONTENT_LENGTH);
} }
@ -330,7 +333,7 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper {
} }
this.contentLength = null; this.contentLength = null;
} }
if (complete || this.contentType != null) { if (this.contentType != null) {
rawResponse.setContentType(this.contentType); rawResponse.setContentType(this.contentType);
this.contentType = null; this.contentType = null;
} }

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

@ -16,21 +16,32 @@
package org.springframework.web.filter; package org.springframework.web.filter;
import java.util.function.BiConsumer;
import java.util.stream.Stream;
import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponse;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.springframework.http.MediaType;
import org.springframework.util.FileCopyUtils; import org.springframework.util.FileCopyUtils;
import org.springframework.web.testfixture.servlet.MockHttpServletResponse; import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
import org.springframework.web.util.ContentCachingResponseWrapper; import org.springframework.web.util.ContentCachingResponseWrapper;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Named.named;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; import static org.springframework.http.HttpHeaders.CONTENT_LENGTH;
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING; import static org.springframework.http.HttpHeaders.TRANSFER_ENCODING;
/** /**
* Unit tests for {@link ContentCachingResponseWrapper}. * Unit tests for {@link ContentCachingResponseWrapper}.
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Sam Brannen
*/ */
public class ContentCachingResponseWrapperTests { public class ContentCachingResponseWrapperTests {
@ -49,6 +60,124 @@ public class ContentCachingResponseWrapperTests {
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody); assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
} }
@Test
void copyBodyToResponseWithPresetHeaders() throws Exception {
String PUZZLE = "puzzle";
String ENIGMA = "enigma";
String NUMBER = "number";
String MAGIC = "42";
byte[] responseBody = "Hello World".getBytes(UTF_8);
int responseLength = responseBody.length;
int originalContentLength = 999;
String contentType = MediaType.APPLICATION_JSON_VALUE;
MockHttpServletResponse response = new MockHttpServletResponse();
response.setContentType(contentType);
response.setContentLength(originalContentLength);
response.setHeader(PUZZLE, ENIGMA);
response.setIntHeader(NUMBER, 42);
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(responseWrapper.getContentSize()).isZero();
assertThat(responseWrapper.getHeaderNames())
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
assertHeader(responseWrapper, PUZZLE, ENIGMA);
assertHeader(responseWrapper, NUMBER, MAGIC);
assertHeader(responseWrapper, CONTENT_LENGTH, originalContentLength);
assertContentTypeHeader(responseWrapper, contentType);
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
responseWrapper.copyBodyToResponse();
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(responseWrapper.getContentSize()).isZero();
assertThat(responseWrapper.getHeaderNames())
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
assertHeader(responseWrapper, PUZZLE, ENIGMA);
assertHeader(responseWrapper, NUMBER, MAGIC);
assertHeader(responseWrapper, CONTENT_LENGTH, responseLength);
assertContentTypeHeader(responseWrapper, contentType);
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(response.getContentLength()).isEqualTo(responseLength);
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
assertThat(response.getHeaderNames())
.containsExactlyInAnyOrder(PUZZLE, NUMBER, CONTENT_TYPE, CONTENT_LENGTH);
assertHeader(response, PUZZLE, ENIGMA);
assertHeader(response, NUMBER, MAGIC);
assertHeader(response, CONTENT_LENGTH, responseLength);
assertContentTypeHeader(response, contentType);
}
@ParameterizedTest(name = "[{index}] {0}")
@MethodSource("setContentTypeFunctions")
void copyBodyToResponseWithOverridingHeaders(BiConsumer<HttpServletResponse, String> setContentType) throws Exception {
byte[] responseBody = "Hello World".getBytes(UTF_8);
int responseLength = responseBody.length;
int originalContentLength = 11;
int overridingContentLength = 22;
String originalContentType = MediaType.TEXT_PLAIN_VALUE;
String overridingContentType = MediaType.APPLICATION_JSON_VALUE;
MockHttpServletResponse response = new MockHttpServletResponse();
response.setContentLength(originalContentLength);
response.setContentType(originalContentType);
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
responseWrapper.setStatus(HttpServletResponse.SC_CREATED);
responseWrapper.setContentLength(overridingContentLength);
setContentType.accept(responseWrapper, overridingContentType);
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(responseWrapper.getContentSize()).isZero();
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
assertHeader(response, CONTENT_LENGTH, originalContentLength);
assertHeader(responseWrapper, CONTENT_LENGTH, overridingContentLength);
assertContentTypeHeader(response, originalContentType);
assertContentTypeHeader(responseWrapper, overridingContentType);
FileCopyUtils.copy(responseBody, responseWrapper.getOutputStream());
assertThat(responseWrapper.getContentSize()).isEqualTo(responseLength);
responseWrapper.copyBodyToResponse();
assertThat(responseWrapper.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(responseWrapper.getContentSize()).isZero();
assertThat(responseWrapper.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
assertHeader(response, CONTENT_LENGTH, responseLength);
assertHeader(responseWrapper, CONTENT_LENGTH, responseLength);
assertContentTypeHeader(response, overridingContentType);
assertContentTypeHeader(responseWrapper, overridingContentType);
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_CREATED);
assertThat(response.getContentLength()).isEqualTo(responseLength);
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
assertThat(response.getHeaderNames()).containsExactlyInAnyOrder(CONTENT_TYPE, CONTENT_LENGTH);
}
private static Stream<Arguments> setContentTypeFunctions() {
return Stream.of(
namedArguments("setContentType()", HttpServletResponse::setContentType),
namedArguments("setHeader()", (response, contentType) -> response.setHeader(CONTENT_TYPE, contentType)),
namedArguments("addHeader()", (response, contentType) -> response.addHeader(CONTENT_TYPE, contentType))
);
}
private static Arguments namedArguments(String name, BiConsumer<HttpServletResponse, String> setContentTypeFunction) {
return arguments(named(name, setContentTypeFunction));
}
@Test @Test
void copyBodyToResponseWithTransferEncoding() throws Exception { void copyBodyToResponseWithTransferEncoding() throws Exception {
byte[] responseBody = "6\r\nHello 5\r\nWorld0\r\n\r\n".getBytes(UTF_8); byte[] responseBody = "6\r\nHello 5\r\nWorld0\r\n\r\n".getBytes(UTF_8);
@ -66,6 +195,10 @@ public class ContentCachingResponseWrapperTests {
assertThat(response.getContentAsByteArray()).isEqualTo(responseBody); assertThat(response.getContentAsByteArray()).isEqualTo(responseBody);
} }
private void assertHeader(HttpServletResponse response, String header, int value) {
assertHeader(response, header, Integer.toString(value));
}
private void assertHeader(HttpServletResponse response, String header, String value) { private void assertHeader(HttpServletResponse response, String header, String value) {
if (value == null) { if (value == null) {
assertThat(response.containsHeader(header)).as(header).isFalse(); assertThat(response.containsHeader(header)).as(header).isFalse();
@ -79,4 +212,9 @@ public class ContentCachingResponseWrapperTests {
} }
} }
private void assertContentTypeHeader(HttpServletResponse response, String contentType) {
assertHeader(response, CONTENT_TYPE, contentType);
assertThat(response.getContentType()).as(CONTENT_TYPE).isEqualTo(contentType);
}
} }

Loading…
Cancel
Save