Skip to content

fix: prevent two OOB crashes in updateAnnotatedString (Samsung soft keyboard + large text shrink)#721

Open
tfkci wants to merge 1 commit into
MohamedRejeb:mainfrom
tfkci:fix/upstream-oob-crashes
Open

fix: prevent two OOB crashes in updateAnnotatedString (Samsung soft keyboard + large text shrink)#721
tfkci wants to merge 1 commit into
MohamedRejeb:mainfrom
tfkci:fix/upstream-oob-crashes

Conversation

@tfkci
Copy link
Copy Markdown

@tfkci tfkci commented May 7, 2026

Summary

Two OutOfBoundsException crashes in updateAnnotatedString and its call chain, both reproducible on Samsung budget/mid-range devices (Galaxy A04, A57, M-series) and physical keyboards.

Bug 1 — IndexOutOfBoundsException: index: N, size: N

Location: RichTextState.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.

Fix: Collect stale paragraph indices during the loop (paragraphIndicesToRemove) and remove them in reverse order after buildAnnotatedString completes.

Bug 2 — StringIndexOutOfBoundsException: begin N, end M, length L (M > L)

Location: AnnotatedStringExt.kttext.substring(index, index + richSpan.text.length)

Samsung soft keyboards on budget devices route certain operations (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. Physical keyboards connected to Android hit the same code path.

Typical stack:

StringIndexOutOfBoundsException: begin 132, end 146, length 144
  at AnnotatedStringExt.append (AnnotatedStringExt.kt:142)
  at RichTextState.updateAnnotatedString (RichTextState.kt:...)
  at RichTextState.updateTextFieldValue (RichTextState.kt:...)
  at RichTextState.onTextFieldValueChangeInner (RichTextState.kt:...)
  at TextFieldKeyInput.apply (TextFieldKeyInput.kt:69)   ← soft keyboard via dispatchKeyEvent

Fix: Clamp safeStart and safeEnd to text.length before the substring call so stale span lengths are handled gracefully.

Test plan

  • RichTextStateParagraphRemovalTest — three new regression cases covering both crash paths
  • All existing tests pass (./gradlew :richeditor-compose:desktopTest)

Devices known to trigger Bug 2

Samsung Galaxy A04, A57 (and likely other Samsung A/M-series budget phones using Samsung Keyboard with word suggestions enabled).

**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 <noreply@anthropic.com>
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