Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invalid bounds check in vector::Focus::narrow() causing a bug #330

Closed
kevinaboos opened this issue Jan 20, 2025 · 1 comment · Fixed by #332
Closed

Invalid bounds check in vector::Focus::narrow() causing a bug #330

kevinaboos opened this issue Jan 20, 2025 · 1 comment · Fixed by #332
Assignees
@kevinaboos
Labels
bug Something isn't working

Comments

@kevinaboos
Copy link
Member

Apparently thee usage of Focus::narrow() is incorrect here, as it may panic (not terminally, but still)

robrix/src/sliding_sync.rs

Line 1801 in 85c6f00

.narrow(..starting_index) 

thread 'tokio-runtime-worker' panicked at /Users/kevinboos/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/imbl-3.0.0/src/vector/focus.rs:185:13:
vector::Focus::narrow: range out of bounds
src/sliding_sync.rs:862:37 - Already sent read receipt to room !QQpfJfZvqxbCfeDgCj:matrix.org for event $Ocx_2yOCGEHvVqEAcJt52LQMy45k0HEJlW4Mo7uyqgo
stack backtrace:
src/home/rooms_list.rs:568:17 - RoomsList: processed 1 updates to the list of all rooms
   0:        0x1046a18a4 - std::backtrace_rs::backtrace::libunwind::trace::h45691cc8cca6fb07
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/../../backtrace/src/backtrace/libunwind.rs:116:5
   1:        0x1046a18a4 - std::backtrace_rs::backtrace::trace_unsynchronized::h1807100eb8bcbef9
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x1046a18a4 - std::sys::backtrace::_print_fmt::hd874de7025e9b511
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/sys/backtrace.rs:66:9
   3:        0x1046a18a4 - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::haddca16dac06b840
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/sys/backtrace.rs:39:26
   4:        0x1046c07dc - core::fmt::rt::Argument::fmt::hf88b9a376f66dc2a
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/core/src/fmt/rt.rs:177:76
   5:        0x1046c07dc - core::fmt::write::h466ec29fb8dd1f50
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/core/src/fmt/mod.rs:1440:21
   6:        0x10469e02c - std::io::Write::write_fmt::h71fc6ea740f1fd58
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/io/mod.rs:1887:15
   7:        0x1046a1758 - std::sys::backtrace::BacktraceLock::print::he693110ad096ec5e
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/sys/backtrace.rs:42:9
   8:        0x1046a263c - std::panicking::default_hook::{{closure}}::h86f84a5db39f484d
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/panicking.rs:298:22
   9:        0x1046a2478 - std::panicking::default_hook::h4c632142d7089e18
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/panicking.rs:325:9
  10:        0x1046a316c - std::panicking::rust_panic_with_hook::h5f18f9068b2f58c3
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/panicking.rs:831:13
  11:        0x1046a2e3c - std::panicking::begin_panic::{{closure}}::h9a1f647a7194ffd0
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/panicking.rs:768:9
  12:        0x1046a1d64 - std::sys::backtrace::__rust_end_short_backtrace::h990b1647cb1e0e99
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/sys/backtrace.rs:168:18
  13:        0x104735a7c - std::panicking::begin_panic::h970d28d4c7e90938
                               at /rustc/99db2737c91d1e4b36b2ffc17dcda5878bcae625/library/std/src/panicking.rs:767:5
  14:        0x10163058c - imbl::vector::focus::Focus<A>::narrow::he1dba1793b527b78
                               at /Users/kevinboos/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/imbl-3.0.0/src/vector/focus.rs:185:13
  15:        0x1013e7364 - robrix::sliding_sync::timeline_subscriber_handler::{{closure}}::hb7bc706ed5dfe910
                               at /Users/kevinboos/robius/robrix/src/sliding_sync.rs:1812:58
@kevinaboos kevinaboos added the bug Something isn't working label Jan 20, 2025
@kevinaboos kevinaboos self-assigned this Jan 20, 2025
@kevinaboos
Copy link
Member Author

ah, this is caused by a bug in imbl that was coincidentally fixed a few days ago: jneem/imbl#89

@kevinaboos kevinaboos mentioned this issue Jan 20, 2025
Merged
This was referenced Jan 21, 2025
Merged
Closed
@kevinaboos kevinaboos changed the title Potentially improper use of vector::Focus::narrow() Invalid bounds check in vector::Focus::narrow() causing a bug Jan 25, 2025
kevinaboos added a commit to project-robius/matrix-rust-sdk that referenced this issue Jan 25, 2025
@kevinaboos
Upgrade imbl, eyeball-im, eyeball-im-util to fix bounds check
62ba64c 
A bounds check was recently relaxed in `imbl`'s `Focus::narrow()`
function: jneem/imbl#89,
which fixed a bug that would cause a panic if the downstream user
of `matrix-sdk-ui` attempted to narrow a focus of Timeline items
using a range that included the last item in the Timeline.
Example: project-robius/robrix#330

This fix has been incorporated in `eyeball-im` and `eyeball-im-util`
and has been tested by me to no longer trigger upon the aforementioned
conditions.
@kevinaboos kevinaboos mentioned this issue Jan 25, 2025
Merged
jplatte pushed a commit to matrix-org/matrix-rust-sdk that referenced this issue Jan 25, 2025
@kevinaboos @jplatte
Upgrade imbl, eyeball-im, eyeball-im-util to fix bounds check
756dec2 
A bounds check was recently relaxed in `imbl`'s `Focus::narrow()`
function: jneem/imbl#89,
which fixed a bug that would cause a panic if the downstream user
of `matrix-sdk-ui` attempted to narrow a focus of Timeline items
using a range that included the last item in the Timeline.
Example: project-robius/robrix#330

This fix has been incorporated in `eyeball-im` and `eyeball-im-util`
and has been tested by me to no longer trigger upon the aforementioned
conditions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kevinaboos kevinaboos

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Patch imbl to fix their bug in Focus::narrow() kevinaboos/robrix
1 participant
@kevinaboos