Repository navigation
Fix several ICU usage bugs - #998
Conversation
Leonard Hecker (lhecker)
commented
Oct 27, 2026
Leonard Hecker (lhecker)
commented
- Fixes Replace All hangs when the last match ends at the end of the file and the replacement matches again #994 (Freezes when replacing at the end of a file)
- Fixed look-behind matching (see "U8_SET_CP_START")
- Fixed broken case-folding retries
- Swallowing errors on regex timeout
| }; | ||
|
|
||
| self.find_select_next(search, next_search_offset, true); | ||
| self.find_select_next(search, next_search_offset, true)?; |
There was a problem hiding this comment.
This file contains just Option<> -> Result<Option<>> plumbing.
|
|
||
| impl iter::FusedIterator for Utf8Chars<'_> {} | ||
|
|
||
| /// Returns the last character using the same invalid-UTF-8 recovery as `Utf8Chars`. |
There was a problem hiding this comment.
Changed utf8_decode_last to not only split off the last char but also return the remaining unconsumed text. Used by ICU.
| } | ||
|
|
||
| extern "C" fn utext_access(ut: &mut icu_ffi::UText, native_index: i64, forward: bool) -> bool { | ||
| if let Some(cache) = utext_access_impl(ut, native_index, forward) { |
There was a problem hiding this comment.
The bug with the "freezes when replacing at end of file" was that ICU would request indices that are out of bounds and I would reject them with an early return false WITHOUT updating the chunk contents (bad!). So, here you see: Early return removed, clamp the requested index, and always update the chunk contents.
| if index_contained < 0 || index_contained as usize >= tb.text_length() { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
(This is the bad part.)
| // The worst case ratio for UTF-8 to UTF-16 is 1:1, when the text is ASCII. | ||
| // This allows us to safely subtract the UTF-16 buffer size | ||
| // and assume that whatever we read as UTF-8 will fit. | ||
| // TODO: Test what happens if you have lots of invalid UTF-8 text blow up to U+FFFD. |
There was a problem hiding this comment.
Oh, right I also fixed this "blow up" by limiting the number of chars to probe for UTF8 continuation bytes.
|
|
||
| if forward { | ||
| utf8_len_limit = (tb.text_length() - native_index).min(UTF16_LEN_LIMIT); | ||
| native_start = native_index; |
There was a problem hiding this comment.
So, the issue here is that while reverse iteration below aligned the UTF8 chunk start, we actually need to do it unconditionally.
|
|
||
| // Translate the given range from UTF-8 to UTF-16. | ||
| // NOTE: This code makes the assumption that the `native_index` is always | ||
| // at UTF-8 codepoint boundaries which technically isn't guaranteed. |
There was a problem hiding this comment.
See, I even noted it as a TODO lol.
| if status == icu_ffi::U_BUFFER_OVERFLOW_ERROR && output_len > 0 { | ||
| output.reserve_exact(arena, output_len as usize); | ||
| let output = output.spare_capacity_mut(); | ||
| status = icu_ffi::U_ZERO_ERROR; |
There was a problem hiding this comment.
This fixes fold_case, because without this the ucasemap_utf8FoldCase call below gets skipped (ICU early returns if the given status is not okay).
This was not apparent, because I preallocate a guesstimated amount of memory. This fallback path is only executed when that size was not enough (rare).
| // | ||
| // TODO: It may be worth avoiding this if requested index | ||
| // neighbors the previous block during linear scans by ICU. | ||
| if native_start < tb.text_length() { |
There was a problem hiding this comment.
native_start being len-1 would not be a problem here, right?
There was a problem hiding this comment.
You mean because it enters the if condition? No, that should be fine. The if condition only exists so we can safely do native_start + 1. Otherwise, everyone's welcome.
