Skip to content

Commit 86a68a7

Browse files
committed
Support multi-line comments in Server Sent Events
Prior to this commit, comments sent with Server Sent Events could break the wire format when sent over the network when comments contained line breaks. While comments are mainly used for sending keepalive messages, they can also be used for sending debug data. This commit ensures that line breaks are properly handled in comments. Fixes gh-36866
1 parent fe27ad2 commit 86a68a7

4 files changed

Lines changed: 64 additions & 7 deletions

File tree

spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.springframework.util.Assert;
2424
import org.springframework.util.ObjectUtils;
25-
import org.springframework.util.StringUtils;
2625

2726
/**
2827
* Representation for a Server-Sent Event for use with Spring's reactive Web support.
@@ -112,7 +111,9 @@ public String format() {
112111
appendAttribute("retry", this.retry.toMillis(), sb);
113112
}
114113
if (this.comment != null) {
115-
sb.append(':').append(StringUtils.replace(this.comment, "\n", "\n:")).append('\n');
114+
sb.append(':');
115+
appendEscaped(this.comment, "\n:", sb);
116+
sb.append('\n');
116117
}
117118
if (this.data != null) {
118119
sb.append("data:");
@@ -124,6 +125,30 @@ private void appendAttribute(String fieldName, Object fieldValue, StringBuilder
124125
sb.append(fieldName).append(':').append(fieldValue).append('\n');
125126
}
126127

128+
private void appendEscaped(String input, String replacement, StringBuilder sb) {
129+
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
130+
sb.append(input);
131+
}
132+
else {
133+
int length = input.length();
134+
for (int i = 0; i < length; i++) {
135+
char c = input.charAt(i);
136+
if (c == '\r') {
137+
if (i + 1 < length && input.charAt(i + 1) == '\n') {
138+
i++;
139+
}
140+
sb.append(replacement);
141+
}
142+
else if (c == '\n') {
143+
sb.append(replacement);
144+
}
145+
else {
146+
sb.append(c);
147+
}
148+
}
149+
}
150+
}
151+
127152
@Override
128153
public boolean equals(@Nullable Object other) {
129154
return (this == other || (other instanceof ServerSentEvent<?> that &&

spring-web/src/test/java/org/springframework/http/codec/ServerSentEventTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.junit.jupiter.params.provider.Arguments;
2323
import org.junit.jupiter.params.provider.MethodSource;
2424

25+
import static org.assertj.core.api.Assertions.assertThat;
2526
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
2627

2728
/**
@@ -44,6 +45,15 @@ void rejectsInvalidEvent(String newLine, String description) {
4445
ServerSentEvent.<String>builder().event("first" + newLine + "second").build());
4546
}
4647

48+
49+
@ParameterizedTest(name = "{1}")
50+
@MethodSource("newLineCharacters")
51+
void supportMultiLineComments(String newLine, String description) {
52+
ServerSentEvent<String> event = ServerSentEvent.<String>builder()
53+
.comment("foo" + newLine + "bar" + newLine + "baz").data("payload").build();
54+
assertThat(event.format()).isEqualTo(":foo\n:bar\n:baz\ndata:");
55+
}
56+
4757
private static Stream<Arguments> newLineCharacters() {
4858
return Stream.of(
4959
Arguments.of("\n", "LF"),

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ public SseEventBuilder reconnectTime(long reconnectTimeMillis) {
224224

225225
@Override
226226
public SseEventBuilder comment(String comment) {
227-
append(':').append(StringUtils.replace(comment, "\n", "\n:")).append('\n');
227+
append(':');
228+
appendEscaped(comment, "\n:");
229+
append('\n');
228230
return this;
229231
}
230232

@@ -259,6 +261,16 @@ private void writeStringData(String input, @Nullable MediaType mediaType) {
259261
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
260262
this.dataToSend.add(new DataWithMediaType(input, mediaType));
261263
}
264+
else {
265+
appendEscaped(input, "\ndata:");
266+
saveAppendedText(mediaType);
267+
}
268+
}
269+
270+
private void appendEscaped(String input, String replacement) {
271+
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
272+
append(input);
273+
}
262274
else {
263275
int length = input.length();
264276
for (int i = 0; i < length; i++) {
@@ -267,16 +279,15 @@ private void writeStringData(String input, @Nullable MediaType mediaType) {
267279
if (i + 1 < length && input.charAt(i + 1) == '\n') {
268280
i++;
269281
}
270-
this.sb.append("\ndata:");
282+
append(replacement);
271283
}
272284
else if (c == '\n') {
273-
this.sb.append("\ndata:");
285+
append(replacement);
274286
}
275287
else {
276-
this.sb.append(c);
288+
append(c);
277289
}
278290
}
279-
saveAppendedText(mediaType);
280291
}
281292
}
282293

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitterTests.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,17 @@ void rejectInvalidName(String newLineChars, String description) {
168168
.send(event().name("first" + newLineChars + "second")));
169169
}
170170

171+
@ParameterizedTest(name = "{1}")
172+
@MethodSource("newLineCharacters")
173+
void supportMultiLineComments(String newLineChars, String description) throws Exception {
174+
this.emitter.send(event().comment("foo" + newLineChars + "bar" + newLineChars + "baz").data("payload"));
175+
this.handler.assertSentObjectCount(3);
176+
this.handler.assertObject(0, ":foo\n:bar\n:baz\ndata:", TEXT_PLAIN_UTF8);
177+
this.handler.assertObject(1, "payload");
178+
this.handler.assertObject(2, "\n\n", TEXT_PLAIN_UTF8);
179+
this.handler.assertWriteCount(1);
180+
}
181+
171182
private static Stream<Arguments> newLineCharacters() {
172183
return Stream.of(
173184
Arguments.of("\n", "LF"),

0 commit comments

Comments
 (0)