Skip to content

fix(pdu): clamp wheel rotation to valid range - #1817

Closed
Mariusz Białończyk (manio) wants to merge 1 commit into
Devolutions:masterfrom
manio:mouse-scroll-fix
Closed

fix(pdu): clamp wheel rotation to valid range#1817
Mariusz Białończyk (manio) wants to merge 1 commit into
Devolutions:masterfrom
manio:mouse-scroll-fix

Conversation

@manio

Copy link
Copy Markdown
Contributor

MousePdu::encode asserted that number_of_wheel_rotation_units fits the wire's 9-bit two's-complement range ([-256, 255]) and panicked via debug_assert! otherwise. Database::apply in ironrdp-input forwards the OS-reported wheel delta into this field unclamped, so a fast scroll (delta beyond that range, e.g. -300) panicked any debug build.

Clamp to [-256, 255] in encode instead of asserting, so the encoder can no longer panic regardless of what a caller passes in. Values outside the range now saturate to the nearest representable edge rather than being rejected.

Fixes: #1816

MousePdu::encode asserted that number_of_wheel_rotation_units fits
the wire's 9-bit two's-complement range ([-256, 255]) and panicked
via debug_assert! otherwise. Database::apply in ironrdp-input
forwards the OS-reported wheel delta into this field unclamped, so
a fast scroll (delta beyond that range, e.g. -300) panicked any
debug build.

Clamp to [-256, 255] in encode instead of asserting, so the encoder
can no longer panic regardless of what a caller passes in. Values
outside the range now saturate to the nearest representable edge
rather than being rejected.

Fixes: Devolutions#1816
@github-actions github-actions Bot added maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/XS Size: up to 49 counted lines and 2 files labels Aug 28, 2026
@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

can you see if this PR still makes sense after merging #1818 for the same issue?

@manio

Copy link
Copy Markdown
Contributor Author

can you see if this PR still makes sense after merging #1818 for the same issue?

nope

PR replaced by #1818
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/XS Size: up to 49 counted lines and 2 files

Development

Successfully merging this pull request may close these issues.

panic in mouse.rs

2 participants