From 6dd73ae3e4aec0450220f13e7a652334dae195e1 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 21 Feb 2025 14:23:12 +0100 Subject: [PATCH 1/2] Handle null values in MockHttpServletResponse#setHeader Prior to this commit, `MockHttpServletResponse#setHeader` would not remove the header entry when given a `null` value, as documented in the Servlet API. This commit ensures that this behavior is enforced. Fixes gh-34464 --- .../mock/web/MockHttpServletResponse.java | 7 ++++++- .../mock/web/MockHttpServletResponseTests.java | 12 ++++++++++++ .../testfixture/servlet/MockHttpServletResponse.java | 7 ++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index 592e216763a..38bc1b72d14 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -686,7 +686,12 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void setHeader(String name, @Nullable String value) { - setHeaderValue(name, value); + if (value == null) { + this.headers.remove(name); + } + else { + setHeaderValue(name, value); + } } @Override diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java index 7c066fef0bd..131e0696673 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java @@ -88,6 +88,18 @@ class MockHttpServletResponseTests { assertThat(response.containsHeader(headerName)).isFalse(); } + @ParameterizedTest + @ValueSource(strings = { + CONTENT_TYPE, + CONTENT_LANGUAGE, + "X-Test-Header" + }) + void removeHeaderIfNullValue(String headerName) { + response.addHeader(headerName, "test"); + response.setHeader(headerName, null); + assertThat(response.containsHeader(headerName)).isFalse(); + } + @Test // gh-26493 void setLocaleWithNullValue() { assertThat(response.getLocale()).isEqualTo(Locale.getDefault()); diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java index cbcff8f5736..21db9670176 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java @@ -686,7 +686,12 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public void setHeader(String name, @Nullable String value) { - setHeaderValue(name, value); + if (value == null) { + this.headers.remove(name); + } + else { + setHeaderValue(name, value); + } } @Override From 5a0bd9e5d411d4837294f51660b9659c98b0864a Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 21 Feb 2025 14:33:03 +0100 Subject: [PATCH 2/2] Fix null value support in ContentCachingResponseWrapper Prior to this commit, calling `setHeader` on the response wrapper would have a separate code path for the "Content-Length" header. This did not support calls with `null` values and would result in an exception. This commit ensures that the cached content length value is reset in this case and that the call is forwarded properly to the superclass. Fixes gh-34460 --- .../web/util/ContentCachingResponseWrapper.java | 8 +++++++- .../filter/ContentCachingResponseWrapperTests.java | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java b/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java index 6f150b7547d..ee5ccb55fe4 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java +++ b/spring-web/src/main/java/org/springframework/web/util/ContentCachingResponseWrapper.java @@ -164,7 +164,13 @@ public class ContentCachingResponseWrapper extends HttpServletResponseWrapper { @Override public void setHeader(String name, String value) { if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(name)) { - this.contentLength = toContentLengthInt(Long.parseLong(value)); + if (value != null) { + this.contentLength = toContentLengthInt(Long.parseLong(value)); + } + else { + this.contentLength = null; + super.setHeader(name, null); + } } else { super.setHeader(name, value); diff --git a/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java b/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java index 25ddffbc3b6..5fd7a300c9f 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/ContentCachingResponseWrapperTests.java @@ -270,6 +270,17 @@ class ContentCachingResponseWrapperTests { .withMessageContaining(overflow); } + @Test + void setContentLengthNull() { + MockHttpServletResponse response = new MockHttpServletResponse(); + ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response); + responseWrapper.setContentLength(1024); + responseWrapper.setHeader(CONTENT_LENGTH, null); + + assertThat(response.getHeaderNames()).doesNotContain(CONTENT_LENGTH); + assertThat(responseWrapper.getHeader(CONTENT_LENGTH)).isNull(); + } + private void assertHeader(HttpServletResponse response, String header, int value) { assertHeader(response, header, Integer.toString(value));