Skip to content

Commit ecfa498

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 e2646fe commit ecfa498

4 files changed

Lines changed: 62 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
@@ -21,7 +21,6 @@
2121
import org.springframework.lang.Nullable;
2222
import org.springframework.util.Assert;
2323
import org.springframework.util.ObjectUtils;
24-
import org.springframework.util.StringUtils;
2524

2625
/**
2726
* Representation for a Server-Sent Event for use with Spring's reactive Web support.
@@ -121,7 +120,9 @@ public String format() {
121120
appendAttribute("retry", this.retry.toMillis(), sb);
122121
}
123122
if (this.comment != null) {
124-
sb.append(':').append(StringUtils.replace(this.comment, "\n", "\n:")).append('\n');
123+
sb.append(':');
124+
appendEscaped(this.comment, "\n:", sb);
125+
sb.append('\n');
125126
}
126127
if (this.data != null) {
127128
sb.append("data:");
@@ -133,6 +134,30 @@ private void appendAttribute(String fieldName, Object fieldValue, StringBuilder
133134
sb.append(fieldName).append(':').append(fieldValue).append('\n');
134135
}
135136

137+
private void appendEscaped(String input, String replacement, StringBuilder sb) {
138+
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
139+
sb.append(input);
140+
}
141+
else {
142+
int length = input.length();
143+
for (int i = 0; i < length; i++) {
144+
char c = input.charAt(i);
145+
if (c == '\r') {
146+
if (i + 1 < length && input.charAt(i + 1) == '\n') {
147+
i++;
148+
}
149+
sb.append(replacement);
150+
}
151+
else if (c == '\n') {
152+
sb.append(replacement);
153+
}
154+
else {
155+
sb.append(c);
156+
}
157+
}
158+
}
159+
}
160+
136161
@Override
137162
public boolean equals(@Nullable Object other) {
138163
return (this == other || (other instanceof ServerSentEvent<?> that &&

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ void allowsNullEvent() {
5858
assertThat(event.format()).contains("id:42").doesNotContain("event");
5959
}
6060

61+
@ParameterizedTest(name = "{1}")
62+
@MethodSource("newLineCharacters")
63+
void supportMultiLineComments(String newLine, String description) {
64+
ServerSentEvent<String> event = ServerSentEvent.<String>builder()
65+
.comment("foo" + newLine + "bar" + newLine + "baz").data("payload").build();
66+
assertThat(event.format()).isEqualTo(":foo\n:bar\n:baz\ndata:");
67+
}
68+
6169
private static Stream<Arguments> newLineCharacters() {
6270
return Stream.of(
6371
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
@@ -223,7 +223,9 @@ public SseEventBuilder reconnectTime(long reconnectTimeMillis) {
223223

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

@@ -258,6 +260,16 @@ private void writeStringData(String input, @Nullable MediaType mediaType) {
258260
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
259261
this.dataToSend.add(new DataWithMediaType(input, mediaType));
260262
}
263+
else {
264+
appendEscaped(input, "\ndata:");
265+
saveAppendedText(mediaType);
266+
}
267+
}
268+
269+
private void appendEscaped(String input, String replacement) {
270+
if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) {
271+
append(input);
272+
}
261273
else {
262274
int length = input.length();
263275
for (int i = 0; i < length; i++) {
@@ -266,16 +278,15 @@ private void writeStringData(String input, @Nullable MediaType mediaType) {
266278
if (i + 1 < length && input.charAt(i + 1) == '\n') {
267279
i++;
268280
}
269-
this.sb.append("\ndata:");
281+
append(replacement);
270282
}
271283
else if (c == '\n') {
272-
this.sb.append("\ndata:");
284+
append(replacement);
273285
}
274286
else {
275-
this.sb.append(c);
287+
append(c);
276288
}
277289
}
278-
saveAppendedText(mediaType);
279290
}
280291
}
281292

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)