Skip to content

Only accept a retry value made up of ASCII digits - #485

Merged
adamw merged 2 commits into
masterfrom
sse-retry-ignore-invalid
Sep 15, 2026
Merged

adamw merged 2 commits into
masterfrom
sse-retry-ignore-invalid

Conversation

@adamw

@adamw adamw commented Sep 15, 2026

Copy link
Copy Markdown
Member

Two things were wrong with how ServerSentEvent.parse reads a retry: line. The spec says the value must be ASCII digits only, and that the field is ignored otherwise.

An unreadable value used to reset retry to None, dropping a value from an earlier line of the same event:

ServerSentEvent.parse(List("retry: 5", "retry: x")).retry  // was None, now Some(5)

A signed value used to parse, so a consumer could get a negative reconnect delay:

ServerSentEvent.parse(List("retry: -1")).retry  // was Some(-1), now None

Both now ignore the line and keep whatever was read before.

A note on the digit check: Integer.parseInt accepts non-ASCII digits, and so does Character.isDigit, so retry: ٥ parsed to Some(5). The check is an explicit '0' to '9' range for that reason. toIntOption is still needed after it, to reject a value too long to fit an Int.

Found while reviewing #478, but unrelated to it.

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.
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.
@adamw adamw changed the title Ignore a retry value that isn't a number Only accept a retry value made up of ASCII digits Sep 15, 2026
@adamw
adamw merged commit 37b4c6f into master Sep 15, 2026
10 checks passed
@adamw
adamw deleted the sse-retry-ignore-invalid branch September 15, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant