From 0039bc0cf0b41d0c4153accf7bbff73082ced1a2 Mon Sep 17 00:00:00 2001 From: Robert Winch <362503+rwinch@users.noreply.github.com> Date: Thu, 26 Mar 2026 09:28:06 -0500 Subject: [PATCH] Handle null value in OnCommittedResponseWrapper header methods Closes gh-18970 --- .../web/util/OnCommittedResponseWrapper.java | 4 +- .../util/OnCommittedResponseWrapperTests.java | 115 ++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java b/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java index 6d0f576d76..14f1a3663f 100644 --- a/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java +++ b/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java @@ -87,7 +87,7 @@ public abstract class OnCommittedResponseWrapper extends HttpServletResponseWrap } private void checkContentLengthHeader(String name, String value) { - if ("Content-Length".equalsIgnoreCase(name)) { + if (value != null && "Content-Length".equalsIgnoreCase(name)) { setContentLength(Long.parseLong(value)); } } @@ -505,7 +505,7 @@ public abstract class OnCommittedResponseWrapper extends HttpServletResponseWrap @Override public PrintWriter append(CharSequence csq) { - checkContentLength(csq.length()); + checkContentLength((csq != null) ? csq.length() : 4); return this.delegate.append(csq); } diff --git a/web/src/test/java/org/springframework/security/web/util/OnCommittedResponseWrapperTests.java b/web/src/test/java/org/springframework/security/web/util/OnCommittedResponseWrapperTests.java index bbdb50b20c..94002ee650 100644 --- a/web/src/test/java/org/springframework/security/web/util/OnCommittedResponseWrapperTests.java +++ b/web/src/test/java/org/springframework/security/web/util/OnCommittedResponseWrapperTests.java @@ -28,7 +28,10 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpHeaders; + import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; @@ -1006,6 +1009,16 @@ public class OnCommittedResponseWrapperTests { assertThat(this.committed).isTrue(); } + @Test + public void addHeaderNullNameDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.addHeader(null, "value")); + } + + @Test + public void addHeaderNullValueDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.addHeader(HttpHeaders.CONTENT_LENGTH, null)); + } + @Test public void addIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exception { givenGetWriterThenReturn(); @@ -1015,6 +1028,11 @@ public class OnCommittedResponseWrapperTests { assertThat(this.committed).isTrue(); } + @Test + public void addIntHeaderNullNameDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.addIntHeader(null, 1)); + } + @Test public void setHeaderContentLengthPrintWriterWriteStringCommits() throws Exception { givenGetWriterThenReturn(); @@ -1024,6 +1042,16 @@ public class OnCommittedResponseWrapperTests { assertThat(this.committed).isTrue(); } + @Test + public void setHeaderNullNameDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.setHeader(null, "value")); + } + + @Test + public void setHeaderNullValueDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.setHeader(HttpHeaders.CONTENT_LENGTH, null)); + } + @Test public void setIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exception { givenGetWriterThenReturn(); @@ -1033,6 +1061,11 @@ public class OnCommittedResponseWrapperTests { assertThat(this.committed).isTrue(); } + @Test + public void setIntHeaderNullNameDoesNotThrow() { + assertThatNoException().isThrownBy(() -> this.response.setIntHeader(null, 1)); + } + @Test public void bufferSizePrintWriterWriteCommits() throws Exception { givenGetWriterThenReturn(); @@ -1054,4 +1087,86 @@ public class OnCommittedResponseWrapperTests { assertThat(this.committed).isFalse(); } + @Test + public void printWriterPrintNullStringDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + String s = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().print(s)); + verify(this.writer).print(s); + } + + @Test + public void printWriterPrintlnNullStringDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + String s = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().println(s)); + verify(this.writer).println(s); + } + + @Test + public void printWriterPrintNullObjectDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + Object obj = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().print(obj)); + verify(this.writer).print(obj); + } + + @Test + public void printWriterPrintlnNullObjectDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + Object obj = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().println(obj)); + verify(this.writer).println(obj); + } + + @Test + public void printWriterWriteNullStringDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + String s = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().write(s)); + verify(this.writer).write(s); + } + + @Test + public void printWriterAppendNullCharSequenceDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + CharSequence csq = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().append(csq)); + verify(this.writer).append(csq); + } + + @Test + public void printWriterAppendNullCharSequenceIntIntDoesNotThrow() throws Exception { + givenGetWriterThenReturn(); + CharSequence csq = null; + assertThatNoException().isThrownBy(() -> this.response.getWriter().append(csq, 0, 3)); + verify(this.writer).append(csq, 0, 3); + } + + @Test + public void outputStreamPrintNullStringDoesNotThrow() throws Exception { + givenGetOutputStreamThenReturn(); + String s = null; + assertThatNoException().isThrownBy(() -> this.response.getOutputStream().print(s)); + verify(this.out).print(s); + } + + @Test + public void outputStreamPrintlnNullStringDoesNotThrow() throws Exception { + givenGetOutputStreamThenReturn(); + String s = null; + assertThatNoException().isThrownBy(() -> this.response.getOutputStream().println(s)); + verify(this.out).println(s); + } + + @Test + public void sendErrorWithNullMsgDoesNotThrow() throws Exception { + assertThatNoException().isThrownBy(() -> this.response.sendError(400, null)); + } + + @Test + public void sendRedirectWithNullLocationDoesNotThrow() throws Exception { + assertThatNoException().isThrownBy(() -> this.response.sendRedirect(null)); + } + }