From 4c24fd93ea709a18eea9e03540695dd8485031c2 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 28 Jun 2026 03:58:02 +0300 Subject: [PATCH] chore: dedupe SSE optional-space strip and simplify retry/grow helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three behavior-preserving cleanups in ServerSentEventReader, all internal: - Fold the WHATWG SSE §9.2.6 "drop one optional leading space" rule, which the comment path and the field-value path each implemented separately, into a single private valueFrom(line, start) helper. valueFrom(line, 1) reproduces the old substring(1).removePrefix(" ") for every input and drops its extra intermediate allocation. - Replace the hand-rolled base-10 parse + manual overflow check in parseRetryMillis with toLongOrNull behind an explicit ASCII-digit guard. The guard (not a filter) preserves spec semantics: signs and embedded non-digits are still rejected, empty input still yields null, and toLongOrNull rejects values past Long.MAX_VALUE at the identical boundary. Removes the now-unused DECIMAL_BASE constant. - Drop the unused minCapacity parameter and unreachable else arm from ByteArrayBuilder.grow. Its sole caller always passes bytes.size + 1, so the growth reduces to "first allocation is INITIAL_CAP, otherwise double" with the same 0 -> 64 -> 128 -> ... sequence. Output stays byte-identical; the existing exact-equality test suite is unchanged apart from refreshing one test comment that named a now-removed internal branch. --- .../core/http/sse/ServerSentEventReader.kt | 61 +++++++++---------- .../http/sse/ServerSentEventReaderTest.kt | 4 +- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt index 409af37d..0c8c2db6 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt @@ -77,7 +77,7 @@ public class ServerSentEventReader(private val source: BufferedSource) { // Comment line: latest wins. Per WHATWG SSE §9.2.6 the optional single // leading space after the `:` is stripped (same rule applied to field values). if (line[0] == ':') { - comment = line.substring(1).removePrefix(" ") + comment = valueFrom(line, 1) hasField = true continue } @@ -91,14 +91,7 @@ public class ServerSentEventReader(private val source: BufferedSource) { rawValue = "" } else { field = line.substring(0, colon) - // Per spec: if the value starts with U+0020 SPACE, drop one. - val afterColon = colon + 1 - rawValue = - if (afterColon < line.length && line[afterColon] == ' ') { - line.substring(afterColon + 1) - } else { - line.substring(afterColon) - } + rawValue = valueFrom(line, colon + 1) } when (field) { @@ -200,18 +193,31 @@ public class ServerSentEventReader(private val source: BufferedSource) { * representable range. The spec is silent on overflow; ignoring is conservative. */ private fun parseRetryMillis(value: String): Long? { - if (value.isEmpty()) return null - var result = 0L - for (c in value) { - if (c !in '0'..'9') return null - val digit = (c - '0').toLong() - // Detect overflow before it happens so we don't wrap around. - if (result > (Long.MAX_VALUE - digit) / DECIMAL_BASE) return null - result = result * DECIMAL_BASE + digit - } - return result + // Spec allows ASCII digits only; this guard rejects signs and any other + // non-digit up front (toLongOrNull would otherwise accept a leading +/-). + // toLongOrNull then parses base-10 and returns null on empty input or on + // values past Long.MAX_VALUE — the same overflow boundary as the former + // hand-rolled loop. + if (value.any { it !in '0'..'9' }) return null + return value.toLongOrNull() } + /** + * Returns the value portion of [line] starting at [start], dropping a single + * leading U+0020 SPACE if present. WHATWG SSE §9.2.6 applies this optional + * single-space strip to both comment text and field values, so both code + * paths share this one implementation. + */ + private fun valueFrom( + line: String, + start: Int, + ): String = + if (start < line.length && line[start] == ' ') { + line.substring(start + 1) + } else { + line.substring(start) + } + private companion object { private const val LF: Byte = 0x0A private const val CR: Byte = 0x0D @@ -221,9 +227,6 @@ public class ServerSentEventReader(private val source: BufferedSource) { // Initial capacity for the per-event `data` line accumulator. SSE servers tend to // emit small numbers of data lines per event (one is typical); 4 covers the long tail. private const val DATA_ACCUMULATOR_INITIAL_CAP = 4 - - // Numeric base for `retry:` digit parsing — SSE retry values are unsigned decimals. - private const val DECIMAL_BASE = 10L } /** @@ -237,20 +240,16 @@ public class ServerSentEventReader(private val source: BufferedSource) { private var count: Int = 0 fun append(b: Byte) { - if (count == bytes.size) grow(count + 1) + if (count == bytes.size) grow() bytes[count++] = b } fun toUtf8(): String = if (count == 0) "" else String(bytes, 0, count, Charsets.UTF_8) - private fun grow(minCapacity: Int) { - val oldCap = bytes.size - val newCap = - when { - oldCap == 0 -> INITIAL_CAP - oldCap < minCapacity -> maxOf(oldCap * 2, minCapacity) - else -> oldCap - } + // Called only when the array is full (count == bytes.size), so the first + // allocation is INITIAL_CAP and every later growth simply doubles. + private fun grow() { + val newCap = if (bytes.isEmpty()) INITIAL_CAP else bytes.size * 2 bytes = bytes.copyOf(newCap) } diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReaderTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReaderTest.kt index 31d63155..3f13d116 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReaderTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReaderTest.kt @@ -489,8 +489,8 @@ class ServerSentEventReaderTest { @Test fun `retry value one above Long MAX VALUE is rejected via overflow guard`() { - // 9223372036854775808 — Long.MAX_VALUE + 1, exercises the - // `result > (Long.MAX_VALUE - digit) / 10` branch in parseRetryMillis. + // 9223372036854775808 — Long.MAX_VALUE + 1; one past the representable + // boundary, so parseRetryMillis rejects it rather than wrapping. val src = source("retry: 9223372036854775808\ndata: x\n\n") val event = ServerSentEventReader(src).next() assertNull(event?.retry, "value exceeding Long.MAX_VALUE must be rejected")