Skip to content

Validate RichTextViewModel search match rangesFix/rich text search stale ranges#372

Open
robfeldmann wants to merge 2 commits into
kean:mainfrom
robfeldmann:fix/rich-text-search-stale-ranges
Open

Validate RichTextViewModel search match rangesFix/rich text search stale ranges#372
robfeldmann wants to merge 2 commits into
kean:mainfrom
robfeldmann:fix/rich-text-search-stale-ranges

Conversation

@robfeldmann

Copy link
Copy Markdown

Summary

Fixes a crash in RichTextViewModel when search match ranges computed from originalText are later applied to a changed NSTextStorage.

Crash stack trace...
   Exception Type:  EXC_CRASH (SIGABRT)
   Triggered by Thread:  0

   Last Exception Backtrace:
   0   CoreFoundation   __exceptionPreprocess
   1   libobjc.A.dylib  objc_exception_throw
   2   Foundation       -[NSRLEArray objectAtIndex:effectiveRange:]
   3   Foundation       -[NSMutableAttributedString addAttributes:range:]
   4   UIFoundation     __45-[NSConcreteTextStorage addAttributes:range:]_block_invoke
   5   UIFoundation     __NSConcreteTextStorageLockedForwarding
   6   UIFoundation     -[NSConcreteTextStorage addAttributes:range:]
   7   [App]            specialized closure #1 in RichTextViewModel.didUpdateMatches(_:string:)
                       RichTextViewModel.swift:120
   8   [App]            closure #1 in closure #1 in RichTextViewModel.searchIfNeeded()
                       RichTextViewModel.swift:104
   9   [App]            <deduplicated_symbol>
   10  libdispatch      _dispatch_call_block_and_release
   11  libdispatch      _dispatch_client_callout
   12  libdispatch      _dispatch_main_queue_drain
   13  libdispatch      _dispatch_main_queue_callback_4CF
   14  CoreFoundation   __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__

The crash happens when stale NSRange values are passed into NSTextStorage/NSMutableAttributedString APIs, which can throw an NSRangeException such as:

NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds

What's in this PR:

  • Adds a Swift Testing regression suite for RichTextViewModel.
  • Validates saved match ranges before clearing highlights, applying highlights, or focusing the selected match.
  • Uses a single captured NSTextStorage for each update transaction.
  • Filters stale/out-of-bounds matches instead of applying invalid ranges.

Note

I totally understand if you don't want to add tests as part of this fix or would prefer another testing approach. Feel free to delete the first commit or request that I delete it.

Reproduction

I was not always able to reproduce the crash organically but I did experience first-hand at least once. The steps to reproduce in #370 are accurate though. I have included a unit test that does consistently reproduce the crash and verifies the fix.

The first commit in this branch adds regression tests without the production fix. To verify the crash, check out that commit and run the focused test suite:

git checkout b2d36782
xcodebuild test -scheme Pulse-Package \
  -destination 'platform=iOS Simulator,name=iPhone 17 Pro' \
  -only-testing:PulseUITests/RichTextViewModelTests

Expected result on the first commit: the test process crashes with NSRangeException / NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds.

How to Test

On the final branch, run the same focused test suite:

xcodebuild test -scheme Pulse-Package \
  -destination 'platform=iOS Simulator,name=iPhone 17 Pro' \
  -only-testing:PulseUITests/RichTextViewModelTests

Expected result: all RichTextViewModelTests pass.

Related Issue

Fixes #370.

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.

Crash in RichTextViewModel.clearMatches()

1 participant