From 7304d21e9356e7ab7ac59026b84188e8c62f9db4 Mon Sep 17 00:00:00 2001 From: erentufekci Date: Thu, 7 May 2026 19:30:09 +0300 Subject: [PATCH] fix: prevent two OOB crashes in updateAnnotatedString MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Bug 1 — IndexOutOfBoundsException in updateAnnotatedString** `richParagraphList.removeAt(i)` was called inside `fastForEachIndexed`, which captures the list size once at loop entry. After the first removal the list shrinks but the loop index keeps incrementing, eventually accessing an index beyond the new size and throwing. Fix: collect stale paragraph indices during the loop and remove them in reverse order after `buildAnnotatedString` completes. **Bug 2 — StringIndexOutOfBoundsException in AnnotatedStringExt** `text.substring(index, index + richSpan.text.length)` assumed the span's stored length never exceeds the available text. Samsung soft keyboards on budget devices (Galaxy A04, A57, and similar) route certain input events — autocomplete, autocorrect, word suggestions — through `TextFieldKeyInput` (dispatchKeyEvent) rather than the standard IME protocol. This delivers a new `TextFieldValue` atomically before span reconciliation runs, so a span's stale stored length can exceed the incoming text length → crash. Physical keyboards connected to Android trigger the same code path. Fix: clamp `safeStart` and `safeEnd` to `text.length` before the `substring` call so stale span lengths are handled gracefully. Adds `RichTextStateParagraphRemovalTest` with three regression cases covering both crash paths. Co-Authored-By: Claude Sonnet 4.6 --- .../richeditor/model/RichTextState.kt | 15 +++- .../richeditor/utils/AnnotatedStringExt.kt | 10 ++- .../RichTextStateParagraphRemovalTest.kt | 87 +++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 richeditor-compose/src/commonTest/kotlin/com/mohamedrejeb/richeditor/model/RichTextStateParagraphRemovalTest.kt diff --git a/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/model/RichTextState.kt b/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/model/RichTextState.kt index f867b621..28e910ae 100644 --- a/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/model/RichTextState.kt +++ b/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/model/RichTextState.kt @@ -2304,11 +2304,15 @@ public class RichTextState internal constructor( // measures the prefix and corrects it. applyCachedStartTextWidths() + // Collect paragraph indices to remove after iteration — calling removeAt(i) inside + // fastForEachIndexed throws IndexOutOfBoundsException because the loop captures the + // list size once and the removal shifts subsequent indices. + val paragraphIndicesToRemove = mutableListOf() annotatedString = buildAnnotatedString { var index = 0 richParagraphList.fastForEachIndexed { i, richParagraph -> if (index > newText.length) { - richParagraphList.removeAt(i) + paragraphIndicesToRemove.add(i) return@fastForEachIndexed } @@ -2345,6 +2349,15 @@ public class RichTextState internal constructor( } } + // Remove stale paragraphs in reverse order so earlier indices stay valid. + if (paragraphIndicesToRemove.isNotEmpty()) { + for (idx in paragraphIndicesToRemove.sortedDescending()) { + if (idx in richParagraphList.indices) { + richParagraphList.removeAt(idx) + } + } + } + inlineContentMap.keys.forEach { key -> if (key !in usedInlineContentMapKeys) { inlineContentMap.remove(key) diff --git a/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/utils/AnnotatedStringExt.kt b/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/utils/AnnotatedStringExt.kt index 1f5e8727..10b556b3 100644 --- a/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/utils/AnnotatedStringExt.kt +++ b/richeditor-compose/src/commonMain/kotlin/com/mohamedrejeb/richeditor/utils/AnnotatedStringExt.kt @@ -139,7 +139,15 @@ internal fun AnnotatedString.Builder.append( return@withStyle } - val newText = text.substring(index, index + richSpan.text.length) + // Guard against stale span lengths that exceed the new text. Samsung soft keyboards + // on budget devices (e.g. A04, A57) route some input through TextFieldKeyInput + // (dispatchKeyEvent) rather than the IME protocol, delivering a new TextFieldValue + // atomically before span reconciliation runs. Without this clamp the substring + // throws StringIndexOutOfBoundsException when a span's stored length overshoots + // the incoming text. Physical keyboards trigger the same code path. + val safeStart = index.coerceAtMost(text.length) + val safeEnd = (index + richSpan.text.length).coerceAtMost(text.length) + val newText = text.substring(safeStart, safeEnd) richSpan.text = newText richSpan.textRange = TextRange(index, index + richSpan.text.length) diff --git a/richeditor-compose/src/commonTest/kotlin/com/mohamedrejeb/richeditor/model/RichTextStateParagraphRemovalTest.kt b/richeditor-compose/src/commonTest/kotlin/com/mohamedrejeb/richeditor/model/RichTextStateParagraphRemovalTest.kt new file mode 100644 index 00000000..371d79de --- /dev/null +++ b/richeditor-compose/src/commonTest/kotlin/com/mohamedrejeb/richeditor/model/RichTextStateParagraphRemovalTest.kt @@ -0,0 +1,87 @@ +package com.mohamedrejeb.richeditor.model + +import androidx.compose.ui.text.TextRange +import androidx.compose.ui.text.input.TextFieldValue +import com.mohamedrejeb.richeditor.annotation.ExperimentalRichTextApi +import com.mohamedrejeb.richeditor.paragraph.RichParagraph +import kotlin.test.Test + +/** + * Regression tests for two related crashes in [RichTextState.updateAnnotatedString]. + * + * Bug 1 — IndexOutOfBoundsException (index: 12, size: 12 at SnapshotStateList.get): + * The loop iterated `richParagraphList` via `fastForEachIndexed` (which captures + * the list size once) and called `richParagraphList.removeAt(i)` inside the body. + * Once the first paragraph was removed the loop continued past the new end of the + * list and threw. Fix: collect indices to remove, apply after the loop. + * + * Bug 2 — StringIndexOutOfBoundsException (begin N, end M, length L where M > L): + * `text.substring(index, index + richSpan.text.length)` assumed the span's stored + * length never exceeds the incoming text. Samsung soft keyboards on budget devices + * (e.g. A04, A57) route some input through `TextFieldKeyInput` (dispatchKeyEvent) + * rather than the IME protocol, delivering a new `TextFieldValue` atomically before + * span reconciliation runs. Physical keyboards trigger the same code path. + * Fix: clamp `safeStart`/`safeEnd` to `text.length` before the substring call. + */ +@OptIn(ExperimentalRichTextApi::class) +class RichTextStateParagraphRemovalTest { + + @Test + fun `shrinking text below paragraph range does not throw IndexOutOfBoundsException`() { + val paragraphCount = 13 + val paragraphs = List(paragraphCount) { i -> + RichParagraph(key = i + 1).also { p -> + p.children.add(RichSpan(text = "line$i", paragraph = p)) + } + } + val state = RichTextState(initialRichParagraphList = paragraphs) + + // Force updateAnnotatedString down the `index > newText.length` removal branch + // on many paragraphs at once. Before the fix this crashed with IOOB because + // removeAt(i) was called during fastForEachIndexed. + state.onTextFieldValueChange( + TextFieldValue(text = "", selection = TextRange.Zero) + ) + } + + @Test + fun `repeated shrink-grow cycles do not throw`() { + val state = RichTextState( + initialRichParagraphList = List(20) { i -> + RichParagraph(key = i + 1).also { p -> + p.children.add(RichSpan(text = "paragraph$i", paragraph = p)) + } + } + ) + + repeat(5) { + state.onTextFieldValueChange( + TextFieldValue(text = "a", selection = TextRange(1)) + ) + state.onTextFieldValueChange( + TextFieldValue(text = "aaaa aaaa aaaa", selection = TextRange(14)) + ) + } + } + + @Test + fun `drastic text shrink does not throw StringIndexOutOfBoundsException`() { + // Exercises the AnnotatedStringExt bounds-clamp fix. Build a state with a + // moderately long paragraph, then deliver a TextFieldValue whose text is much + // shorter than the spans currently represent — simulating the atomic update + // that Samsung's soft keyboard sends via TextFieldKeyInput. + val longText = "Hello world this is a longer paragraph with multiple words" + val state = RichTextState( + initialRichParagraphList = listOf( + RichParagraph(key = 1).also { p -> + p.children.add(RichSpan(text = longText, paragraph = p)) + } + ) + ) + + // Deliver a text value that is far shorter than the span's stored length. + state.onTextFieldValueChange( + TextFieldValue(text = "Hi", selection = TextRange(2)) + ) + } +}