diff --git a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java index f2af79ab2ca..e77f3b8a08b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java @@ -20,8 +20,8 @@ import java.time.Duration; import org.jspecify.annotations.Nullable; -import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.web.util.SseUtils; /** * Representation for a Server-Sent Event for use with Spring's reactive Web support. @@ -112,7 +112,7 @@ public final class ServerSentEvent { } if (this.comment != null) { sb.append(':'); - appendEscaped(this.comment, "\n:", sb); + SseUtils.appendFieldValue("", this.comment, sb); sb.append('\n'); } if (this.data != null) { @@ -125,30 +125,6 @@ public final class ServerSentEvent { sb.append(fieldName).append(':').append(fieldValue).append('\n'); } - private void appendEscaped(String input, String replacement, StringBuilder sb) { - if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { - sb.append(input); - } - else { - int length = input.length(); - for (int i = 0; i < length; i++) { - char c = input.charAt(i); - if (c == '\r') { - if (i + 1 < length && input.charAt(i + 1) == '\n') { - i++; - } - sb.append(replacement); - } - else if (c == '\n') { - sb.append(replacement); - } - else { - sb.append(c); - } - } - } - } - @Override public boolean equals(@Nullable Object other) { return (this == other || (other instanceof ServerSentEvent that && @@ -265,23 +241,18 @@ public final class ServerSentEvent { @Override public Builder id(String id) { - checkEvent(id); + SseUtils.assertNoLineSeparator(id); this.id = id; return this; } @Override public Builder event(String event) { - checkEvent(event); + SseUtils.assertNoLineSeparator(event); this.event = event; return this; } - private static void checkEvent(String content) { - Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, - "illegal character '\\n' or '\\r' in event content"); - } - @Override public Builder retry(Duration retry) { this.retry = retry; diff --git a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java index 24b6e46207b..8e7f02caaa4 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java @@ -40,6 +40,7 @@ import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.web.util.SseUtils; /** * {@code HttpMessageWriter} for {@code "text/event-stream"} responses. @@ -142,27 +143,7 @@ public class ServerSentEventHttpMessageWriter implements HttpMessageWriterServer-Sent Events, + * shared by the Servlet and Reactive SSE support. + * + * @author Brian Clozel + * @since 7.0.9 + */ +public abstract class SseUtils { + + /** + * Append {@code value} to {@code output}, replacing each line separator + * ({@code "\n"}, {@code "\r"}, or {@code "\r\n"}) it contains with a new + * {@code field} line (that is, {@code "\n" + field + ":"}). This keeps a + * multi-line field value from breaking out of the current SSE field when + * written on the wire. + * @param field the name of the SSE field that {@code value} belongs to + * (for example, {@code "data"}), or an empty string for a comment + * @param value the field value to escape and append + * @param output the {@code StringBuilder} to append the escaped value to + */ + public static void appendFieldValue(String field, String value, StringBuilder output) { + if (value.indexOf('\n') == -1 && value.indexOf('\r') == -1) { + output.append(value); + return; + } + String lineSeparatorReplacement = "\n" + field + ":"; + int length = value.length(); + for (int i = 0; i < length; i++) { + char c = value.charAt(i); + if (c == '\r') { + if (i + 1 < length && value.charAt(i + 1) == '\n') { + i++; + } + output.append(lineSeparatorReplacement); + } + else if (c == '\n') { + output.append(lineSeparatorReplacement); + } + else { + output.append(c); + } + } + } + + /** + * Assert that the given single-line SSE field value, such as an + * {@code id} or {@code event} name, does not contain a line separator. + * @param content the field value to check + * @throws IllegalArgumentException if {@code content} contains {@code "\n"} or {@code "\r"} + */ + public static void assertNoLineSeparator(String content) { + Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, + "illegal character '\\n' or '\\r' in event content"); + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java b/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java new file mode 100644 index 00000000000..94a649a0e2d --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.util; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link SseUtils}. + * @author Brian Clozel + */ +class SseUtilsTests { + + @Test + void appendFieldValueWithoutLineSeparatorAppendsAsIs() { + StringBuilder sb = new StringBuilder(); + sb.append("data:"); + SseUtils.appendFieldValue("data", "no newlines here", sb); + assertThat(sb).hasToString("data:no newlines here"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void appendFieldValueReplacesLineSeparatorWithFieldPrefix(String newLine, String description) { + StringBuilder sb = new StringBuilder(); + sb.append("data:"); + SseUtils.appendFieldValue("data", "first" + newLine + "second", sb); + assertThat(sb).hasToString("data:first\ndata:second"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void appendFieldValueUsesEmptyFieldForComments(String newLine, String description) { + StringBuilder sb = new StringBuilder(); + sb.append(":"); + SseUtils.appendFieldValue("", "first" + newLine + "second", sb); + assertThat(sb).hasToString(":first\n:second"); + } + + @Test + void assertNoLineSeparatorAcceptsPlainContent() { + SseUtils.assertNoLineSeparator("no newlines here"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void assertNoLineSeparatorRejectsLineSeparator(String newLine, String description) { + assertThatIllegalArgumentException().isThrownBy(() -> + SseUtils.assertNoLineSeparator("first" + newLine + "second")); + } + + static Stream newLineCharacters() { + return Stream.of( + Arguments.of("\n", "LF"), + Arguments.of("\r", "CR"), + Arguments.of("\r\n", "CRLF") + ); + } + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index 9c11846c9c3..b0a7c0e41df 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -64,6 +64,7 @@ import org.springframework.web.reactive.result.HandlerResultHandlerSupport; import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.SseUtils; /** * {@code HandlerResultHandler} that encapsulates the view resolution algorithm @@ -603,8 +604,9 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp finally { DataBufferUtils.release(buffer); } - text = escapeSseFragment(text); - return bufferFactory.wrap(text.getBytes(charset)); + StringBuilder escaped = new StringBuilder(); + SseUtils.appendFieldValue("data", text, escaped); + return bufferFactory.wrap(escaped.toString().getBytes(charset)); }); return Flux.concat(Flux.just(prefix), content, Flux.just(suffix)); @@ -615,29 +617,6 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp return bufferFactory.wrap(bytes); } - private String escapeSseFragment(String content) { - if (content.indexOf('\n') == -1 && content.indexOf('\r') == -1) { - return content; - } - StringBuilder fragment = new StringBuilder(); - int length = content.length(); - for (int i = 0; i < length; i++) { - char c = content.charAt(i); - if (c == '\r') { - if (i + 1 < length && content.charAt(i + 1) == '\n') { - i++; - } - fragment.append("\ndata:"); - } - else if (c == '\n') { - fragment.append("\ndata:"); - } - else { - fragment.append(c); - } - } - return fragment.toString(); - } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java index 1ca6712f64a..52f6b6f6523 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java @@ -42,6 +42,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.util.SseUtils; /** * Implementation of {@link ServerResponse} for sending @@ -149,33 +150,35 @@ final class SseServerResponse extends AbstractServerResponse { @Override public SseBuilder id(String id) { Assert.hasLength(id, "Id must not be empty"); - return field("id", id); + SseUtils.assertNoLineSeparator(id); + this.builder.append("id:").append(id).append('\n'); + return this; } @Override public SseBuilder event(String eventName) { Assert.hasLength(eventName, "Name must not be empty"); - return field("event", eventName); + SseUtils.assertNoLineSeparator(eventName); + this.builder.append("event:").append(eventName).append('\n'); + return this; } @Override public SseBuilder retry(Duration duration) { Assert.notNull(duration, "Duration must not be null"); - String millis = Long.toString(duration.toMillis()); - return field("retry", millis); + this.builder.append("retry:").append(duration.toMillis()).append('\n'); + return this; } @Override public SseBuilder comment(String comment) { - String[] lines = comment.split("\n"); - for (String line : lines) { - field("", line); - } - return this; + return field("", comment); } private SseBuilder field(String name, String value) { - this.builder.append(name).append(':').append(value).append('\n'); + this.builder.append(name).append(':'); + SseUtils.appendFieldValue(name, value, this.builder); + this.builder.append('\n'); return this; } @@ -191,10 +194,7 @@ final class SseServerResponse extends AbstractServerResponse { } private void writeString(String string) throws IOException { - String[] lines = string.split("\n"); - for (String line : lines) { - field("data", line); - } + field("data", string); this.send(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java index 1c5cd5dd2a3..159fe369fd0 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java @@ -66,6 +66,7 @@ import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; import org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver; import org.springframework.web.servlet.view.FragmentsRendering; +import org.springframework.web.util.SseUtils; /** * Handler for return values of type: @@ -476,26 +477,8 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur public byte[] getFragmentContent() { this.writer.flush(); String content = this.outputStream.toString(this.charset); - if (content.indexOf('\n') == -1 && content.indexOf('\r') == -1) { - return content.getBytes(this.charset); - } StringBuilder fragment = new StringBuilder(); - int length = content.length(); - for (int i = 0; i < length; i++) { - char c = content.charAt(i); - if (c == '\r') { - if (i + 1 < length && content.charAt(i + 1) == '\n') { - i++; - } - fragment.append("\ndata:"); - } - else if (c == '\n') { - fragment.append("\ndata:"); - } - else { - fragment.append(c); - } - } + SseUtils.appendFieldValue("data", content, fragment); return fragment.toString().getBytes(this.charset); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java index f0bbe9cd9b4..42ba09c6855 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java @@ -27,10 +27,10 @@ import org.jspecify.annotations.Nullable; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.server.ServerHttpResponse; -import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.util.SseUtils; /** * A specialization of {@link ResponseBodyEmitter} for sending @@ -203,14 +203,14 @@ public class SseEmitter extends ResponseBodyEmitter { @Override public SseEventBuilder id(String id) { - checkEvent(id); + SseUtils.assertNoLineSeparator(id); append("id:").append(id).append('\n'); return this; } @Override public SseEventBuilder name(String name) { - checkEvent(name); + SseUtils.assertNoLineSeparator(name); this.hasName = true; append("event:").append(name).append('\n'); return this; @@ -225,7 +225,7 @@ public class SseEmitter extends ResponseBodyEmitter { @Override public SseEventBuilder comment(String comment) { append(':'); - appendEscaped(comment, "\n:"); + SseUtils.appendFieldValue("", comment, this.sb); append('\n'); return this; } @@ -252,45 +252,16 @@ public class SseEmitter extends ResponseBodyEmitter { return this; } - private static void checkEvent(String content) { - Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, - "illegal character '\\n' or '\\r' in event content"); - } - private void writeStringData(String input, @Nullable MediaType mediaType) { if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { this.dataToSend.add(new DataWithMediaType(input, mediaType)); } else { - appendEscaped(input, "\ndata:"); + SseUtils.appendFieldValue("data", input, this.sb); saveAppendedText(mediaType); } } - private void appendEscaped(String input, String replacement) { - if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { - append(input); - } - else { - int length = input.length(); - for (int i = 0; i < length; i++) { - char c = input.charAt(i); - if (c == '\r') { - if (i + 1 < length && input.charAt(i + 1) == '\n') { - i++; - } - append(replacement); - } - else if (c == '\n') { - append(replacement); - } - else { - append(c); - } - } - } - } - SseEventBuilderImpl append(String text) { this.sb.append(text); return this; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java index 964238abe81..b95da1ab4e9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java @@ -212,6 +212,47 @@ class SseServerResponseTests { assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); } + @Test + void sendStringWithCarriageReturn() throws Exception { + String body = "line1\rline2\r\nline3"; + ServerResponse response = ServerResponse.sse(sse -> { + try { + sse.send(body); + } + catch (IOException ex) { + throw new UncheckedIOException(ex); + } + }); + + ServerResponse.Context context = Collections::emptyList; + + ModelAndView mav = response.writeTo(this.mockRequest, this.mockResponse, context); + assertThat(mav).isNull(); + + String expected = "data:line1\ndata:line2\ndata:line3\n\n"; + assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); + } + + @Test + void commentWithCarriageReturn() throws Exception { + ServerResponse response = ServerResponse.sse(sse -> { + try { + sse.comment("line1\rline2").send(); + } + catch (IOException ex) { + throw new UncheckedIOException(ex); + } + }); + + ServerResponse.Context context = Collections::emptyList; + + ModelAndView mav = response.writeTo(this.mockRequest, this.mockResponse, context); + assertThat(mav).isNull(); + + String expected = ":line1\n:line2\n\n"; + assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); + } + @Test // gh-34608 void sendHeartbeat() throws Exception { ServerResponse response = ServerResponse.sse(sse -> {