From b55ce9ec1843d667e0cf44eb195554ae7f64e3ff Mon Sep 17 00:00:00 2001 From: Optio Agent Date: Mon, 13 Jul 2026 19:28:47 +0000 Subject: [PATCH] Bound the flat-ring draw walk so an odd-length ring can't overrun MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The equirectangular land renderer (`MapScreen.drawWorldLand`), the azimuthal land renderer (`MapScreen.drawAzimuthalLand`), and the POTA / QSO-path map renderers (`PotaActivationMap.buildRingPath`, `QsoSheet.buildRingPath`) all walked a flattened `[lon, lat, lon, lat, ...]` polygon ring with `while (i < ring.size) { ...ring[i + 1]...; i += 2 }`. That reads one element past the end of an odd-length ring, throwing ArrayIndexOutOfBoundsException on the Compose draw thread (uncaught → whole-app crash). Two siblings in the same codebase already avoid this by bounding to `ring.size / 2` (`WorldOutlines.pointInRing`, `CarMapSurfaceRenderer.buildPaths`); the four draw loops were the outliers. Root cause: the pair walk trusted `ring.size` to be even. Today every ring comes from `WorldOutlines.ringToFlat`, which allocates `FloatArray(n * 2)` (always even), so the overrun is latent — but the four live draw paths have no guard against a malformed/regenerated basemap ring, unlike their bounded siblings. Fix: extract a single `forEachRingVertex` helper (next to the flat-ring format docs and `pointInRing` in WorldOutlines) that walks exactly `ring.size / 2` complete vertices, dropping a trailing unpaired coordinate, and skips rings with fewer than 3 vertices (equivalent to the old `if (ring.size < 6) continue`). All four draw loops now call it. `inline` keeps the per-vertex callback allocation-free on the draw hot path. Byte-identical output for every even-length ring. Test: RingVertexWalkTest (pure JVM, JUnit4 + Truth) covers even/odd/short/empty rings, the first-vertex flag, and a reproduction of the pre-fix unbounded walk that asserts it overruns an odd ring while the bounded walk does not. Verified: :app:testDebugUnitTest and :app:assembleDebug (4 ABIs) both green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../radio/ks3ckc/ft8af/ui/decode/QsoSheet.kt | 13 +-- .../radio/ks3ckc/ft8af/ui/map/MapScreen.kt | 29 ++--- .../ks3ckc/ft8af/ui/map/WorldOutlines.kt | 31 +++++ .../ks3ckc/ft8af/ui/pota/PotaActivationMap.kt | 13 +-- .../ks3ckc/ft8af/ui/map/RingVertexWalkTest.kt | 106 ++++++++++++++++++ 5 files changed, 156 insertions(+), 36 deletions(-) create mode 100644 ft8af/app/src/test/kotlin/radio/ks3ckc/ft8af/ui/map/RingVertexWalkTest.kt diff --git a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/decode/QsoSheet.kt b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/decode/QsoSheet.kt index fb31b09b0..98575672d 100644 --- a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/decode/QsoSheet.kt +++ b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/decode/QsoSheet.kt @@ -65,6 +65,7 @@ import radio.ks3ckc.ft8af.ui.components.QsoStatus import radio.ks3ckc.ft8af.ui.components.StatusPill import radio.ks3ckc.ft8af.ui.map.UsStateOutlines import radio.ks3ckc.ft8af.ui.map.WorldOutlines +import radio.ks3ckc.ft8af.ui.map.forEachRingVertex import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -1015,14 +1016,12 @@ private fun buildRingPath(rings: List, proj: QsoPathProjection): Pat val path = Path() for (off in doubleArrayOf(-360.0, 0.0, 360.0)) { for (ring in rings) { - if (ring.size < 6) continue - path.moveTo(proj.projectX(ring[0].toDouble() + off), proj.projectY(ring[1].toDouble())) - var i = 2 - while (i < ring.size) { - path.lineTo(proj.projectX(ring[i].toDouble() + off), proj.projectY(ring[i + 1].toDouble())) - i += 2 + val plotted = forEachRingVertex(ring) { lon, lat, first -> + val x = proj.projectX(lon.toDouble() + off) + val y = proj.projectY(lat.toDouble()) + if (first) path.moveTo(x, y) else path.lineTo(x, y) } - path.close() + if (plotted) path.close() } } return path diff --git a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/MapScreen.kt b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/MapScreen.kt index 0c0397f42..3d8fbc704 100644 --- a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/MapScreen.kt +++ b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/MapScreen.kt @@ -1347,14 +1347,10 @@ private fun DrawScope.drawWorldLand(rings: List, vp: EquirectViewpor val path = Path() for (ring in rings) { - if (ring.size < 6) continue // need at least 3 points for a polygon - path.moveTo(px(ring[0]), py(ring[1])) - var i = 2 - while (i < ring.size) { - path.lineTo(px(ring[i]), py(ring[i + 1])) - i += 2 + val plotted = forEachRingVertex(ring) { lon, lat, first -> + if (first) path.moveTo(px(lon), py(lat)) else path.lineTo(px(lon), py(lat)) } - path.close() + if (plotted) path.close() } drawPath(path, color = Color(0x4094A3B8)) // fill drawPath(path, color = Color(0x9094A3B8), style = Stroke(width = 0.75f)) // outline @@ -1377,24 +1373,13 @@ private fun DrawScope.drawAzimuthalLand( ) { val land = Path() for (ring in rings) { - if (ring.size < 6) continue - var first = true - var i = 0 - while (i < ring.size) { - val lon = ring[i].toDouble() - val lat = ring[i + 1].toDouble() - val proj = azProject(opLat, opLon, lat, lon) + val plotted = forEachRingVertex(ring) { lonF, latF, first -> + val proj = azProject(opLat, opLon, latF.toDouble(), lonF.toDouble()) val px = cx + proj.x * r * scale + panX val py = cy + proj.y * r * scale + panY - if (first) { - land.moveTo(px, py) - first = false - } else { - land.lineTo(px, py) - } - i += 2 + if (first) land.moveTo(px, py) else land.lineTo(px, py) } - land.close() + if (plotted) land.close() } // Clip to the disc so anything that strays past the horizon (or wraps // weirdly near the antipode) is hidden. diff --git a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/WorldOutlines.kt b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/WorldOutlines.kt index 6e49e4c66..6ce7b5527 100644 --- a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/WorldOutlines.kt +++ b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/map/WorldOutlines.kt @@ -44,6 +44,37 @@ private fun ringToFlat(ring: JSONArray): FloatArray { return out } +/** + * Invokes [plot] once per complete `(lon, lat)` vertex of a flattened polygon + * [ring] (`[lon0, lat0, lon1, lat1, ...]`), passing `first == true` on the + * opening vertex so callers can `moveTo` it and `lineTo` the rest. Returns + * `true` when at least one vertex was plotted (i.e. the caller should + * `close()` the path), or `false` when the ring has fewer than [minVertices] + * complete vertices and was skipped entirely. + * + * The vertex count is bounded to `ring.size / 2`, so a trailing unpaired + * coordinate in a malformed odd-length ring is ignored rather than read out of + * bounds. This centralizes the safe bound already used by [pointInRing] and + * `CarMapSurfaceRenderer.buildPaths`, replacing the `while (i < ring.size)` + * walks (which read `ring[i + 1]` one past the end of an odd-length ring) in + * the equirectangular / azimuthal / POTA / QSO-path land renderers. + * + * `inline` keeps the per-vertex [plot] call allocation-free on the draw hot + * path (this runs for every land ring, every frame). + */ +internal inline fun forEachRingVertex( + ring: FloatArray, + minVertices: Int = 3, + plot: (lon: Float, lat: Float, first: Boolean) -> Unit, +): Boolean { + val vertices = ring.size / 2 + if (vertices < minVertices) return false + for (v in 0 until vertices) { + plot(ring[v * 2], ring[v * 2 + 1], v == 0) + } + return true +} + /** * Lazily loads Natural Earth 110m land outlines from res/raw/world_land.json, * caching the parsed rings for the process lifetime. diff --git a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/pota/PotaActivationMap.kt b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/pota/PotaActivationMap.kt index 9b2b9b058..d056dcdc9 100644 --- a/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/pota/PotaActivationMap.kt +++ b/ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/pota/PotaActivationMap.kt @@ -29,6 +29,7 @@ import radio.ks3ckc.ft8af.theme.BgSurface import radio.ks3ckc.ft8af.theme.Signal import radio.ks3ckc.ft8af.ui.map.UsStateOutlines import radio.ks3ckc.ft8af.ui.map.WorldOutlines +import radio.ks3ckc.ft8af.ui.map.forEachRingVertex /** * Compact equirectangular map auto-zoomed to frame the operator's location and @@ -152,14 +153,12 @@ private fun buildRingPath(rings: List, proj: PotaActivationProjectio val path = Path() for (off in doubleArrayOf(-360.0, 0.0, 360.0)) { for (ring in rings) { - if (ring.size < 6) continue - path.moveTo(proj.projectX(ring[0].toDouble() + off), proj.projectY(ring[1].toDouble())) - var i = 2 - while (i < ring.size) { - path.lineTo(proj.projectX(ring[i].toDouble() + off), proj.projectY(ring[i + 1].toDouble())) - i += 2 + val plotted = forEachRingVertex(ring) { lon, lat, first -> + val x = proj.projectX(lon.toDouble() + off) + val y = proj.projectY(lat.toDouble()) + if (first) path.moveTo(x, y) else path.lineTo(x, y) } - path.close() + if (plotted) path.close() } } return path diff --git a/ft8af/app/src/test/kotlin/radio/ks3ckc/ft8af/ui/map/RingVertexWalkTest.kt b/ft8af/app/src/test/kotlin/radio/ks3ckc/ft8af/ui/map/RingVertexWalkTest.kt new file mode 100644 index 000000000..23b97df93 --- /dev/null +++ b/ft8af/app/src/test/kotlin/radio/ks3ckc/ft8af/ui/map/RingVertexWalkTest.kt @@ -0,0 +1,106 @@ +package radio.ks3ckc.ft8af.ui.map + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +/** + * [forEachRingVertex] — the shared, bounded walk over a flattened polygon ring + * (`[lon0, lat0, lon1, lat1, ...]`) used by the equirectangular / azimuthal / + * POTA / QSO-path land renderers. + * + * The pre-fix draw loops walked `while (i < ring.size) { ...ring[i + 1]...; i += 2 }`, + * which reads one element past the end of an **odd-length** ring → + * ArrayIndexOutOfBoundsException on the Compose draw thread. These cases prove + * the walk is now bounded to `ring.size / 2` complete vertices, byte-identical + * to the old behavior for every even-length ring. + */ +class RingVertexWalkTest { + + private data class Vertex(val lon: Float, val lat: Float, val first: Boolean) + + private fun walk(ring: FloatArray, minVertices: Int = 3): Pair> { + val out = ArrayList() + val plotted = forEachRingVertex(ring, minVertices) { lon, lat, first -> + out.add(Vertex(lon, lat, first)) + } + return plotted to out + } + + @Test + fun `even-length ring plots every vertex, first flag on the opener`() { + // Triangle: 3 complete (lon,lat) vertices. + val (plotted, verts) = walk(floatArrayOf(1f, 2f, 3f, 4f, 5f, 6f)) + assertThat(plotted).isTrue() + assertThat(verts).containsExactly( + Vertex(1f, 2f, true), + Vertex(3f, 4f, false), + Vertex(5f, 6f, false), + ).inOrder() + } + + @Test + fun `odd-length ring ignores the trailing unpaired coordinate without overrun`() { + // 7 floats = 3 complete vertices + a stray 7f. The old walk read ring[6] + // then ring[7] (out of bounds); the bounded walk stops after vertex 3. + val (plotted, verts) = walk(floatArrayOf(1f, 2f, 3f, 4f, 5f, 6f, 7f)) + assertThat(plotted).isTrue() + assertThat(verts).containsExactly( + Vertex(1f, 2f, true), + Vertex(3f, 4f, false), + Vertex(5f, 6f, false), + ).inOrder() + } + + @Test + fun `larger odd-length ring plots all complete pairs and drops the stray`() { + // 9 floats = 4 complete vertices + a stray 9f. + val (plotted, verts) = walk(floatArrayOf(0f, 0f, 1f, 1f, 2f, 2f, 3f, 3f, 9f)) + assertThat(plotted).isTrue() + assertThat(verts.map { it.lon }).containsExactly(0f, 1f, 2f, 3f).inOrder() + } + + @Test + fun `ring with fewer than three complete vertices is skipped entirely`() { + // 4 floats = 2 vertices (< 3): matches the old `if (ring.size < 6) continue`. + val (plotted, verts) = walk(floatArrayOf(1f, 2f, 3f, 4f)) + assertThat(plotted).isFalse() + assertThat(verts).isEmpty() + } + + @Test + fun `five-float ring (two complete vertices) is skipped like an under-length even ring`() { + val (plotted, verts) = walk(floatArrayOf(1f, 2f, 3f, 4f, 5f)) + assertThat(plotted).isFalse() + assertThat(verts).isEmpty() + } + + @Test + fun `empty ring is skipped`() { + val (plotted, verts) = walk(floatArrayOf()) + assertThat(plotted).isFalse() + assertThat(verts).isEmpty() + } + + @Test + fun `the pre-fix unbounded walk really did overrun an odd-length ring`() { + // Guards against anyone reverting the bound: this reproduces the old + // `while (i < ring.size)` walk and asserts it throws on odd input, while + // forEachRingVertex over the same ring does not. + val ring = floatArrayOf(1f, 2f, 3f, 4f, 5f, 6f, 7f) + var threw = false + try { + var i = 0 + var sink = 0f + while (i < ring.size) { + sink += ring[i] + ring[i + 1] + i += 2 + } + assertThat(sink).isNotNaN() // keep `sink` live + } catch (e: IndexOutOfBoundsException) { + threw = true + } + assertThat(threw).isTrue() + // The bounded walk over the identical ring does not throw. + assertThat(walk(ring).first).isTrue() + } +}