From 9e2dd6c9515d196e11210ba7cdb3630ae4721e1c Mon Sep 17 00:00:00 2001 From: Adam Warski Date: Tue, 15 Sep 2026 10:53:06 +0000 Subject: [PATCH 1/2] Ignore a retry value that isn't a number Such a line used to set retry back to None, dropping a value parsed from an earlier line of the same event. The spec says to ignore the field instead, so the earlier value is now kept. --- core/src/main/scala/sttp/model/sse/ServerSentEvent.scala | 5 ++++- core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala b/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala index 3919a58..82243e3 100644 --- a/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala +++ b/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala @@ -35,8 +35,11 @@ object ServerSentEvent { event.foldLeft(ServerSentEvent()) { (event, line) => if (line.startsWith("data:")) combineData(event, removeLeadingSpace(line.substring(5))) else if (line.startsWith("id:")) event.copy(id = Some(removeLeadingSpace(line.substring(3)))) + // the spec says to ignore a retry value that isn't a number, so any previously parsed one is kept else if (line.startsWith("retry:")) - event.copy(retry = ParseUtils.toIntOption(removeLeadingSpace(line.substring(6)))) + ParseUtils + .toIntOption(removeLeadingSpace(line.substring(6))) + .fold(event)(retry => event.copy(retry = Some(retry))) else if (line.startsWith("event:")) event.copy(eventType = Some(removeLeadingSpace(line.substring(6)))) else if (line == "data") combineData(event, "") else if (line == "id") event.copy(id = Some("")) diff --git a/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala b/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala index dc36118..1b4d032 100644 --- a/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala +++ b/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala @@ -108,4 +108,8 @@ class ServerSentEventTest extends AnyFlatSpec with Matchers { val sse = ServerSentEvent(Some("a\n")) ServerSentEvent.parse(sse.toString.split("\n").toList) shouldBe sse } + + "parse" should "keep an earlier retry value when a later one is not a number" in { + ServerSentEvent.parse(List("retry: 5", "retry: x")).retry shouldBe Some(5) + } } From 3f9eab5e83fa3ed2f4d1719109dde20db82ca3fd Mon Sep 17 00:00:00 2001 From: Adam Warski Date: Tue, 15 Sep 2026 10:57:02 +0000 Subject: [PATCH 2/2] Accept only ASCII digits as a retry value The spec allows nothing else. A signed value such as `retry: -1` used to parse, giving a negative reconnect delay, and could overwrite a valid value read from an earlier line of the same event. --- .../scala/sttp/model/sse/ServerSentEvent.scala | 15 ++++++++++----- .../sttp/model/sse/ServerSentEventTest.scala | 6 ++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala b/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala index 82243e3..e40b088 100644 --- a/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala +++ b/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala @@ -35,11 +35,7 @@ object ServerSentEvent { event.foldLeft(ServerSentEvent()) { (event, line) => if (line.startsWith("data:")) combineData(event, removeLeadingSpace(line.substring(5))) else if (line.startsWith("id:")) event.copy(id = Some(removeLeadingSpace(line.substring(3)))) - // the spec says to ignore a retry value that isn't a number, so any previously parsed one is kept - else if (line.startsWith("retry:")) - ParseUtils - .toIntOption(removeLeadingSpace(line.substring(6))) - .fold(event)(retry => event.copy(retry = Some(retry))) + else if (line.startsWith("retry:")) combineRetry(event, removeLeadingSpace(line.substring(6))) else if (line.startsWith("event:")) event.copy(eventType = Some(removeLeadingSpace(line.substring(6)))) else if (line == "data") combineData(event, "") else if (line == "id") event.copy(id = Some("")) @@ -48,6 +44,15 @@ object ServerSentEvent { } } + /** The spec accepts only ASCII digits here, and says to ignore the field otherwise - so a value that isn't accepted + * leaves any previously parsed one in place. `toIntOption` is still needed to reject a value too large for an `Int`, + * and `isDigit` would not do instead of the range check: it, like `toIntOption`, accepts non-ASCII digits. + */ + private def combineRetry(event: ServerSentEvent, newRetry: String): ServerSentEvent = + if (newRetry.nonEmpty && newRetry.forall(c => c >= '0' && c <= '9')) + ParseUtils.toIntOption(newRetry).fold(event)(retry => event.copy(retry = Some(retry))) + else event + private def combineData(event: ServerSentEvent, newData: String): ServerSentEvent = { event match { case e @ ServerSentEvent(Some(oldData), _, _, _) => e.copy(data = Some(s"$oldData\n$newData")) diff --git a/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala b/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala index 1b4d032..1451f6c 100644 --- a/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala +++ b/core/src/test/scala/sttp/model/sse/ServerSentEventTest.scala @@ -112,4 +112,10 @@ class ServerSentEventTest extends AnyFlatSpec with Matchers { "parse" should "keep an earlier retry value when a later one is not a number" in { ServerSentEvent.parse(List("retry: 5", "retry: x")).retry shouldBe Some(5) } + + "parse" should "ignore a retry value that is not made up of ASCII digits" in { + ServerSentEvent.parse(List("retry: -1")).retry shouldBe None + ServerSentEvent.parse(List("retry: +5")).retry shouldBe None + ServerSentEvent.parse(List("retry: ٥")).retry shouldBe None + } }