Fix #113154: Improved Cursor Step Error Handling #113172

Harley Acheson · 2023-10-02T19:49:04+02:00

Harley Acheson commented

2023-10-02 19:49:04 +02:00

Allow BLI_str_cursor_step_bounds_utf8 to handle being called with -1
for pos, which can sometimes happen.

In this complaint, double-clicking on that data-block selector, it is a UI_BTYPE_SEARCH_MENU so we call ui_do_but_TEX. It has a blank str and its pos has a value of -1. It makes it to the event handling for KM_DBL_CLICK in ui_do_but_textedit. There it calls BLI_str_cursor_step_bounds_utf8 with a blank string and pos of -1. Because pos < str_maxlen we call cursor_delim_type_utf8 where the cast to size_t turns the -1 into maxint64, which causes the assert in BLI_str_utf8_as_unicode_step_or_error.

To my eye the only part actually misbehaving is BLI_str_cursor_step_bounds_utf8. This PR just makes it handle pos being negative. And then an assert is added to cursor_delim_type_utf8 that it never get a negative pos.

Allow `BLI_str_cursor_step_bounds_utf8` to handle being called with -1 for `pos`, which can sometimes happen. --- In this complaint, double-clicking on that data-block selector, it is a `UI_BTYPE_SEARCH_MENU` so we call `ui_do_but_TEX`. It has a blank str and its pos has a value of -1. It makes it to the event handling for KM_DBL_CLICK in `ui_do_but_textedit`. There it calls `BLI_str_cursor_step_bounds_utf8` with a blank string and pos of -1. Because `pos < str_maxlen` we call `cursor_delim_type_utf8` where the cast to size_t turns the -1 into maxint64, which causes the assert in `BLI_str_utf8_as_unicode_step_or_error`. To my eye the only part actually misbehaving is `BLI_str_cursor_step_bounds_utf8`. This PR just makes it handle pos being negative. And then an assert is added to `cursor_delim_type_utf8` that it never get a negative pos.

Harley Acheson added 1 commit 2023-10-02 19:49:15 +02:00

e9957f7aee Fix #113154 : Improved Cursor Step Error Handling

Allow `BLI_str_cursor_step_bounds_utf8` to handle being called with -1
for `pos`, which can sometimes happen.

Harley Acheson added this to the Core project 2023-10-02 19:49:23 +02:00

Harley Acheson requested review from Campbell Barton 2023-10-02 19:49:31 +02:00

Campbell Barton commented

2023-10-03 05:06:13 +02:00

Thanks for looking into this, committed an alternate fix: 26bb029ad8.

As cursor manipulation happens in a few isolated places, I'd rather assume the input position is always valid.
If there is some risk the input is out of range the caller can clamp.

Otherwise there is the risk of invalid cursor positions relying on cursor stepping to correct/clamp the cursor position.

In this case the press event wasn't inside the button, leaving but->pos set to -1, so calling word selection didn't make sense, now fixed.

Thanks for looking into this, committed an alternate fix: 26bb029ad89da22eb4266a4e274de568e221c3a3. As cursor manipulation happens in a few isolated places, I'd rather assume the input position is always valid. If there is some risk the input is out of range the caller can clamp. Otherwise there is the risk of invalid cursor positions relying on cursor stepping to correct/clamp the cursor position. In this case the press event wasn't inside the button, leaving `but->pos` set to -1, so calling word selection didn't make sense, now fixed.

Campbell Barton closed this pull request

2023-10-03 05:06:15 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #113154: Improved Cursor Step Error Handling #113172

Pull request closed