mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 08:24:13 +00:00
Ignore invalid SSE retry field
Per the SSE specification, a "retry" field whose value is not made up solely of ASCII digits must be ignored. ServerSentEventHttpMessageReader passed the value straight to Long.parseLong, so "retry:none", an empty "retry:", or a value too large for a long raised NumberFormatException and terminated the event stream. A client cannot control what a server sends, so an unusable reconnection hint would kill an otherwise healthy subscription. Signed-off-by: Artyom Tsvirko <36863599+lArtiquel@users.noreply.github.com>
This commit is contained in:
committed by
Brian Clozel
parent
60b9f4cd3a
commit
adbc8ceeab
+32
-1
@@ -163,7 +163,10 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
|
||||
sseBuilder.event(line.substring(6).trim());
|
||||
}
|
||||
else if (line.startsWith("retry:")) {
|
||||
sseBuilder.retry(Duration.ofMillis(Long.parseLong(line.substring(6).trim())));
|
||||
Long retry = parseRetry(line.substring(6).trim());
|
||||
if (retry != null) {
|
||||
sseBuilder.retry(Duration.ofMillis(retry));
|
||||
}
|
||||
}
|
||||
else if (line.startsWith(":")) {
|
||||
comment = (comment != null ? comment : new StringBuilder());
|
||||
@@ -188,6 +191,34 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the value of a {@code retry} field, which is to be ignored unless it
|
||||
* consists solely of ASCII digits and fits into a {@code long}.
|
||||
* @return the reconnection time in milliseconds, or {@code null} to ignore the field
|
||||
* @see <a href="https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation">
|
||||
* HTML Living Standard: interpreting an event stream</a>
|
||||
*/
|
||||
private static @Nullable Long parseRetry(String value) {
|
||||
// Long#parseLong is more lenient than the spec: it accepts a leading sign and
|
||||
// non-ASCII digits, so "-1", "+5000" and "\u0665\u0660\u0660\u0660" would all parse.
|
||||
if (value.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
for (int i = 0; i < value.length(); i++) {
|
||||
char ch = value.charAt(i);
|
||||
if (ch < '0' || ch > '9') {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
try {
|
||||
return Long.parseLong(value);
|
||||
}
|
||||
catch (NumberFormatException ex) {
|
||||
// Too large for a long: ignore the field rather than failing the stream.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private @Nullable Object decodeData(StringBuilder data, ResolvableType dataType, Map<String, Object> hints) {
|
||||
if (String.class == dataType.resolve()) {
|
||||
return data.substring(0, data.length() - 1);
|
||||
|
||||
+28
@@ -137,6 +137,34 @@ class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingTests {
|
||||
.verify();
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("rawtypes")
|
||||
void ignoreInvalidRetry() {
|
||||
MockServerHttpRequest request = MockServerHttpRequest.post("/")
|
||||
.body(Mono.just(stringBuffer(
|
||||
"retry:none\ndata:foo\n\n" +
|
||||
"retry:\ndata:bar\n\n" +
|
||||
"retry:-1\ndata:baz\n\n" +
|
||||
"retry:+5000\ndata:qux\n\n" +
|
||||
// Arabic-Indic digits: accepted by Long#parseLong, not by the spec.
|
||||
"retry:\u0665\u0660\u0660\u0660\ndata:quux\n\n" +
|
||||
"retry:99999999999999999999\ndata:corge\n\n")));
|
||||
|
||||
Flux<ServerSentEvent> events = this.reader
|
||||
.read(ResolvableType.forClassWithGenerics(ServerSentEvent.class, String.class),
|
||||
request, Collections.emptyMap()).cast(ServerSentEvent.class);
|
||||
|
||||
StepVerifier.create(events)
|
||||
.expectNext(ServerSentEvent.builder().data("foo").build())
|
||||
.expectNext(ServerSentEvent.builder().data("bar").build())
|
||||
.expectNext(ServerSentEvent.builder().data("baz").build())
|
||||
.expectNext(ServerSentEvent.builder().data("qux").build())
|
||||
.expectNext(ServerSentEvent.builder().data("quux").build())
|
||||
.expectNext(ServerSentEvent.builder().data("corge").build())
|
||||
.expectComplete()
|
||||
.verify();
|
||||
}
|
||||
|
||||
@Test // gh-35412
|
||||
void emptyLines() {
|
||||
MockServerHttpRequest request = MockServerHttpRequest.post("/")
|
||||
|
||||
Reference in New Issue
Block a user