Skip to content

Commit f0e71a8

Browse files
authored
Merge pull request #18990 from rwinch/7.0.x-gh-18970-null-oncommitted
Merge Handle null value in OnCommittedResponseWrapper header methods
2 parents 671a53e + 2848b95 commit f0e71a8

2 files changed

Lines changed: 137 additions & 21 deletions

File tree

web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import jakarta.servlet.WriteListener;
2525
import jakarta.servlet.http.HttpServletResponse;
2626
import jakarta.servlet.http.HttpServletResponseWrapper;
27+
import org.jspecify.annotations.Nullable;
2728

2829
/**
2930
* Base class for response wrappers which encapsulate the logic for handling an event when
@@ -57,37 +58,37 @@ public OnCommittedResponseWrapper(HttpServletResponse response) {
5758
}
5859

5960
@Override
60-
public void addHeader(String name, String value) {
61+
public void addHeader(@Nullable String name, @Nullable String value) {
6162
checkContentLengthHeader(name, value);
6263
super.addHeader(name, value);
6364
}
6465

6566
@Override
66-
public void addIntHeader(String name, int value) {
67+
public void addIntHeader(@Nullable String name, int value) {
6768
checkContentLengthHeader(name, value);
6869
super.addIntHeader(name, value);
6970
}
7071

7172
@Override
72-
public void setHeader(String name, String value) {
73+
public void setHeader(@Nullable String name, @Nullable String value) {
7374
checkContentLengthHeader(name, value);
7475
super.setHeader(name, value);
7576
}
7677

7778
@Override
78-
public void setIntHeader(String name, int value) {
79+
public void setIntHeader(@Nullable String name, int value) {
7980
checkContentLengthHeader(name, value);
8081
super.setIntHeader(name, value);
8182
}
8283

83-
private void checkContentLengthHeader(String name, int value) {
84+
private void checkContentLengthHeader(@Nullable String name, int value) {
8485
if ("Content-Length".equalsIgnoreCase(name)) {
8586
setContentLength(value);
8687
}
8788
}
8889

89-
private void checkContentLengthHeader(String name, String value) {
90-
if ("Content-Length".equalsIgnoreCase(name)) {
90+
private void checkContentLengthHeader(@Nullable String name, @Nullable String value) {
91+
if (value != null && "Content-Length".equalsIgnoreCase(name)) {
9192
setContentLength(Long.parseLong(value));
9293
}
9394
}
@@ -149,7 +150,7 @@ public final void sendError(int sc) throws IOException {
149150
* before calling the superclass <code>sendError()</code>
150151
*/
151152
@Override
152-
public final void sendError(int sc, String msg) throws IOException {
153+
public final void sendError(int sc, @Nullable String msg) throws IOException {
153154
doOnResponseCommitted();
154155
super.sendError(sc, msg);
155156
}
@@ -159,7 +160,7 @@ public final void sendError(int sc, String msg) throws IOException {
159160
* before calling the superclass <code>sendRedirect()</code>
160161
*/
161162
@Override
162-
public final void sendRedirect(String location) throws IOException {
163+
public final void sendRedirect(@Nullable String location) throws IOException {
163164
doOnResponseCommitted();
164165
super.sendRedirect(location);
165166
}
@@ -206,7 +207,7 @@ private void trackContentLength(char content) {
206207
}
207208
}
208209

209-
private void trackContentLength(Object content) {
210+
private void trackContentLength(@Nullable Object content) {
210211
if (!this.disableOnCommitted) {
211212
trackContentLength(String.valueOf(content));
212213
}
@@ -248,7 +249,7 @@ private void trackContentLengthLn() {
248249
}
249250
}
250251

251-
private void trackContentLength(String content) {
252+
private void trackContentLength(@Nullable String content) {
252253
if (!this.disableOnCommitted) {
253254
int contentLength = (content != null) ? content.length() : 4;
254255
checkContentLength(contentLength);
@@ -355,7 +356,7 @@ public void write(String s, int off, int len) {
355356
}
356357

357358
@Override
358-
public void write(String s) {
359+
public void write(@Nullable String s) {
359360
trackContentLength(s);
360361
this.delegate.write(s);
361362
}
@@ -403,13 +404,13 @@ public void print(char[] s) {
403404
}
404405

405406
@Override
406-
public void print(String s) {
407+
public void print(@Nullable String s) {
407408
trackContentLength(s);
408409
this.delegate.print(s);
409410
}
410411

411412
@Override
412-
public void print(Object obj) {
413+
public void print(@Nullable Object obj) {
413414
trackContentLength(obj);
414415
this.delegate.print(obj);
415416
}
@@ -470,14 +471,14 @@ public void println(char[] x) {
470471
}
471472

472473
@Override
473-
public void println(String x) {
474+
public void println(@Nullable String x) {
474475
trackContentLength(x);
475476
trackContentLengthLn();
476477
this.delegate.println(x);
477478
}
478479

479480
@Override
480-
public void println(Object x) {
481+
public void println(@Nullable Object x) {
481482
trackContentLength(x);
482483
trackContentLengthLn();
483484
this.delegate.println(x);
@@ -504,13 +505,13 @@ public PrintWriter format(Locale l, String format, Object... args) {
504505
}
505506

506507
@Override
507-
public PrintWriter append(CharSequence csq) {
508-
checkContentLength(csq.length());
508+
public PrintWriter append(@Nullable CharSequence csq) {
509+
checkContentLength((csq != null) ? csq.length() : 4);
509510
return this.delegate.append(csq);
510511
}
511512

512513
@Override
513-
public PrintWriter append(CharSequence csq, int start, int end) {
514+
public PrintWriter append(@Nullable CharSequence csq, int start, int end) {
514515
checkContentLength(end - start);
515516
return this.delegate.append(csq, start, end);
516517
}
@@ -595,7 +596,7 @@ public void print(long l) throws IOException {
595596
}
596597

597598
@Override
598-
public void print(String s) throws IOException {
599+
public void print(@Nullable String s) throws IOException {
599600
trackContentLength(s);
600601
this.delegate.print(s);
601602
}
@@ -649,7 +650,7 @@ public void println(long l) throws IOException {
649650
}
650651

651652
@Override
652-
public void println(String s) throws IOException {
653+
public void println(@Nullable String s) throws IOException {
653654
trackContentLength(s);
654655
trackContentLengthLn();
655656
this.delegate.println(s);

web/src/test/java/org/springframework/security/web/util/OnCommittedResponseWrapperTests.java

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import org.mockito.Mock;
2929
import org.mockito.junit.jupiter.MockitoExtension;
3030

31+
import org.springframework.http.HttpHeaders;
32+
3133
import static org.assertj.core.api.Assertions.assertThat;
34+
import static org.assertj.core.api.Assertions.assertThatNoException;
3235
import static org.mockito.BDDMockito.given;
3336
import static org.mockito.Mockito.verify;
3437

@@ -1006,6 +1009,16 @@ public void addHeaderContentLengthPrintWriterWriteStringCommits() throws Excepti
10061009
assertThat(this.committed).isTrue();
10071010
}
10081011

1012+
@Test
1013+
public void addHeaderNullNameDoesNotThrow() {
1014+
assertThatNoException().isThrownBy(() -> this.response.addHeader(null, "value"));
1015+
}
1016+
1017+
@Test
1018+
public void addHeaderNullValueDoesNotThrow() {
1019+
assertThatNoException().isThrownBy(() -> this.response.addHeader(HttpHeaders.CONTENT_LENGTH, null));
1020+
}
1021+
10091022
@Test
10101023
public void addIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exception {
10111024
givenGetWriterThenReturn();
@@ -1015,6 +1028,11 @@ public void addIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exce
10151028
assertThat(this.committed).isTrue();
10161029
}
10171030

1031+
@Test
1032+
public void addIntHeaderNullNameDoesNotThrow() {
1033+
assertThatNoException().isThrownBy(() -> this.response.addIntHeader(null, 1));
1034+
}
1035+
10181036
@Test
10191037
public void setHeaderContentLengthPrintWriterWriteStringCommits() throws Exception {
10201038
givenGetWriterThenReturn();
@@ -1024,6 +1042,16 @@ public void setHeaderContentLengthPrintWriterWriteStringCommits() throws Excepti
10241042
assertThat(this.committed).isTrue();
10251043
}
10261044

1045+
@Test
1046+
public void setHeaderNullNameDoesNotThrow() {
1047+
assertThatNoException().isThrownBy(() -> this.response.setHeader(null, "value"));
1048+
}
1049+
1050+
@Test
1051+
public void setHeaderNullValueDoesNotThrow() {
1052+
assertThatNoException().isThrownBy(() -> this.response.setHeader(HttpHeaders.CONTENT_LENGTH, null));
1053+
}
1054+
10271055
@Test
10281056
public void setIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exception {
10291057
givenGetWriterThenReturn();
@@ -1033,6 +1061,11 @@ public void setIntHeaderContentLengthPrintWriterWriteStringCommits() throws Exce
10331061
assertThat(this.committed).isTrue();
10341062
}
10351063

1064+
@Test
1065+
public void setIntHeaderNullNameDoesNotThrow() {
1066+
assertThatNoException().isThrownBy(() -> this.response.setIntHeader(null, 1));
1067+
}
1068+
10361069
@Test
10371070
public void bufferSizePrintWriterWriteCommits() throws Exception {
10381071
givenGetWriterThenReturn();
@@ -1054,4 +1087,86 @@ public void bufferSizeCommitsOnce() throws Exception {
10541087
assertThat(this.committed).isFalse();
10551088
}
10561089

1090+
@Test
1091+
public void printWriterPrintNullStringDoesNotThrow() throws Exception {
1092+
givenGetWriterThenReturn();
1093+
String s = null;
1094+
assertThatNoException().isThrownBy(() -> this.response.getWriter().print(s));
1095+
verify(this.writer).print(s);
1096+
}
1097+
1098+
@Test
1099+
public void printWriterPrintlnNullStringDoesNotThrow() throws Exception {
1100+
givenGetWriterThenReturn();
1101+
String s = null;
1102+
assertThatNoException().isThrownBy(() -> this.response.getWriter().println(s));
1103+
verify(this.writer).println(s);
1104+
}
1105+
1106+
@Test
1107+
public void printWriterPrintNullObjectDoesNotThrow() throws Exception {
1108+
givenGetWriterThenReturn();
1109+
Object obj = null;
1110+
assertThatNoException().isThrownBy(() -> this.response.getWriter().print(obj));
1111+
verify(this.writer).print(obj);
1112+
}
1113+
1114+
@Test
1115+
public void printWriterPrintlnNullObjectDoesNotThrow() throws Exception {
1116+
givenGetWriterThenReturn();
1117+
Object obj = null;
1118+
assertThatNoException().isThrownBy(() -> this.response.getWriter().println(obj));
1119+
verify(this.writer).println(obj);
1120+
}
1121+
1122+
@Test
1123+
public void printWriterWriteNullStringDoesNotThrow() throws Exception {
1124+
givenGetWriterThenReturn();
1125+
String s = null;
1126+
assertThatNoException().isThrownBy(() -> this.response.getWriter().write(s));
1127+
verify(this.writer).write(s);
1128+
}
1129+
1130+
@Test
1131+
public void printWriterAppendNullCharSequenceDoesNotThrow() throws Exception {
1132+
givenGetWriterThenReturn();
1133+
CharSequence csq = null;
1134+
assertThatNoException().isThrownBy(() -> this.response.getWriter().append(csq));
1135+
verify(this.writer).append(csq);
1136+
}
1137+
1138+
@Test
1139+
public void printWriterAppendNullCharSequenceIntIntDoesNotThrow() throws Exception {
1140+
givenGetWriterThenReturn();
1141+
CharSequence csq = null;
1142+
assertThatNoException().isThrownBy(() -> this.response.getWriter().append(csq, 0, 3));
1143+
verify(this.writer).append(csq, 0, 3);
1144+
}
1145+
1146+
@Test
1147+
public void outputStreamPrintNullStringDoesNotThrow() throws Exception {
1148+
givenGetOutputStreamThenReturn();
1149+
String s = null;
1150+
assertThatNoException().isThrownBy(() -> this.response.getOutputStream().print(s));
1151+
verify(this.out).print(s);
1152+
}
1153+
1154+
@Test
1155+
public void outputStreamPrintlnNullStringDoesNotThrow() throws Exception {
1156+
givenGetOutputStreamThenReturn();
1157+
String s = null;
1158+
assertThatNoException().isThrownBy(() -> this.response.getOutputStream().println(s));
1159+
verify(this.out).println(s);
1160+
}
1161+
1162+
@Test
1163+
public void sendErrorWithNullMsgDoesNotThrow() throws Exception {
1164+
assertThatNoException().isThrownBy(() -> this.response.sendError(400, null));
1165+
}
1166+
1167+
@Test
1168+
public void sendRedirectWithNullLocationDoesNotThrow() throws Exception {
1169+
assertThatNoException().isThrownBy(() -> this.response.sendRedirect(null));
1170+
}
1171+
10571172
}

0 commit comments

Comments
 (0)