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.

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

Fix bounds checks on Focus::narrow and split_at #89

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

goffrie
Copy link
Contributor

@goffrie goffrie commented Jan 17, 2025

I noticed that these methods were more restrictive than necessary, as compared to subslicing a slice (or even Vector::slice), making it hard (though not impossible) to end up with an empty Focus. Additionally narrow() allowed its end index to go past len() which could lead to other problems.

I added some tests for these methods too.

Copy link
Owner

@jneem jneem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jneem jneem merged commit 7c5f240 into jneem:main Jan 17, 2025
15 of 16 checks passed
@goffrie goffrie deleted the focus-bounds branch January 21, 2025 20:42
kevinaboos added a commit to project-robius/matrix-rust-sdk that referenced this pull request Jan 25, 2025
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.
jplatte pushed a commit to matrix-org/matrix-rust-sdk that referenced this pull request Jan 25, 2025
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants