From dd4754e43ed3a777fb447bb1101b896725b2c155 Mon Sep 17 00:00:00 2001 From: Alexandr Yusuk Date: Thu, 4 Sep 2025 16:01:49 +0300 Subject: [PATCH 1/8] refactor: enable `unwrap_used` clippy correctness lint --- Cargo.toml | 2 +- benches/src/perfenc.rs | 28 ++++--- clippy.toml | 1 + crates/ironrdp-acceptor/src/connection.rs | 2 +- crates/ironrdp-ainput/src/lib.rs | 28 +++++-- crates/ironrdp-bench/benches/bench.rs | 25 ++++-- crates/ironrdp-blocking/src/connector.rs | 4 +- crates/ironrdp-client/src/app.rs | 64 +++++++++----- crates/ironrdp-client/src/config.rs | 5 +- crates/ironrdp-client/src/main.rs | 4 +- crates/ironrdp-client/src/rdp.rs | 29 +++++-- crates/ironrdp-cliprdr-format/src/bitmap.rs | 6 +- .../ironrdp-cliprdr/src/pdu/file_contents.rs | 7 +- .../src/connection_activation.rs | 22 +++-- crates/ironrdp-displaycontrol/src/pdu/mod.rs | 19 +++-- crates/ironrdp-dvc/src/client.rs | 5 +- crates/ironrdp-dvc/src/lib.rs | 2 +- .../ironrdp-graphics/src/color_conversion.rs | 24 +++--- .../ironrdp-graphics/src/image_processing.rs | 19 ++++- crates/ironrdp-graphics/src/rdp6/rle.rs | 29 ++++--- crates/ironrdp-graphics/src/rlgr.rs | 59 +++++++------ crates/ironrdp-graphics/src/zgfx/mod.rs | 7 +- crates/ironrdp-input/src/lib.rs | 5 +- crates/ironrdp-rdcleanpath/src/lib.rs | 28 ++++--- crates/ironrdp-rdpdr/src/pdu/efs.rs | 53 +----------- crates/ironrdp-rdpdr/src/pdu/esc.rs | 5 +- crates/ironrdp-rdpsnd-native/examples/cpal.rs | 2 +- crates/ironrdp-rdpsnd-native/src/cpal.rs | 27 ++++-- crates/ironrdp-rdpsnd/src/pdu/mod.rs | 2 +- crates/ironrdp-server/src/builder.rs | 22 ++--- crates/ironrdp-server/src/display.rs | 6 +- crates/ironrdp-server/src/encoder/bitmap.rs | 71 +++++++++------- crates/ironrdp-server/src/encoder/mod.rs | 83 +++++++++++++------ crates/ironrdp-server/src/encoder/rfx.rs | 13 +-- crates/ironrdp-server/src/server.rs | 10 ++- crates/ironrdp-session/src/active_stage.rs | 3 +- crates/ironrdp-session/src/image.rs | 22 +++-- crates/ironrdp-session/src/rfx.rs | 6 +- crates/ironrdp-testsuite-core/src/lib.rs | 1 + .../tests/graphics/color_conversion.rs | 4 +- crates/ironrdp-testsuite-core/tests/main.rs | 1 + crates/ironrdp-testsuite-extra/tests/tests.rs | 6 +- crates/ironrdp-web/src/canvas.rs | 20 ++--- crates/ironrdp-web/src/session.rs | 37 +++++---- crates/ironrdp/examples/screenshot.rs | 7 +- crates/ironrdp/examples/server.rs | 41 ++++++--- ffi/build.rs | 14 ++-- xtask/src/main.rs | 3 +- 48 files changed, 522 insertions(+), 361 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 04e26877d..674065a81 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,7 +102,7 @@ float_cmp = "warn" lossy_float_literal = "warn" float_cmp_const = "warn" as_underscore = "warn" -# TODO: unwrap_used = "warn" # Let’s either handle `None`, `Err` or use `expect` to give a reason. +unwrap_used = "warn" large_stack_frames = "warn" mem_forget = "warn" mixed_read_write_in_expression = "warn" diff --git a/benches/src/perfenc.rs b/benches/src/perfenc.rs index 45b1383fc..a44821d8b 100644 --- a/benches/src/perfenc.rs +++ b/benches/src/perfenc.rs @@ -2,7 +2,7 @@ #![allow(clippy::print_stderr)] #![allow(clippy::print_stdout)] -use core::num::NonZero; +use core::num::{NonZeroU16, NonZeroUsize}; use core::time::Duration; use std::io::Write as _; use std::time::Instant; @@ -59,13 +59,14 @@ async fn main() -> Result<(), anyhow::Error> { OptCodec::QoiZ => update_codecs.set_qoiz(Some(0)), }; - let mut encoder = UpdateEncoder::new(DesktopSize { width, height }, flags, update_codecs); + let mut encoder = UpdateEncoder::new(DesktopSize { width, height }, flags, update_codecs) + .context("failed to initialize update encoder")?; let mut total_raw = 0u64; let mut total_enc = 0u64; let mut n_updates = 0u64; let mut updates = DisplayUpdates::new(file, DesktopSize { width, height }, fps); - while let Some(up) = updates.next_update().await { + while let Some(up) = updates.next_update().await? { if let DisplayUpdate::Bitmap(ref up) = up { total_raw += up.data.len() as u64; } else { @@ -82,7 +83,7 @@ async fn main() -> Result<(), anyhow::Error> { } n_updates += 1; print!("."); - std::io::stdout().flush().unwrap(); + std::io::stdout().flush()?; } println!(); @@ -119,20 +120,21 @@ impl DisplayUpdates { #[async_trait::async_trait] impl RdpServerDisplayUpdates for DisplayUpdates { - async fn next_update(&mut self) -> Option { + async fn next_update(&mut self) -> anyhow::Result> { let stride = self.desktop_size.width as usize * 4; let frame_size = stride * self.desktop_size.height as usize; let mut buf = vec![0u8; frame_size]; - if self.file.read_exact(&mut buf).await.is_err() { - return None; - } + // FIXME: AsyncReadExt::read_exact is not cancellation safe. + self.file.read_exact(&mut buf).await.context("read exact")?; let now = Instant::now(); if let Some(last_update_time) = self.last_update_time { let elapsed = now - last_update_time; if self.fps > 0 && elapsed < Duration::from_millis(1000 / self.fps) { sleep(Duration::from_millis( - 1000 / self.fps - u64::try_from(elapsed.as_millis()).unwrap(), + 1000 / self.fps + - u64::try_from(elapsed.as_millis()) + .context("invalid `elapsed millis`: out of range integral conversion")?, )) .await; } @@ -142,13 +144,13 @@ impl RdpServerDisplayUpdates for DisplayUpdates { let up = DisplayUpdate::Bitmap(BitmapUpdate { x: 0, y: 0, - width: self.desktop_size.width.try_into().unwrap(), - height: self.desktop_size.height.try_into().unwrap(), + width: NonZeroU16::new(self.desktop_size.width).context("width cannot be zero")?, + height: NonZeroU16::new(self.desktop_size.height).context("height cannot be zero")?, format: PixelFormat::RgbX32, data: buf.into(), - stride: NonZero::new(stride).unwrap(), + stride: NonZeroUsize::new(stride).context("stride cannot be zero")?, }); - Some(up) + Ok(Some(up)) } } diff --git a/clippy.toml b/clippy.toml index 7fc7b2d0d..dd6a9fe4e 100644 --- a/clippy.toml +++ b/clippy.toml @@ -3,3 +3,4 @@ semicolon-outside-block-ignore-multiline = true accept-comment-above-statement = true accept-comment-above-attributes = true allow-panic-in-tests = true +allow-unwrap-in-tests = true diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index f4355293e..ca27245ce 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -404,7 +404,7 @@ impl Sequence for Acceptor { .into_iter() .enumerate() .map(|(i, channel)| { - let channel_id = u16::try_from(i).unwrap() + self.io_channel_id + 1; + let channel_id = u16::try_from(i).expect("always in the range") + self.io_channel_id + 1; if let Some((type_id, c)) = channel { self.static_channels.attach_channel_id(type_id, channel_id); (channel_id, Some(c)) diff --git a/crates/ironrdp-ainput/src/lib.rs b/crates/ironrdp-ainput/src/lib.rs index c6cfdb9eb..c6ad27e2d 100644 --- a/crates/ironrdp-ainput/src/lib.rs +++ b/crates/ironrdp-ainput/src/lib.rs @@ -6,8 +6,8 @@ use ironrdp_core::{ ensure_fixed_part_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor, }; use ironrdp_dvc::DvcEncode; -use num_derive::{FromPrimitive, ToPrimitive}; -use num_traits::{FromPrimitive as _, ToPrimitive as _}; +use num_derive::FromPrimitive; +use num_traits::FromPrimitive as _; // Advanced Input channel as defined from Freerdp, [here]: // // [here]: https://github.com/FreeRDP/FreeRDP/blob/master/include/freerdp/channels/ainput.h @@ -93,11 +93,19 @@ impl<'de> Decode<'de> for VersionPdu { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)] pub enum ServerPduType { Version = 0x01, } +impl ServerPduType { + fn as_u16(&self) -> u16 { + match self { + Self::Version => 0x01, + } + } +} + impl<'a> From<&'a ServerPdu> for ServerPduType { fn from(s: &'a ServerPdu) -> Self { match s { @@ -121,7 +129,7 @@ impl Encode for ServerPdu { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_fixed_part_size!(in: dst); - dst.write_u16(ServerPduType::from(self).to_u16().unwrap()); + dst.write_u16(ServerPduType::from(self).as_u16()); match self { ServerPdu::Version(pdu) => pdu.encode(dst), } @@ -220,7 +228,7 @@ impl Encode for ClientPdu { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_fixed_part_size!(in: dst); - dst.write_u16(ClientPduType::from(self).to_u16().unwrap()); + dst.write_u16(ClientPduType::from(self).as_u16()); match self { ClientPdu::Mouse(pdu) => pdu.encode(dst), } @@ -254,11 +262,19 @@ impl<'de> Decode<'de> for ClientPdu { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)] pub enum ClientPduType { Mouse = 0x02, } +impl ClientPduType { + fn as_u16(&self) -> u16 { + match self { + Self::Mouse => 0x02, + } + } +} + impl<'a> From<&'a ClientPdu> for ClientPduType { fn from(s: &'a ClientPdu) -> Self { match s { diff --git a/crates/ironrdp-bench/benches/bench.rs b/crates/ironrdp-bench/benches/bench.rs index 164343000..cf0d7c2e4 100644 --- a/crates/ironrdp-bench/benches/bench.rs +++ b/crates/ironrdp-bench/benches/bench.rs @@ -1,4 +1,4 @@ -use core::num::NonZero; +use core::num::{NonZeroU16, NonZeroUsize}; use criterion::{criterion_group, criterion_main, Criterion}; use ironrdp_graphics::color_conversion::to_64x64_ycbcr_tile; @@ -9,14 +9,19 @@ use ironrdp_server::BitmapUpdate; pub fn rfx_enc_tile_bench(c: &mut Criterion) { let quant = rfx::Quant::default(); let algo = rfx::EntropyAlgorithm::Rlgr3; + + const WIDTH: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero"); + const HEIGHT: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero"); + const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero"); + let bitmap = BitmapUpdate { x: 0, y: 0, - width: NonZero::new(64).unwrap(), - height: NonZero::new(64).unwrap(), + width: WIDTH, + height: HEIGHT, format: ironrdp_server::PixelFormat::ARgb32, data: vec![0; 64 * 64 * 4].into(), - stride: NonZero::new(64 * 4).unwrap(), + stride: STRIDE, }; c.bench_function("rfx_enc_tile", |b| b.iter(|| rfx_enc_tile(&bitmap, &quant, algo, 0, 0))); } @@ -24,14 +29,20 @@ pub fn rfx_enc_tile_bench(c: &mut Criterion) { pub fn rfx_enc_bench(c: &mut Criterion) { let quant = rfx::Quant::default(); let algo = rfx::EntropyAlgorithm::Rlgr3; + + const WIDTH: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero"); + const HEIGHT: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero"); + // QUESTION: It looks like we have a bug here, don't we? The stride value should be 2048 * 4. + const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero"); + let bitmap = BitmapUpdate { x: 0, y: 0, - width: NonZero::new(2048).unwrap(), - height: NonZero::new(2048).unwrap(), + width: WIDTH, + height: HEIGHT, format: ironrdp_server::PixelFormat::ARgb32, data: vec![0; 2048 * 2048 * 4].into(), - stride: NonZero::new(64 * 4).unwrap(), + stride: STRIDE, }; c.bench_function("rfx_enc", |b| b.iter(|| rfx_enc(&bitmap, &quant, algo))); } diff --git a/crates/ironrdp-blocking/src/connector.rs b/crates/ironrdp-blocking/src/connector.rs index 80623e791..9375b66ab 100644 --- a/crates/ironrdp-blocking/src/connector.rs +++ b/crates/ironrdp-blocking/src/connector.rs @@ -100,7 +100,9 @@ fn resolve_generator( loop { match state { GeneratorState::Suspended(request) => { - let response = network_client.send(&request).unwrap(); + let response = network_client.send(&request).map_err(|e| { + ConnectorError::new("network client send", ironrdp_connector::ConnectorErrorKind::Credssp(e)) + })?; state = generator.resume(Ok(response)); } GeneratorState::Completed(client_state) => { diff --git a/crates/ironrdp-client/src/app.rs b/crates/ironrdp-client/src/app.rs index 81aca9960..6a7e1ff56 100644 --- a/crates/ironrdp-client/src/app.rs +++ b/crates/ironrdp-client/src/app.rs @@ -5,9 +5,10 @@ use core::time::Duration; use std::sync::Arc; use std::time::Instant; +use anyhow::{bail, Context as _}; use raw_window_handle::{DisplayHandle, HasDisplayHandle as _}; use tokio::sync::mpsc; -use tracing::{debug, error, trace}; +use tracing::{debug, error, trace, warn}; use winit::application::ApplicationHandler; use winit::dpi::{LogicalPosition, PhysicalSize}; use winit::event::{self, WindowEvent}; @@ -38,7 +39,9 @@ impl App { // SAFETY: We drop the softbuffer context right before the event loop is stopped, thus making this safe. // FIXME: This is not a sufficient proof and the API is actually unsound as-is. let display_handle = unsafe { - core::mem::transmute::, DisplayHandle<'static>>(event_loop.display_handle().unwrap()) + core::mem::transmute::, DisplayHandle<'static>>( + event_loop.display_handle().context("get display handle")?, + ) }; let context = softbuffer::Context::new(display_handle) .map_err(|e| anyhow::anyhow!("unable to initialize softbuffer context: {e}"))?; @@ -56,25 +59,36 @@ impl App { }) } - fn send_resize_event(&mut self) { + fn send_resize_event(&mut self) -> anyhow::Result<()> { let Some(size) = self.last_size.take() else { - return; + return Ok(()); }; let Some((window, _)) = self.window.as_mut() else { - return; + return Ok(()); }; let scale_factor = (window.scale_factor() * 100.0) as u32; - let _ = self.input_event_sender.send(RdpInputEvent::Resize { - width: u16::try_from(size.width).unwrap(), - height: u16::try_from(size.height).unwrap(), - scale_factor, - // TODO: it should be possible to get the physical size here, however winit doesn't make it straightforward. - // FreeRDP does it based on DPI reading grabbed via [`SDL_GetDisplayDPI`](https://wiki.libsdl.org/SDL2/SDL_GetDisplayDPI): - // https://github.com/FreeRDP/FreeRDP/blob/ba8cf8cf2158018fb7abbedb51ab245f369be813/client/SDL/sdl_monitor.cpp#L250-L262 - // See also: https://github.com/rust-windowing/winit/issues/826 - physical_size: None, - }); + let width = u16::try_from(size.width).context("invalid `width`: out of range integral conversion")?; + let height = u16::try_from(size.height).context("invalid `height`: out of range integral conversion")?; + + if self + .input_event_sender + .send(RdpInputEvent::Resize { + width, + height, + scale_factor, + // TODO: it should be possible to get the physical size here, however winit doesn't make it straightforward. + // FreeRDP does it based on DPI reading grabbed via [`SDL_GetDisplayDPI`](https://wiki.libsdl.org/SDL2/SDL_GetDisplayDPI): + // https://github.com/FreeRDP/FreeRDP/blob/ba8cf8cf2158018fb7abbedb51ab245f369be813/client/SDL/sdl_monitor.cpp#L250-L262 + // See also: https://github.com/rust-windowing/winit/issues/826 + physical_size: None, + }) + .is_err() + { + bail!("failed to send resize event"); + }; + + Ok(()) } fn draw(&mut self) { @@ -96,7 +110,9 @@ impl ApplicationHandler for App { if let Some(timeout) = timeout.checked_duration_since(Instant::now()) { event_loop.set_control_flow(ControlFlow::wait_duration(timeout)); } else { - self.send_resize_event(); + if let Err(err) = self.send_resize_event() { + error!(?err, "Failed to send resize event"); + }; self.resize_timeout = None; event_loop.set_control_flow(ControlFlow::Wait); } @@ -160,7 +176,14 @@ impl ApplicationHandler for App { // } WindowEvent::KeyboardInput { event, .. } => { if let Some(scancode) = event.physical_key.to_scancode() { - let scancode = ironrdp::input::Scancode::from_u16(u16::try_from(scancode).unwrap()); + let scancode = match u16::try_from(scancode) { + Ok(scancode) => scancode, + Err(_) => { + warn!("Unsupported scancode: `{scancode:#X}`. Keyboard event will be ignored."); + return; + } + }; + let scancode = ironrdp::input::Scancode::from_u16(scancode); let operation = match event.state { event::ElementState::Pressed => ironrdp::input::Operation::KeyPressed(scancode), @@ -325,13 +348,10 @@ impl ApplicationHandler for App { RdpOutputEvent::Image { buffer, width, height } => { trace!(width = ?width, height = ?height, "Received image with size"); trace!(window_physical_size = ?window.inner_size(), "Drawing image to the window with size"); - self.buffer_size = (width, height); + self.buffer_size = (width.get(), height.get()); self.buffer = buffer; surface - .resize( - NonZeroU32::new(u32::from(width)).unwrap(), - NonZeroU32::new(u32::from(height)).unwrap(), - ) + .resize(NonZeroU32::from(width), NonZeroU32::from(height)) .expect("surface resize"); window.request_redraw(); diff --git a/crates/ironrdp-client/src/config.rs b/crates/ironrdp-client/src/config.rs index a24d7ccac..fbfd8d557 100644 --- a/crates/ironrdp-client/src/config.rs +++ b/crates/ironrdp-client/src/config.rs @@ -439,10 +439,9 @@ impl Config { desktop_scale_factor: 0, // Default to 0 per FreeRDP bitmap: Some(bitmap), client_build: semver::Version::parse(env!("CARGO_PKG_VERSION")) - .map(|version| version.major * 100 + version.minor * 10 + version.patch) - .unwrap_or(0) + .map_or(0, |version| version.major * 100 + version.minor * 10 + version.patch) .pipe(u32::try_from) - .unwrap(), + .context("cargo package version")?, client_name: whoami::fallible::hostname().unwrap_or_else(|_| "ironrdp".to_owned()), // NOTE: hardcode this value like in freerdp // https://github.com/FreeRDP/FreeRDP/blob/4e24b966c86fdf494a782f0dfcfc43a057a2ea60/libfreerdp/core/settings.c#LL49C34-L49C70 diff --git a/crates/ironrdp-client/src/main.rs b/crates/ironrdp-client/src/main.rs index 15caaf99a..18d41f347 100644 --- a/crates/ironrdp-client/src/main.rs +++ b/crates/ironrdp-client/src/main.rs @@ -22,8 +22,8 @@ fn main() -> anyhow::Result<()> { // TODO: get window size & scale factor from GUI/App let window_size = (1024, 768); config.connector.desktop_scale_factor = 0; - config.connector.desktop_size.width = u16::try_from(window_size.0).unwrap(); - config.connector.desktop_size.height = u16::try_from(window_size.1).unwrap(); + config.connector.desktop_size.width = window_size.0; + config.connector.desktop_size.height = window_size.1; let rt = runtime::Builder::new_multi_thread() .enable_all() diff --git a/crates/ironrdp-client/src/rdp.rs b/crates/ironrdp-client/src/rdp.rs index c3f7341a2..394050dd7 100644 --- a/crates/ironrdp-client/src/rdp.rs +++ b/crates/ironrdp-client/src/rdp.rs @@ -1,3 +1,4 @@ +use core::num::NonZeroU16; use std::sync::Arc; use ironrdp::cliprdr::backend::{ClipboardMessage, CliprdrBackendFactory}; @@ -30,11 +31,18 @@ use crate::config::{Config, RDCleanPathConfig}; #[derive(Debug)] pub enum RdpOutputEvent { - Image { buffer: Vec, width: u16, height: u16 }, + Image { + buffer: Vec, + width: NonZeroU16, + height: NonZeroU16, + }, ConnectionFailure(connector::ConnectorError), PointerDefault, PointerHidden, - PointerPosition { x: u16, y: u16 }, + PointerPosition { + x: u16, + y: u16, + }, PointerBitmap(Arc), Terminated(SessionResult), } @@ -516,14 +524,21 @@ async fn active_session( match input_event { RdpInputEvent::Resize { width, height, scale_factor, physical_size } => { trace!(width, height, "Resize event"); - let (width, height) = MonitorLayoutEntry::adjust_display_size(width.into(), height.into()); + let width = u32::from(width); + let height = u32::from(height); + // TODO: Make adjust_display_size take and return width and height as u16. + // From the function's doc comment, the width and height values must be less than or equal to 8192 pixels. + // Therefore, we can remove unnecessary casts from u16 to u32 and back. + let (width, height) = MonitorLayoutEntry::adjust_display_size(width, height); debug!(width, height, "Adjusted display size"); if let Some(response_frame) = active_stage.encode_resize(width, height, Some(scale_factor), physical_size) { vec![ActiveStageOutput::ResponseFrame(response_frame?)] } else { // TODO(#271): use the "auto-reconnect cookie": https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/15b0d1c9-2891-4adb-a45e-deb4aeeeab7c debug!("Reconnecting with new size"); - return Ok(RdpControlFlow::ReconnectWithNewSize { width: width.try_into().unwrap(), height: height.try_into().unwrap() }) + let width = u16::try_from(width).expect("always in the range"); + let height = u16::try_from(height).expect("always in the range"); + return Ok(RdpControlFlow::ReconnectWithNewSize { width, height }) } }, RdpInputEvent::FastPath(events) => { @@ -596,8 +611,10 @@ async fn active_session( event_loop_proxy .send_event(RdpOutputEvent::Image { buffer, - width: image.width(), - height: image.height(), + width: NonZeroU16::new(image.width()) + .ok_or_else(|| session::general_err!("width is zero"))?, + height: NonZeroU16::new(image.height()) + .ok_or_else(|| session::general_err!("height is zero"))?, }) .map_err(|e| session::custom_err!("event_loop_proxy", e))?; } diff --git a/crates/ironrdp-cliprdr-format/src/bitmap.rs b/crates/ironrdp-cliprdr-format/src/bitmap.rs index 5645d4ebb..6fb996cb5 100644 --- a/crates/ironrdp-cliprdr-format/src/bitmap.rs +++ b/crates/ironrdp-cliprdr-format/src/bitmap.rs @@ -283,16 +283,14 @@ impl BitmapInfoHeader { fn width(&self) -> u16 { let abs = self.width.abs(); debug_assert!(abs <= 10_000); - // Per the invariant on self.width, this cast is infallible. - u16::try_from(abs).unwrap() + u16::try_from(abs).expect("Per the invariant on self.width, this cast is infallible") } // INVARIANT: output (height) <= 10_000 fn height(&self) -> u16 { let abs = self.height.abs(); debug_assert!(abs <= 10_000); - // Per the invariant on self.height, this cast is infallible. - u16::try_from(abs).unwrap() + u16::try_from(abs).expect("Per the invariant on self.height, this cast is infallible") } fn is_bottom_up(&self) -> bool { diff --git a/crates/ironrdp-cliprdr/src/pdu/file_contents.rs b/crates/ironrdp-cliprdr/src/pdu/file_contents.rs index daebe6900..0b329af97 100644 --- a/crates/ironrdp-cliprdr/src/pdu/file_contents.rs +++ b/crates/ironrdp-cliprdr/src/pdu/file_contents.rs @@ -101,7 +101,12 @@ impl<'a> FileContentsResponse<'a> { )); } - Ok(u64::from_le_bytes(self.data.as_ref().try_into().unwrap())) + Ok(u64::from_le_bytes( + self.data + .as_ref() + .try_into() + .expect("data contains exactly eight u8 elements"), + )) } } diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index d70b5ddf7..fb7895b67 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -5,7 +5,8 @@ use ironrdp_pdu::rdp::{self}; use tracing::{debug, warn}; use crate::{ - general_err, legacy, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, State, Written, + general_err, legacy, reason_err, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, + State, Written, }; /// Represents the Capability Exchange and Connection Finalization phases @@ -155,7 +156,7 @@ impl Sequence for ConnectionActivationSequence { }); let client_confirm_active = rdp::headers::ShareControlPdu::ClientConfirmActive( - create_client_confirm_active(&self.config, capability_sets, desktop_size), + create_client_confirm_active(&self.config, capability_sets, desktop_size)?, ); debug!(message = ?client_confirm_active, "Send"); @@ -263,7 +264,7 @@ fn create_client_confirm_active( config: &Config, mut server_capability_sets: Vec, desktop_size: DesktopSize, -) -> rdp::capability_sets::ClientConfirmActive { +) -> ConnectorResult { use ironrdp_pdu::rdp::capability_sets::{ client_codecs_capabilities, Bitmap, BitmapCache, BitmapDrawingFlags, Brush, CacheDefinition, CacheEntry, ClientConfirmActive, CmdFlags, DemandActive, FrameAcknowledge, General, GeneralExtraFlags, GlyphCache, @@ -365,13 +366,10 @@ fn create_client_confirm_active( CapabilitySet::SurfaceCommands(SurfaceCommands { flags: CmdFlags::SET_SURFACE_BITS | CmdFlags::STREAM_SURFACE_BITS | CmdFlags::FRAME_MARKER, }), - CapabilitySet::BitmapCodecs( - config - .bitmap - .as_ref() - .map(|b| b.codecs.clone()) - .unwrap_or_else(|| client_codecs_capabilities(&[]).unwrap()), - ), + CapabilitySet::BitmapCodecs(match config.bitmap.as_ref().map(|b| b.codecs.clone()) { + Some(codecs) => codecs, + None => client_codecs_capabilities(&[]).map_err(|e| reason_err!("client codecs capabilities", "{e}"))?, + }), CapabilitySet::FrameAcknowledge(FrameAcknowledge { // FIXME(#447): Revert this to 2 per FreeRDP. // This is a temporary hack to fix a resize bug, see: @@ -389,11 +387,11 @@ fn create_client_confirm_active( })); } - ClientConfirmActive { + Ok(ClientConfirmActive { originator_id: SERVER_CHANNEL_ID, pdu: DemandActive { source_descriptor: "IRONRDP".to_owned(), capability_sets: server_capability_sets, }, - } + }) } diff --git a/crates/ironrdp-displaycontrol/src/pdu/mod.rs b/crates/ironrdp-displaycontrol/src/pdu/mod.rs index 6d7fab014..d89c632f9 100644 --- a/crates/ironrdp-displaycontrol/src/pdu/mod.rs +++ b/crates/ironrdp-displaycontrol/src/pdu/mod.rs @@ -3,7 +3,8 @@ //! [1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpedisp/d2954508-f487-48bc-8731-39743e0854a9 use ironrdp_core::{ - ensure_fixed_part_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor, + cast_length, ensure_fixed_part_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor, + WriteCursor, }; use ironrdp_dvc::DvcEncode; use tracing::warn; @@ -45,11 +46,11 @@ impl Encode for DisplayControlPdu { // This will never overflow as per invariants. #[expect(clippy::arithmetic_side_effects)] - let pdu_size = payload_length + Self::FIXED_PART_SIZE; + let pdu_size = cast_length!("pdu size", payload_length + Self::FIXED_PART_SIZE)?; // Write `DISPLAYCONTROL_HEADER` fields. dst.write_u32(kind); - dst.write_u32(pdu_size.try_into().unwrap()); + dst.write_u32(pdu_size); match self { DisplayControlPdu::Caps(caps) => caps.encode(dst), @@ -87,7 +88,7 @@ impl<'de> Decode<'de> for DisplayControlPdu { let pdu_length = src.read_u32(); let _payload_length = pdu_length - .checked_sub(Self::FIXED_PART_SIZE.try_into().unwrap()) + .checked_sub(Self::FIXED_PART_SIZE.try_into().expect("always in range")) .ok_or_else(|| invalid_field_err!("Length", "Display control PDU length is too small"))?; match kind { @@ -274,7 +275,7 @@ impl DisplayControlMonitorLayout { entry }; - Ok(DisplayControlMonitorLayout::new(&[entry]).unwrap()) + DisplayControlMonitorLayout::new(&[entry]) } pub fn monitors(&self) -> &[MonitorLayoutEntry] { @@ -286,7 +287,7 @@ impl Encode for DisplayControlMonitorLayout { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_fixed_part_size!(in: dst); - dst.write_u32(MonitorLayoutEntry::FIXED_PART_SIZE.try_into().unwrap()); + dst.write_u32(MonitorLayoutEntry::FIXED_PART_SIZE.try_into().expect("always in range")); let monitors_count: u32 = self .monitors @@ -323,20 +324,20 @@ impl<'de> Decode<'de> for DisplayControlMonitorLayout { let monitor_layout_size = src.read_u32(); - if monitor_layout_size != MonitorLayoutEntry::FIXED_PART_SIZE.try_into().unwrap() { + if monitor_layout_size != MonitorLayoutEntry::FIXED_PART_SIZE.try_into().expect("always in range") { return Err(invalid_field_err!( "MonitorLayoutSize", "Monitor layout size is invalid" )); } - let num_monitors = src.read_u32(); + let num_monitors = cast_length!("number of monitors", src.read_u32())?; if num_monitors > MAX_SUPPORTED_MONITORS.into() { return Err(invalid_field_err!("NumMonitors", "Too many monitors")); } - let mut monitors = Vec::with_capacity(usize::try_from(num_monitors).unwrap()); + let mut monitors = Vec::with_capacity(num_monitors); for _ in 0..num_monitors { let monitor = MonitorLayoutEntry::decode(src)?; monitors.push(monitor); diff --git a/crates/ironrdp-dvc/src/client.rs b/crates/ironrdp-dvc/src/client.rs index 25cf7eeaf..532b10001 100644 --- a/crates/ironrdp-dvc/src/client.rs +++ b/crates/ironrdp-dvc/src/client.rs @@ -133,7 +133,10 @@ impl SvcProcessor for DrdynvcClient { // and get any start messages. self.dynamic_channels .attach_channel_id(channel_name.clone(), channel_id); - let dynamic_channel = self.dynamic_channels.get_by_channel_name_mut(&channel_name).unwrap(); + let dynamic_channel = self + .dynamic_channels + .get_by_channel_name_mut(&channel_name) + .expect("channel exists"); (CreationStatus::OK, dynamic_channel.start()?) } else { (CreationStatus::NO_LISTENER, Vec::new()) diff --git a/crates/ironrdp-dvc/src/lib.rs b/crates/ironrdp-dvc/src/lib.rs index bb2a9f6c2..48294324b 100644 --- a/crates/ironrdp-dvc/src/lib.rs +++ b/crates/ironrdp-dvc/src/lib.rs @@ -73,7 +73,7 @@ pub fn encode_dvc_messages( while off < total_length { let first = off == 0; - let remaining_length = total_length.checked_sub(off).unwrap(); + let remaining_length = total_length.checked_sub(off).expect("never overflow"); let size = core::cmp::min(remaining_length, DrdynvcDataPdu::MAX_DATA_SIZE); let end = off .checked_add(size) diff --git a/crates/ironrdp-graphics/src/color_conversion.rs b/crates/ironrdp-graphics/src/color_conversion.rs index 7ce4c8295..fe053d6d7 100644 --- a/crates/ironrdp-graphics/src/color_conversion.rs +++ b/crates/ironrdp-graphics/src/color_conversion.rs @@ -50,7 +50,7 @@ pub fn to_64x64_ycbcr_tile( y: &mut [i16; 64 * 64], cb: &mut [i16; 64 * 64], cr: &mut [i16; 64 * 64], -) { +) -> io::Result<()> { assert!(width <= 64); assert!(height <= 64); @@ -64,17 +64,21 @@ pub fn to_64x64_ycbcr_tile( u_stride: 64, v_plane, v_stride: 64, - width: width.try_into().unwrap(), - height: height.try_into().unwrap(), + width: width.try_into().map_err(io::Error::other)?, + height: height.try_into().map_err(io::Error::other)?, }; - let res = match format { - PixelFormat::RgbA32 | PixelFormat::RgbX32 => rdp_rgba_to_yuv444(&mut plane, input, stride.try_into().unwrap()), - PixelFormat::ARgb32 | PixelFormat::XRgb32 => rdp_argb_to_yuv444(&mut plane, input, stride.try_into().unwrap()), - PixelFormat::BgrA32 | PixelFormat::BgrX32 => rdp_bgra_to_yuv444(&mut plane, input, stride.try_into().unwrap()), - PixelFormat::ABgr32 | PixelFormat::XBgr32 => rdp_abgr_to_yuv444(&mut plane, input, stride.try_into().unwrap()), - }; - res.unwrap(); + let stride = u32::try_from(stride).map_err(io::Error::other)?; + + match format { + PixelFormat::RgbA32 | PixelFormat::RgbX32 => rdp_rgba_to_yuv444(&mut plane, input, stride), + PixelFormat::ARgb32 | PixelFormat::XRgb32 => rdp_argb_to_yuv444(&mut plane, input, stride), + PixelFormat::BgrA32 | PixelFormat::BgrX32 => rdp_bgra_to_yuv444(&mut plane, input, stride), + PixelFormat::ABgr32 | PixelFormat::XBgr32 => rdp_abgr_to_yuv444(&mut plane, input, stride), + } + .map_err(io::Error::other)?; + + Ok(()) } /// Convert a 16-bit RDP color to RGB representation. Input value should be represented in diff --git a/crates/ironrdp-graphics/src/image_processing.rs b/crates/ironrdp-graphics/src/image_processing.rs index 1dc22bbf8..501424eeb 100644 --- a/crates/ironrdp-graphics/src/image_processing.rs +++ b/crates/ironrdp-graphics/src/image_processing.rs @@ -3,8 +3,6 @@ use std::io; use byteorder::WriteBytesExt as _; use ironrdp_pdu::geometry::{InclusiveRectangle, Rectangle as _}; -use num_derive::ToPrimitive; -use num_traits::ToPrimitive as _; const ALPHA_OPAQUE: u8 = 0xff; @@ -99,7 +97,7 @@ impl ImageRegion<'_> { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, ToPrimitive)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum PixelFormat { ARgb32 = 536_971_400, XRgb32 = 536_938_632, @@ -130,6 +128,19 @@ impl TryFrom for PixelFormat { } impl PixelFormat { + fn as_u32(&self) -> u32 { + match self { + Self::ARgb32 => 536_971_400, + Self::XRgb32 => 536_938_632, + Self::ABgr32 => 537_036_936, + Self::XBgr32 => 537_004_168, + Self::BgrA32 => 537_168_008, + Self::BgrX32 => 537_135_240, + Self::RgbA32 => 537_102_472, + Self::RgbX32 => 537_069_704, + } + } + pub const fn bytes_per_pixel(self) -> u8 { match self { Self::ARgb32 @@ -146,7 +157,7 @@ impl PixelFormat { pub fn eq_no_alpha(self, other: Self) -> bool { let mask = !(8 << 12); - (self.to_u32().unwrap() & mask) == (other.to_u32().unwrap() & mask) + (self.as_u32() & mask) == (other.as_u32() & mask) } pub fn read_color(self, buffer: &[u8]) -> io::Result { diff --git a/crates/ironrdp-graphics/src/rdp6/rle.rs b/crates/ironrdp-graphics/src/rdp6/rle.rs index e8540f21e..ae2380334 100644 --- a/crates/ironrdp-graphics/src/rdp6/rle.rs +++ b/crates/ironrdp-graphics/src/rdp6/rle.rs @@ -250,7 +250,7 @@ macro_rules! ensure_size { let available = $buf.len(); let needed = $expected; if !(available >= needed) { - return None; + return Err(RleEncodeError::BufferTooSmall); } }}; } @@ -289,9 +289,7 @@ impl RlePlaneEncoder { } else { match count { 3.. => { - written += self - .encode_segment(&raw, count, dst) - .ok_or(RleEncodeError::BufferTooSmall)?; + written += self.encode_segment(&raw, count, dst)?; raw.clear(); } 2 => raw.extend_from_slice(&[last, last]), @@ -311,14 +309,16 @@ impl RlePlaneEncoder { count = 0; } - written += self - .encode_segment(&raw, count, dst) - .ok_or(RleEncodeError::BufferTooSmall)?; + written += self.encode_segment(&raw, count, dst)?; Ok(written) } - fn encode_segment(&self, mut raw: &[u8], run: usize, dst: &mut WriteCursor<'_>) -> Option { + fn encode_segment(&self, mut raw: &[u8], run: usize, dst: &mut WriteCursor<'_>) -> Result { + if raw.is_empty() { + return Err(RleEncodeError::NotEnoughBytes); + } + let mut extra_bytes = 0; while raw.len() > 15 { @@ -334,14 +334,19 @@ impl RlePlaneEncoder { dst.write_slice(raw); if run > 15 { - let last = raw.last().unwrap(); + let last = raw.last().expect("buffer cannot be empty"); extra_bytes += self.encode_long_sequence(run - 15, *last, dst)?; } - Some(1 + raw.len() + extra_bytes) + Ok(1 + raw.len() + extra_bytes) } - fn encode_long_sequence(&self, mut run: usize, last: u8, dst: &mut WriteCursor<'_>) -> Option { + fn encode_long_sequence( + &self, + mut run: usize, + last: u8, + dst: &mut WriteCursor<'_>, + ) -> Result { let mut written = 0; while run >= 16 { @@ -370,7 +375,7 @@ impl RlePlaneEncoder { } } - Some(written) + Ok(written) } } diff --git a/crates/ironrdp-graphics/src/rlgr.rs b/crates/ironrdp-graphics/src/rlgr.rs index e6aa9cc1f..4fcb496e8 100644 --- a/crates/ironrdp-graphics/src/rlgr.rs +++ b/crates/ironrdp-graphics/src/rlgr.rs @@ -104,37 +104,42 @@ pub fn encode(mode: EntropyAlgorithm, input: &[i16], tile: &mut [u8]) -> Result< kp = kp.saturating_sub(DN_GR); k = kp >> LS_GR; } - CompressionMode::GolombRice => match mode { - EntropyAlgorithm::Rlgr1 => { - let two_ms = get_2magsign(*input.next().unwrap()); - code_gr(&mut bits, &mut krp, two_ms); - if two_ms == 0 { - kp = min(kp + UP_GR, KP_MAX); - } else { - kp = kp.saturating_sub(DQ_GR); - } - k = kp >> LS_GR; - } - EntropyAlgorithm::Rlgr3 => { - let two_ms1 = input.next().map(|&n| get_2magsign(n)).unwrap(); - let two_ms2 = input.next().map(|&n| get_2magsign(n)).unwrap_or(1); - let sum2ms = two_ms1 + two_ms2; - code_gr(&mut bits, &mut krp, sum2ms); - - let m = 32 - sum2ms.leading_zeros() as usize; - if m != 0 { - bits.output_bits(m, two_ms1); + CompressionMode::GolombRice => { + let input_first = *input + .next() + .expect("value is guaranteed to be `Some` due to the prior check"); + match mode { + EntropyAlgorithm::Rlgr1 => { + let two_ms = get_2magsign(input_first); + code_gr(&mut bits, &mut krp, two_ms); + if two_ms == 0 { + kp = min(kp + UP_GR, KP_MAX); + } else { + kp = kp.saturating_sub(DQ_GR); + } + k = kp >> LS_GR; } + EntropyAlgorithm::Rlgr3 => { + let two_ms1 = get_2magsign(input_first); + let two_ms2 = input.next().map(|&n| get_2magsign(n)).unwrap_or(1); + let sum2ms = two_ms1 + two_ms2; + code_gr(&mut bits, &mut krp, sum2ms); + + let m = 32 - sum2ms.leading_zeros() as usize; + if m != 0 { + bits.output_bits(m, two_ms1); + } - if two_ms1 != 0 && two_ms2 != 0 { - kp = kp.saturating_sub(2 * DQ_GR); - k = kp >> LS_GR; - } else if two_ms1 == 0 && two_ms2 == 0 { - kp = min(kp + 2 * UQ_GR, KP_MAX); - k = kp >> LS_GR; + if two_ms1 != 0 && two_ms2 != 0 { + kp = kp.saturating_sub(2 * DQ_GR); + k = kp >> LS_GR; + } else if two_ms1 == 0 && two_ms2 == 0 { + kp = min(kp + 2 * UQ_GR, KP_MAX); + k = kp >> LS_GR; + } } } - }, + } } } diff --git a/crates/ironrdp-graphics/src/zgfx/mod.rs b/crates/ironrdp-graphics/src/zgfx/mod.rs index 4e6a3e47c..90aaa3a2c 100644 --- a/crates/ironrdp-graphics/src/zgfx/mod.rs +++ b/crates/ironrdp-graphics/src/zgfx/mod.rs @@ -71,10 +71,15 @@ impl Decompressor { } fn decompress_segment(&mut self, encoded_data: &[u8], output: &mut Vec) -> Result { + if encoded_data.is_empty() { + return Ok(0); + } + let mut bits = BitSlice::from_slice(encoded_data); // The value of the last byte indicates the number of unused bits in the final byte - bits = &bits[..8 * (encoded_data.len() - 1) - *encoded_data.last().unwrap() as usize]; + bits = + &bits[..8 * (encoded_data.len() - 1) - *encoded_data.last().expect("encoded_data is not empty") as usize]; let mut bits = Bits::new(bits); let mut bytes_written = 0; diff --git a/crates/ironrdp-input/src/lib.rs b/crates/ironrdp-input/src/lib.rs index 628aa98c9..d40d9e535 100644 --- a/crates/ironrdp-input/src/lib.rs +++ b/crates/ironrdp-input/src/lib.rs @@ -362,12 +362,13 @@ impl Database { events.push(event) } + // The keyboard bit array size is 512. for idx in self.keyboard.iter_ones() { let (scancode, extended) = if idx >= 256 { let extended_code = idx.checked_sub(256).expect("never underflow"); - (u8::try_from(extended_code).unwrap(), true) + (u8::try_from(extended_code).expect("always in the range"), true) } else { - (u8::try_from(idx).unwrap(), false) + (u8::try_from(idx).expect("always in the range"), false) }; let mut flags = KeyboardFlags::RELEASE; diff --git a/crates/ironrdp-rdcleanpath/src/lib.rs b/crates/ironrdp-rdcleanpath/src/lib.rs index daff4c1cd..69bfe633d 100644 --- a/crates/ironrdp-rdcleanpath/src/lib.rs +++ b/crates/ironrdp-rdcleanpath/src/lib.rs @@ -314,8 +314,8 @@ pub enum RDCleanPath { } impl RDCleanPath { - pub fn into_pdu(self) -> RDCleanPathPdu { - RDCleanPathPdu::from(self) + pub fn try_into_pdu(self) -> Result { + RDCleanPathPdu::try_from(self) } } @@ -368,8 +368,10 @@ impl TryFrom for RDCleanPath { } } -impl From for RDCleanPathPdu { - fn from(value: RDCleanPath) -> Self { +impl TryFrom for RDCleanPathPdu { + type Error = der::Error; + + fn try_from(value: RDCleanPath) -> Result { match value { RDCleanPath::Request { destination, @@ -377,7 +379,7 @@ impl From for RDCleanPathPdu { server_auth, preconnection_blob, x224_connection_request, - } => Self { + } => Ok(Self { version: VERSION_1, destination: Some(destination), proxy_auth: Some(proxy_auth), @@ -385,26 +387,26 @@ impl From for RDCleanPathPdu { preconnection_blob, x224_connection_pdu: Some(x224_connection_request), ..Default::default() - }, + }), RDCleanPath::Response { x224_connection_response, server_cert_chain, server_addr, - } => Self { + } => Ok(Self { version: VERSION_1, x224_connection_pdu: Some(x224_connection_response), server_cert_chain: Some(server_cert_chain), server_addr: Some(server_addr), ..Default::default() - }, - RDCleanPath::GeneralErr(error) => Self { + }), + RDCleanPath::GeneralErr(error) => Ok(Self { version: VERSION_1, error: Some(error), ..Default::default() - }, + }), RDCleanPath::NegotiationErr { x224_connection_response, - } => Self { + } => Ok(Self { version: VERSION_1, error: Some(RDCleanPathErr { error_code: NEGOTIATION_ERROR_CODE, @@ -412,9 +414,9 @@ impl From for RDCleanPathPdu { wsa_last_error: None, tls_alert_code: None, }), - x224_connection_pdu: Some(OctetString::new(x224_connection_response).unwrap()), + x224_connection_pdu: Some(OctetString::new(x224_connection_response)?), ..Default::default() - }, + }), } } } diff --git a/crates/ironrdp-rdpdr/src/pdu/efs.rs b/crates/ironrdp-rdpdr/src/pdu/efs.rs index 5edd7f33f..621bc15be 100644 --- a/crates/ironrdp-rdpdr/src/pdu/efs.rs +++ b/crates/ironrdp-rdpdr/src/pdu/efs.rs @@ -1329,47 +1329,6 @@ pub struct DeviceControlResponse { pub output_buffer: Option>, } -impl PartialEq for DeviceControlResponse { - fn eq(&self, other: &Self) -> bool { - if (self.device_io_reply != other.device_io_reply) - || (self.output_buffer.is_some() != other.output_buffer.is_some()) - { - return false; - } - - // If both are `None`, they are equal. - if self.output_buffer.is_none() && other.output_buffer.is_none() { - return true; - } - - // device_io_reply is equal and both output_buffers are Some - - // If the sizes are different, the buffers are not equal. - let self_size = self.output_buffer.as_ref().unwrap().size(); - let other_size = other.output_buffer.as_ref().unwrap().size(); - if self_size != other_size { - return false; - } - - // Sizes are the same. Last check is to encode the output buffers and compare the encoded bytes directly. - let mut self_buf = vec![0u8; self_size]; - let mut other_buf = vec![0u8; other_size]; - self.output_buffer - .as_ref() - .unwrap() - .encode(&mut WriteCursor::new(self_buf.as_mut_slice())) - .unwrap(); - other - .output_buffer - .as_ref() - .unwrap() - .encode(&mut WriteCursor::new(other_buf.as_mut_slice())) - .unwrap(); - - self_buf == other_buf - } -} - impl DeviceControlResponse { const NAME: &'static str = "DR_CONTROL_RSP"; @@ -2736,11 +2695,7 @@ impl ClientDriveQueryDirectoryResponse { dst.write_u32(cast_length!( "ClientDriveQueryDirectoryResponse", "length", - if self.buffer.is_some() { - self.buffer.as_ref().unwrap().size() - } else { - 0 - } + self.buffer.as_ref().map_or(0, |buf| buf.size()) )?); if let Some(buffer) = &self.buffer { buffer.encode(dst)?; @@ -3153,11 +3108,7 @@ impl ClientDriveQueryVolumeInformationResponse { dst.write_u32(cast_length!( "ClientDriveQueryVolumeInformationResponse", "length", - if self.buffer.is_some() { - self.buffer.as_ref().unwrap().size() - } else { - 0 - } + self.buffer.as_ref().map_or(0, |buf| buf.size()) )?); if let Some(buffer) = &self.buffer { buffer.encode(dst)?; diff --git a/crates/ironrdp-rdpdr/src/pdu/esc.rs b/crates/ironrdp-rdpdr/src/pdu/esc.rs index 82c584bdf..b7deefde6 100644 --- a/crates/ironrdp-rdpdr/src/pdu/esc.rs +++ b/crates/ironrdp-rdpdr/src/pdu/esc.rs @@ -1836,10 +1836,7 @@ impl rpce::HeaderlessEncode for GetReaderIconReturn { } fn expect_charset(charset: Option) -> DecodeResult { - if charset.is_none() { - return Err(other_err!("internal error: missing character set")); - } - Ok(charset.unwrap()) + charset.ok_or_else(|| other_err!("internal error: missing character set")) } fn expect_no_charset(charset: Option) -> DecodeResult<()> { diff --git a/crates/ironrdp-rdpsnd-native/examples/cpal.rs b/crates/ironrdp-rdpsnd-native/examples/cpal.rs index e7d2f4796..9486abbe6 100644 --- a/crates/ironrdp-rdpsnd-native/examples/cpal.rs +++ b/crates/ironrdp-rdpsnd-native/examples/cpal.rs @@ -43,7 +43,7 @@ fn main() -> anyhow::Result<()> { data: None, }; let (tx, rx) = mpsc::channel(); - let stream = DecodeStream::new(&rx_format, rx).unwrap(); + let stream = DecodeStream::new(&rx_format, rx)?; let producer = thread::spawn(move || { let data_chunks = vec![vec![1u8, 2, 3], vec![4, 5, 6], vec![7, 8, 9]]; diff --git a/crates/ironrdp-rdpsnd-native/src/cpal.rs b/crates/ironrdp-rdpsnd-native/src/cpal.rs index 831b6aa51..c3bddb4cf 100644 --- a/crates/ironrdp-rdpsnd-native/src/cpal.rs +++ b/crates/ironrdp-rdpsnd-native/src/cpal.rs @@ -124,7 +124,9 @@ impl RdpsndClientHandler for RdpsndBackend { if let Some(stream) = self.stream_handle.take() { self.stream_ended.store(true, Ordering::Relaxed); stream.thread().unpark(); - stream.join().unwrap(); + if let Err(err) = stream.join() { + error!(?err, "Failed to join a stream thread"); + } } } } @@ -150,11 +152,26 @@ impl DecodeStream { let mut dec = opus::Decoder::new(rx_format.n_samples_per_sec, chan)?; dec_thread = Some(thread::spawn(move || { while let Ok(pkt) = rx.recv() { - let nb_samples = dec.get_nb_samples(&pkt).unwrap(); + let nb_samples = match dec.get_nb_samples(&pkt) { + Ok(nb_samples) => nb_samples, + Err(err) => { + error!(?err, "Failed to get the number of samples of an Opus packet"); + continue; + } + }; + let mut pcm = vec![0u8; nb_samples * chan as usize * size_of::()]; - dec.decode(&pkt, bytemuck::cast_slice_mut(pcm.as_mut_slice()), false) - .unwrap(); - dec_tx.send(pcm).unwrap(); + if let Err(err) = dec.decode(&pkt, bytemuck::cast_slice_mut(pcm.as_mut_slice()), false) { + error!(?err, "Failed to decode an Opus packet"); + continue; + } + + if dec_tx.send(pcm).is_err() { + error!("Failed to send the decoded Opus packet over the channel"); + // If send has failed, it means that the receiver has been dropped. + // There is no point in continuing the loop in this case. + break; + } } })); rx = dec_rx; diff --git a/crates/ironrdp-rdpsnd/src/pdu/mod.rs b/crates/ironrdp-rdpsnd/src/pdu/mod.rs index d5f3893db..51e1b26d2 100644 --- a/crates/ironrdp-rdpsnd/src/pdu/mod.rs +++ b/crates/ironrdp-rdpsnd/src/pdu/mod.rs @@ -950,7 +950,7 @@ impl Encode for WaveEncryptPdu { fn size(&self) -> usize { Self::FIXED_PART_SIZE .checked_add(self.signature.map_or(0, |_| 8)) - .unwrap() + .expect("never overflow") .checked_add(self.data.len()) .expect("never overflow") } diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index 4d4373678..ad520f949 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -1,6 +1,6 @@ use core::net::SocketAddr; -use anyhow::Result; +use anyhow::{anyhow, Result}; use ironrdp_pdu::rdp::capability_sets::{server_codecs_capabilities, BitmapCodecs}; use tokio_rustls::TlsAcceptor; @@ -113,11 +113,11 @@ impl RdpServerBuilder { } impl RdpServerBuilder { - pub fn with_display_handler(self, display: D) -> RdpServerBuilder + pub fn with_display_handler(self, display: D) -> Result> where D: RdpServerDisplay + 'static, { - RdpServerBuilder { + Ok(RdpServerBuilder { state: BuilderDone { addr: self.state.addr, security: self.state.security, @@ -125,13 +125,14 @@ impl RdpServerBuilder { display: Box::new(display), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]).unwrap(), + codecs: server_codecs_capabilities(&[]) + .map_err(|e| anyhow!("failed to generate server codecs capabilities: {e}"))?, }, - } + }) } - pub fn with_no_display(self) -> RdpServerBuilder { - RdpServerBuilder { + pub fn with_no_display(self) -> Result> { + Ok(RdpServerBuilder { state: BuilderDone { addr: self.state.addr, security: self.state.security, @@ -139,9 +140,10 @@ impl RdpServerBuilder { display: Box::new(NoopDisplay), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]).unwrap(), + codecs: server_codecs_capabilities(&[]) + .map_err(|e| anyhow!("failed to generate server codecs capabilities: {e}"))?, }, - } + }) } } @@ -187,7 +189,7 @@ struct NoopDisplayUpdates; #[async_trait::async_trait] impl RdpServerDisplayUpdates for NoopDisplayUpdates { - async fn next_update(&mut self) -> Option { + async fn next_update(&mut self) -> Result> { let () = core::future::pending().await; unreachable!() } diff --git a/crates/ironrdp-server/src/display.rs b/crates/ironrdp-server/src/display.rs index c0b7c3b0d..2450b2017 100644 --- a/crates/ironrdp-server/src/display.rs +++ b/crates/ironrdp-server/src/display.rs @@ -243,7 +243,7 @@ pub trait RdpServerDisplayUpdates { /// This method MUST be cancellation safe because it is used in a /// `tokio::select!` statement. If some other branch completes first, it /// MUST be guaranteed that no data is lost. - async fn next_update(&mut self) -> Option; + async fn next_update(&mut self) -> Result>; } /// Display for an RDP server @@ -260,8 +260,8 @@ pub trait RdpServerDisplayUpdates { /// /// #[async_trait::async_trait] /// impl RdpServerDisplayUpdates for DisplayUpdates { -/// async fn next_update(&mut self) -> Option { -/// self.receiver.recv().await +/// async fn next_update(&mut self) -> anyhow::Result> { +/// Ok(self.receiver.recv().await) /// } /// } /// diff --git a/crates/ironrdp-server/src/encoder/bitmap.rs b/crates/ironrdp-server/src/encoder/bitmap.rs index 9a1647d0b..e7a78c7b4 100644 --- a/crates/ironrdp-server/src/encoder/bitmap.rs +++ b/crates/ironrdp-server/src/encoder/bitmap.rs @@ -1,8 +1,10 @@ use core::num::NonZeroUsize; -use ironrdp_core::{invalid_field_err, Encode as _, EncodeResult, WriteCursor}; +use ironrdp_core::{cast_int, cast_length, invalid_field_err, Encode as _, WriteCursor}; use ironrdp_graphics::image_processing::PixelFormat; -use ironrdp_graphics::rdp6::{ABgrChannels, ARgbChannels, BgrAChannels, BitmapStreamEncoder, RgbAChannels}; +use ironrdp_graphics::rdp6::{ + ABgrChannels, ARgbChannels, BgrAChannels, BitmapEncodeError, BitmapStreamEncoder, RgbAChannels, +}; use ironrdp_pdu::bitmap::{self, BitmapData, BitmapUpdateData, Compression}; use ironrdp_pdu::geometry::InclusiveRectangle; @@ -21,84 +23,95 @@ impl BitmapEncoder { } } - pub(crate) fn encode(&mut self, bitmap: &BitmapUpdate, output: &mut [u8]) -> EncodeResult { + pub(crate) fn encode(&mut self, bitmap: &BitmapUpdate, output: &mut [u8]) -> Result { // FIXME: support non-multiple of 4 widths. // // It’s not clear how to achieve that yet, but generally, server uses multiple of 4-widths, // and client has surface capabilities, so this path is unlikely. if bitmap.width.get() % 4 != 0 { - return Err(invalid_field_err!("bitmap", "Width must be a multiple of 4")); + return Err(BitmapEncodeError::Encode(invalid_field_err!( + "bitmap", + "Width must be a multiple of 4" + ))); } - let bytes_per_pixel = usize::from(bitmap.format.bytes_per_pixel()); - let row_len = usize::from(bitmap.width.get()) * bytes_per_pixel; - let chunk_height = usize::from(u16::MAX) / row_len; + let bytes_per_pixel = u16::from(bitmap.format.bytes_per_pixel()); + let row_len = bitmap.width.get() * bytes_per_pixel; + let chunk_height = u16::MAX / row_len; let mut cursor = WriteCursor::new(output); let stride = bitmap.stride.get(); - let chunks = bitmap.data.chunks(stride * chunk_height); + let chunks = bitmap.data.chunks(stride * usize::from(chunk_height)); - let total = u16::try_from(chunks.size_hint().0).unwrap(); - BitmapUpdateData::encode_header(total, &mut cursor)?; + let total = cast_int!("chunks length lower bound", chunks.size_hint().0).map_err(BitmapEncodeError::Encode)?; + BitmapUpdateData::encode_header(total, &mut cursor).map_err(BitmapEncodeError::Encode)?; for (i, chunk) in chunks.enumerate() { - let height = chunk.len() / stride; - let top = usize::from(bitmap.y) + i * chunk_height; + let height = cast_int!("bitmap height", chunk.len() / stride).map_err(BitmapEncodeError::Encode)?; + let i: u16 = cast_int!("chunk idx", i).map_err(BitmapEncodeError::Encode)?; + let top = bitmap.y + i * chunk_height; - let encoder = BitmapStreamEncoder::new(NonZeroUsize::from(bitmap.width).get(), height); + let encoder = BitmapStreamEncoder::new(NonZeroUsize::from(bitmap.width).get(), usize::from(height)); let len = { let pixels = chunk .chunks(stride) - .map(|row| &row[..row_len]) + .map(|row| &row[..usize::from(row_len)]) .rev() - .flat_map(|row| row.chunks(bytes_per_pixel)); + .flat_map(|row| row.chunks(usize::from(bytes_per_pixel))); - Self::encode_iter(encoder, bitmap.format, pixels, self.buffer.as_mut_slice()) + Self::encode_iter(encoder, bitmap.format, pixels, self.buffer.as_mut_slice())? }; let data = BitmapData { rectangle: InclusiveRectangle { left: bitmap.x, - top: u16::try_from(top).unwrap(), + top, right: bitmap.x + bitmap.width.get() - 1, - bottom: u16::try_from(top + height - 1).unwrap(), + bottom: top + height - 1, }, width: u16::from(bitmap.width), - height: u16::try_from(height).unwrap(), + height, bits_per_pixel: u16::from(bitmap.format.bytes_per_pixel()) * 8, compression_flags: Compression::BITMAP_COMPRESSION, compressed_data_header: Some(bitmap::CompressedDataHeader { - main_body_size: u16::try_from(len).unwrap(), + main_body_size: cast_length!("main body size", len).map_err(BitmapEncodeError::Encode)?, scan_width: u16::from(bitmap.width), - uncompressed_size: u16::try_from(height * row_len).unwrap(), + uncompressed_size: height * row_len, }), bitmap_data: &self.buffer[..len], }; - data.encode(&mut cursor)?; + data.encode(&mut cursor).map_err(BitmapEncodeError::Encode)?; } Ok(cursor.pos()) } - fn encode_iter<'a, P>(mut encoder: BitmapStreamEncoder, format: PixelFormat, src: P, dst: &mut [u8]) -> usize + fn encode_iter<'a, P>( + mut encoder: BitmapStreamEncoder, + format: PixelFormat, + src: P, + dst: &mut [u8], + ) -> Result where P: Iterator + Clone, { - match format { + let written = match format { PixelFormat::ARgb32 | PixelFormat::XRgb32 => { - encoder.encode_pixels_stream::<_, ARgbChannels>(src, dst, true).unwrap() + encoder.encode_pixels_stream::<_, ARgbChannels>(src, dst, true)? } PixelFormat::RgbA32 | PixelFormat::RgbX32 => { - encoder.encode_pixels_stream::<_, RgbAChannels>(src, dst, true).unwrap() + encoder.encode_pixels_stream::<_, RgbAChannels>(src, dst, true)? } PixelFormat::ABgr32 | PixelFormat::XBgr32 => { - encoder.encode_pixels_stream::<_, ABgrChannels>(src, dst, true).unwrap() + encoder.encode_pixels_stream::<_, ABgrChannels>(src, dst, true)? } PixelFormat::BgrA32 | PixelFormat::BgrX32 => { - encoder.encode_pixels_stream::<_, BgrAChannels>(src, dst, true).unwrap() + encoder.encode_pixels_stream::<_, BgrAChannels>(src, dst, true)? } - } + }; + + Ok(written) } } diff --git a/crates/ironrdp-server/src/encoder/mod.rs b/crates/ironrdp-server/src/encoder/mod.rs index 520cf97d8..7907b8317 100644 --- a/crates/ironrdp-server/src/encoder/mod.rs +++ b/crates/ironrdp-server/src/encoder/mod.rs @@ -1,7 +1,7 @@ use core::fmt; use core::num::NonZeroU16; -use anyhow::{Context as _, Result}; +use anyhow::{anyhow, Context as _, Result}; use ironrdp_acceptor::DesktopSize; use ironrdp_graphics::diff::{find_different_rects_sub, Rect}; use ironrdp_pdu::encode_vec; @@ -23,6 +23,7 @@ mod fast_path; pub(crate) mod rfx; pub(crate) use fast_path::*; +use ironrdp_graphics::rdp6::BitmapEncodeError; #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[repr(u8)] @@ -93,7 +94,7 @@ impl fmt::Debug for UpdateEncoder { impl UpdateEncoder { #[cfg_attr(feature = "__bench", visibility::make(pub))] - pub(crate) fn new(desktop_size: DesktopSize, surface_flags: CmdFlags, codecs: UpdateEncoderCodecs) -> Self { + pub(crate) fn new(desktop_size: DesktopSize, surface_flags: CmdFlags, codecs: UpdateEncoderCodecs) -> Result { let bitmap_updater = if surface_flags.contains(CmdFlags::SET_SURFACE_BITS) { let mut bitmap = BitmapUpdater::None(NoneHandler); @@ -107,7 +108,7 @@ impl UpdateEncoder { } #[cfg(feature = "qoiz")] if let Some(id) = codecs.qoiz { - bitmap = BitmapUpdater::Qoiz(QoizHandler::new(id)); + bitmap = BitmapUpdater::Qoiz(QoizHandler::new(id).context("failed to initialize qoiz handler")?); } bitmap @@ -115,11 +116,11 @@ impl UpdateEncoder { BitmapUpdater::Bitmap(BitmapHandler::new()) }; - Self { + Ok(Self { desktop_size, framebuffer: None, bitmap_updater: Some(bitmap_updater), - } + }) } #[cfg_attr(feature = "__bench", visibility::make(pub))] @@ -246,8 +247,7 @@ impl UpdateEncoder { let result = time_warn!("Encoding bitmap", 10, updater.handle(&bitmap)); (result, updater) }) - .await - .unwrap(); + .await?; self.bitmap_updater = Some(updater); @@ -301,12 +301,31 @@ impl EncoderIter<'_> { return None; }; let Rect { x, y, width, height } = *rect; - let Some(sub) = bitmap.sub( - u16::try_from(x).unwrap(), - u16::try_from(y).unwrap(), - NonZeroU16::new(u16::try_from(width).unwrap()).unwrap(), - NonZeroU16::new(u16::try_from(height).unwrap()).unwrap(), - ) else { + + let x = match u16::try_from(x) { + Ok(x) => x, + Err(_) => return Some(Err(anyhow!("invalid `x`: out of range integral conversion"))), + }; + let y = match u16::try_from(y) { + Ok(y) => y, + Err(_) => return Some(Err(anyhow!("invalid `y`: out of range integral conversion"))), + }; + let width = match u16::try_from(width) { + Ok(width) => match NonZeroU16::new(width) { + Some(width) => width, + None => return Some(Err(anyhow!("rectangle width cannot be zero"))), + }, + Err(_) => return Some(Err(anyhow!("invalid `width`: out of range integral conversion"))), + }; + let height = match u16::try_from(height) { + Ok(height) => match NonZeroU16::new(height) { + Some(height) => height, + None => return Some(Err(anyhow!("rectangle height cannot be zero"))), + }, + Err(_) => return Some(Err(anyhow!("invalid `height`: out of range integral conversion"))), + }; + + let Some(sub) = bitmap.sub(x, y, width, height) else { warn!("Failed to extract bitmap subregion"); return None; }; @@ -398,13 +417,15 @@ impl BitmapUpdateHandler for BitmapHandler { let mut buffer = vec![0; bitmap.data.len() * 2]; // TODO: estimate bitmap encoded size let len = loop { match self.bitmap.encode(bitmap, buffer.as_mut_slice()) { - Err(e) => match e.kind() { - ironrdp_core::EncodeErrorKind::NotEnoughBytes { .. } => { - buffer.resize(buffer.len() * 2, 0); - debug!("encoder buffer resized to: {}", buffer.len() * 2); - } - - _ => Err(e).context("bitmap encode error")?, + Err(err) => match err { + BitmapEncodeError::Encode(e) => match e.kind() { + ironrdp_core::EncodeErrorKind::NotEnoughBytes { .. } => { + buffer.resize(buffer.len() * 2, 0); + debug!("encoder buffer resized to: {}", buffer.len() * 2); + } + _ => Err(e).context("bitmap encode error")?, + }, + BitmapEncodeError::Rle(e) => Err(e).context("bitmap RLE encode error")?, }, Ok(len) => break len, } @@ -495,15 +516,27 @@ impl fmt::Debug for QoizHandler { #[cfg(feature = "qoiz")] impl QoizHandler { - fn new(codec_id: u8) -> Self { + fn new(codec_id: u8) -> Result { let mut zctxt = zstd_safe::CCtx::default(); - zctxt.set_parameter(zstd_safe::CParameter::CompressionLevel(3)).unwrap(); + zctxt + .set_parameter(zstd_safe::CParameter::CompressionLevel(3)) + .map_err(|code| { + anyhow!( + "failed to set Zstd compression level: {}", + zstd_safe::get_error_name(code) + ) + })?; zctxt .set_parameter(zstd_safe::CParameter::EnableLongDistanceMatching(true)) - .unwrap(); + .map_err(|code| { + anyhow!( + "failed to set Zstd enable long distance matching: {}", + zstd_safe::get_error_name(code) + ) + })?; - Self { codec_id, zctxt } + Ok(Self { codec_id, zctxt }) } } @@ -525,7 +558,7 @@ impl BitmapUpdateHandler for QoizHandler { &mut inb, zstd_safe::zstd_sys::ZSTD_EndDirective::ZSTD_e_flush, ) - .map_err(|code| anyhow::anyhow!("failed to zstd compress: {}", zstd_safe::get_error_name(code)))?; + .map_err(|code| anyhow!("failed to Zstd compress: {}", zstd_safe::get_error_name(code)))?; if res == 0 { break; } diff --git a/crates/ironrdp-server/src/encoder/rfx.rs b/crates/ironrdp-server/src/encoder/rfx.rs index 4f9cf2894..6b1b3d2ce 100644 --- a/crates/ironrdp-server/src/encoder/rfx.rs +++ b/crates/ironrdp-server/src/encoder/rfx.rs @@ -1,5 +1,5 @@ use ironrdp_acceptor::DesktopSize; -use ironrdp_core::{cast_length, other_err, Encode as _, EncodeResult}; +use ironrdp_core::{cast_int, cast_length, other_err, Encode as _, EncodeResult}; use ironrdp_graphics::color_conversion::to_64x64_ycbcr_tile; use ironrdp_graphics::rfx_encode_component; use ironrdp_graphics::rlgr::RlgrError; @@ -164,8 +164,8 @@ impl<'a> UpdateEncoder<'a> { y_quant_index: 0, cb_quant_index: 0, cr_quant_index: 0, - x: u16::try_from(tile_x).unwrap(), - y: u16::try_from(tile_y).unwrap(), + x: cast_int!("tile_x", tile_x)?, + y: cast_int!("tile_y", tile_y)?, y_data, cb_data, cr_data, @@ -194,7 +194,7 @@ impl<'a> UpdateEncoder<'a> { let y = &mut [0i16; 4096]; let cb = &mut [0i16; 4096]; let cr = &mut [0i16; 4096]; - to_64x64_ycbcr_tile(input, tile_width, tile_height, stride, self.bitmap.format, y, cb, cr); + to_64x64_ycbcr_tile(input, tile_width, tile_height, stride, self.bitmap.format, y, cb, cr)?; let (y_data, buf) = buf.split_at_mut(4096); let (cb_data, cr_data) = buf.split_at_mut(4096); @@ -227,12 +227,13 @@ pub(crate) mod bench { ) { let (enc, mut data) = UpdateEncoder::new(bitmap, quant.clone(), algo); - enc.encode_tile(tile_x, tile_y, &mut data.0).unwrap(); + enc.encode_tile(tile_x, tile_y, &mut data.0) + .expect("cannot propagate error in benchmark"); } pub fn rfx_enc(bitmap: &BitmapUpdate, quant: &Quant, algo: rfx::EntropyAlgorithm) { let (enc, mut data) = UpdateEncoder::new(bitmap, quant.clone(), algo); - enc.encode(&mut data).unwrap(); + enc.encode(&mut data).expect("cannot propagate error in benchmark"); } } diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 190d6a816..26d174bc5 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -178,7 +178,7 @@ impl DisplayControlHandler for DisplayControlBackend { ///# todo!() ///# } ///# } -///# async fn stub() { +///# async fn stub() -> Result<()> { /// fn make_tls_acceptor() -> TlsAcceptor { /// /* snip */ ///# todo!() @@ -202,10 +202,11 @@ impl DisplayControlHandler for DisplayControlBackend { /// .with_addr(([127, 0, 0, 1], 3389)) /// .with_tls(tls_acceptor) /// .with_input_handler(input_handler) -/// .with_display_handler(display_handler) +/// .with_display_handler(display_handler)? /// .build(); /// /// server.run().await; +/// Ok(()) ///# } /// ``` pub struct RdpServer { @@ -602,7 +603,7 @@ impl RdpServer { let dispatch_display = async move { let mut buffer = vec![0u8; 4096]; loop { - if let Some(update) = display_updates.next_update().await { + if let Some(update) = display_updates.next_update().await? { match Self::dispatch_display_update( update, &mut display_writer, @@ -776,7 +777,8 @@ impl RdpServer { } let desktop_size = self.display.lock().await.size().await; - let encoder = UpdateEncoder::new(desktop_size, surface_flags, update_codecs); + let encoder = UpdateEncoder::new(desktop_size, surface_flags, update_codecs) + .context("failed to initialize update encoder")?; let state = self .client_loop(reader, writer, result.io_channel_id, result.user_channel_id, encoder) diff --git a/crates/ironrdp-session/src/active_stage.rs b/crates/ironrdp-session/src/active_stage.rs index 234071aec..5ae3f80e3 100644 --- a/crates/ironrdp-session/src/active_stage.rs +++ b/crates/ironrdp-session/src/active_stage.rs @@ -224,9 +224,8 @@ impl ActiveStage { physical_dims: Option<(u32, u32)>, ) -> Option>> { if let Some(dvc) = self.get_dvc::() { - if dvc.is_open() { + if let Some(channel_id) = dvc.channel_id() { let display_control = dvc.channel_processor_downcast_ref::()?; - let channel_id = dvc.channel_id().unwrap(); // Safe to unwrap, as we checked if the channel is open let svc_messages = match display_control.encode_single_primary_monitor( channel_id, width, diff --git a/crates/ironrdp-session/src/image.rs b/crates/ironrdp-session/src/image.rs index 9da0379e8..7e27c3f12 100644 --- a/crates/ironrdp-session/src/image.rs +++ b/crates/ironrdp-session/src/image.rs @@ -554,7 +554,11 @@ impl DecodedImage { row.chunks_exact(SRC_COLOR_DEPTH) .enumerate() .for_each(|(col_idx, src_pixel)| { - let rgb16_value = u16::from_le_bytes(src_pixel.try_into().unwrap()); + let rgb16_value = u16::from_le_bytes( + src_pixel + .try_into() + .expect("src_pixel contains exactly two u8 elements"), + ); let dst_idx = ((top + row_idx) * image_width + left + col_idx) * DST_COLOR_DEPTH; let [r, g, b] = rdp_16bit_to_rgb(rgb16_value); @@ -658,16 +662,22 @@ impl DecodedImage { .chunks_exact(rectangle_width * SRC_COLOR_DEPTH) .rev() .enumerate() - .for_each(|(row_idx, row)| { + .try_for_each(|(row_idx, row)| { row.chunks_exact(SRC_COLOR_DEPTH) .enumerate() - .for_each(|(col_idx, src_pixel)| { + .try_for_each(|(col_idx, src_pixel)| { let dst_idx = ((top + row_idx) * image_width + left + col_idx) * DST_COLOR_DEPTH; - let c = format.read_color(src_pixel).unwrap(); + let c = format + .read_color(src_pixel) + .map_err(|err| custom_err!("read color", err))?; self.data[dst_idx..dst_idx + SRC_COLOR_DEPTH].copy_from_slice(&[c.r, c.g, c.b, c.a]); - }) - }); + + Ok(()) + })?; + + Ok(()) + })?; } let update_rectangle = self.pointer_rendering_end(pointer_rendering_state)?; diff --git a/crates/ironrdp-session/src/rfx.rs b/crates/ironrdp-session/src/rfx.rs index 89edea452..7bd21d6d0 100644 --- a/crates/ironrdp-session/src/rfx.rs +++ b/crates/ironrdp-session/src/rfx.rs @@ -108,7 +108,11 @@ impl DecodingContext { image: &mut DecodedImage, destination: &InclusiveRectangle, ) -> SessionResult<(FrameId, InclusiveRectangle)> { - let channel = self.channels.0.first().unwrap(); + let channel = self + .channels + .0 + .first() + .ok_or_else(|| general_err!("no RFX channel found"))?; let width = channel.width.try_into().map_err(|_| general_err!("invalid width"))?; let height = channel.height.try_into().map_err(|_| general_err!("invalid height"))?; let entropy_algorithm = self.context.entropy_algorithm; diff --git a/crates/ironrdp-testsuite-core/src/lib.rs b/crates/ironrdp-testsuite-core/src/lib.rs index aeb08ee53..3318fa493 100644 --- a/crates/ironrdp-testsuite-core/src/lib.rs +++ b/crates/ironrdp-testsuite-core/src/lib.rs @@ -5,6 +5,7 @@ #![allow(clippy::cast_possible_wrap)] #![allow(clippy::cast_sign_loss)] #![allow(unused_crate_dependencies)] +#![allow(clippy::unwrap_used, reason = "unwrap is fine in tests")] mod macros; diff --git a/crates/ironrdp-testsuite-core/tests/graphics/color_conversion.rs b/crates/ironrdp-testsuite-core/tests/graphics/color_conversion.rs index 585eb6a9a..1e63d887f 100644 --- a/crates/ironrdp-testsuite-core/tests/graphics/color_conversion.rs +++ b/crates/ironrdp-testsuite-core/tests/graphics/color_conversion.rs @@ -8,7 +8,7 @@ fn to_64x64_ycbcr() { let mut y = [0; 64 * 64]; let mut cb = [0; 64 * 64]; let mut cr = [0; 64 * 64]; - to_64x64_ycbcr_tile(&input, 1, 1, 4, PixelFormat::ABgr32, &mut y, &mut cb, &mut cr); + to_64x64_ycbcr_tile(&input, 1, 1, 4, PixelFormat::ABgr32, &mut y, &mut cb, &mut cr).unwrap(); } #[ignore] @@ -24,7 +24,7 @@ fn rgb_to_ycbcr_converts_large_buffer() { let mut y = [0; 4096]; let mut cb = [0; 4096]; let mut cr = [0; 4096]; - to_64x64_ycbcr_tile(xrgb, 64, 64, 64 * 4, PixelFormat::XRgb32, &mut y, &mut cb, &mut cr); + to_64x64_ycbcr_tile(xrgb, 64, 64, 64 * 4, PixelFormat::XRgb32, &mut y, &mut cb, &mut cr).unwrap(); assert_eq!(expected.y, y.as_slice()); } diff --git a/crates/ironrdp-testsuite-core/tests/main.rs b/crates/ironrdp-testsuite-core/tests/main.rs index 246feea82..ba011f85c 100644 --- a/crates/ironrdp-testsuite-core/tests/main.rs +++ b/crates/ironrdp-testsuite-core/tests/main.rs @@ -1,5 +1,6 @@ #![allow(unused_crate_dependencies)] // false positives because there is both a library and a binary #![allow(clippy::panic, reason = "panic is acceptable in tests")] +#![allow(clippy::unwrap_used, reason = "unwrap is fine in tests")] //! Integration Tests (IT) //! //! Integration tests are all contained in this single crate, and organized in modules. diff --git a/crates/ironrdp-testsuite-extra/tests/tests.rs b/crates/ironrdp-testsuite-extra/tests/tests.rs index 29bd2af2f..c8da8e36f 100644 --- a/crates/ironrdp-testsuite-extra/tests/tests.rs +++ b/crates/ironrdp-testsuite-extra/tests/tests.rs @@ -1,4 +1,5 @@ #![allow(unused_crate_dependencies)] // false positives because there is both a library and a binary +#![allow(clippy::unwrap_used, reason = "unwrap is fine in tests")] use core::future::Future; use std::path::Path; @@ -122,10 +123,10 @@ struct TestDisplayUpdates { #[async_trait::async_trait] impl RdpServerDisplayUpdates for TestDisplayUpdates { - async fn next_update(&mut self) -> Option { + async fn next_update(&mut self) -> Result> { let mut rx = self.rx.lock().await; - rx.recv().await + Ok(rx.recv().await) } } @@ -177,6 +178,7 @@ where .with_display_handler(TestDisplay { rx: Arc::new(Mutex::new(display_rx)), }) + .unwrap() .build(); server.set_credentials(Some(server::Credentials { username: USERNAME.into(), diff --git a/crates/ironrdp-web/src/canvas.rs b/crates/ironrdp-web/src/canvas.rs index 322c05068..1c8b776d6 100644 --- a/crates/ironrdp-web/src/canvas.rs +++ b/crates/ironrdp-web/src/canvas.rs @@ -5,14 +5,14 @@ use softbuffer::{NoDisplayHandle, NoWindowHandle}; use web_sys::HtmlCanvasElement; pub(crate) struct Canvas { - width: u32, + width: NonZeroU32, surface: softbuffer::Surface, } impl Canvas { - pub(crate) fn new(render_canvas: HtmlCanvasElement, width: u32, height: u32) -> anyhow::Result { - render_canvas.set_width(width); - render_canvas.set_height(height); + pub(crate) fn new(render_canvas: HtmlCanvasElement, width: NonZeroU32, height: NonZeroU32) -> anyhow::Result { + render_canvas.set_width(width.get()); + render_canvas.set_height(height.get()); #[cfg(target_arch = "wasm32")] let mut surface = { @@ -29,16 +29,14 @@ impl Canvas { stub(render_canvas) }; - surface - .resize(NonZeroU32::new(width).unwrap(), NonZeroU32::new(height).unwrap()) - .expect("surface resize"); + surface.resize(width, height).expect("surface resize"); Ok(Self { width, surface }) } pub(crate) fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) { self.surface.resize(width, height).expect("surface resize"); - self.width = width.get(); + self.width = width; } pub(crate) fn draw(&mut self, buffer: &[u8], region: InclusiveRectangle) -> anyhow::Result<()> { @@ -63,7 +61,7 @@ impl Canvas { let region_width_usize = usize::from(region_width); for dst_row in dst - .chunks_exact_mut(self.width as usize) + .chunks_exact_mut(self.width.get() as usize) .skip(region_top_usize) .take(region_height_usize) { @@ -81,8 +79,8 @@ impl Canvas { let damage_rect = softbuffer::Rect { x: u32::from(region.left), y: u32::from(region.top), - width: NonZeroU32::new(u32::from(region_width)).unwrap(), - height: NonZeroU32::new(u32::from(region_height)).unwrap(), + width: NonZeroU32::new(u32::from(region_width)).expect("Per invariants: 0 < region_width"), + height: NonZeroU32::new(u32::from(region_height)).expect("Per invariants: 0 < region_height"), }; dst.present_with_damage(&[damage_rect]).expect("buffer present"); diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index a29ee2e1b..2c7f8dd18 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -5,7 +5,7 @@ use core::time::Duration; use std::borrow::Cow; use std::rc::Rc; -use anyhow::Context as _; +use anyhow::{anyhow, Context as _}; use base64::Engine as _; use futures_channel::mpsc; use futures_util::io::{ReadHalf, WriteHalf}; @@ -280,7 +280,8 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { info!("Connect to RDP host"); - let mut config = build_config(username, password, server_domain, client_name, desktop_size); + let mut config = + build_config(username, password, server_domain, client_name, desktop_size).context("build config")?; let enable_credssp = self.0.borrow().enable_credssp; config.enable_credssp = enable_credssp; @@ -477,12 +478,13 @@ impl iron_remote_desktop::Session for Session { debug!("Initialize canvas"); - let mut gui = Canvas::new( - self.render_canvas.clone(), - u32::from(connection_result.desktop_size.width), - u32::from(connection_result.desktop_size.height), - ) - .context("canvas initialization")?; + let desktop_width = NonZeroU32::new(u32::from(connection_result.desktop_size.width)) + .ok_or_else(|| anyhow!("desktop width is zero"))?; + let desktop_height = NonZeroU32::new(u32::from(connection_result.desktop_size.height)) + .ok_or_else(|| anyhow!("desktop height is zero"))?; + + let mut gui = + Canvas::new(self.render_canvas.clone(), desktop_width, desktop_height).context("canvas initialization")?; debug!("Canvas initialized"); @@ -559,7 +561,10 @@ impl iron_remote_desktop::Session for Session { warn!("Resize event ignored: width or height is zero"); Vec::new() } else if let Some(response_frame) = active_stage.encode_resize(width, height, scale_factor, physical_size) { - requested_resize = Some((NonZeroU32::new(width).unwrap(), NonZeroU32::new(height).unwrap())); + let width = NonZeroU32::new(width).expect("width is guaranteed to be non-zero due to the prior check"); + let height = NonZeroU32::new(height).expect("height is guaranteed to be non-zero due to the prior check"); + + requested_resize = Some((width, height)); vec![ActiveStageOutput::ResponseFrame(response_frame?)] } else { debug!("Resize event ignored"); @@ -844,8 +849,8 @@ fn build_config( domain: Option, client_name: String, desktop_size: DesktopSize, -) -> connector::Config { - connector::Config { +) -> anyhow::Result { + Ok(connector::Config { credentials: Credentials::UsernamePassword { username, password }, domain, // TODO(#327): expose these options from the WASM module. @@ -864,14 +869,12 @@ fn build_config( bitmap: Some(connector::BitmapConfig { color_depth: 16, lossy_compression: true, - codecs: client_codecs_capabilities(&[]).unwrap(), + codecs: client_codecs_capabilities(&[]).map_err(|err| anyhow::anyhow!("client codecs capabilities error: {err}"))?, }), #[expect(clippy::arithmetic_side_effects)] // fine unless we end up with an insanely big version client_build: semver::Version::parse(env!("CARGO_PKG_VERSION")) - .map(|version| version.major * 100 + version.minor * 10 + version.patch) - .unwrap_or(0) - .pipe(u32::try_from) - .unwrap(), + .map_or(0, |version| version.major * 100 + version.minor * 10 + version.patch) + .pipe(u32::try_from).context("cargo package version")?, client_name, // NOTE: hardcode this value like in freerdp // https://github.com/FreeRDP/FreeRDP/blob/4e24b966c86fdf494a782f0dfcfc43a057a2ea60/libfreerdp/core/settings.c#LL49C34-L49C70 @@ -887,7 +890,7 @@ fn build_config( hardware_id: None, license_cache: None, timezone_info: TimezoneInfo::default(), - } + }) } async fn writer_task( diff --git a/crates/ironrdp/examples/screenshot.rs b/crates/ironrdp/examples/screenshot.rs index 431b86a29..94144ce6a 100644 --- a/crates/ironrdp/examples/screenshot.rs +++ b/crates/ironrdp/examples/screenshot.rs @@ -302,7 +302,10 @@ fn active_stage( fn lookup_addr(hostname: &str, port: u16) -> anyhow::Result { use std::net::ToSocketAddrs as _; - let addr = (hostname, port).to_socket_addrs()?.next().unwrap(); + let addr = (hostname, port) + .to_socket_addrs()? + .next() + .context("socket address not found")?; Ok(addr) } @@ -327,7 +330,7 @@ fn tls_upgrade( let config = std::sync::Arc::new(config); - let server_name = server_name.try_into().unwrap(); + let server_name = server_name.try_into()?; let client = rustls::ClientConnection::new(config, server_name)?; diff --git a/crates/ironrdp/examples/server.rs b/crates/ironrdp/examples/server.rs index 57e488e9a..7a346f3dd 100644 --- a/crates/ironrdp/examples/server.rs +++ b/crates/ironrdp/examples/server.rs @@ -4,7 +4,7 @@ #![allow(clippy::print_stdout)] use core::net::SocketAddr; -use core::num::{NonZero, NonZeroU16, NonZeroUsize}; +use core::num::{NonZeroU16, NonZeroUsize}; use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -153,21 +153,24 @@ struct DisplayUpdates; #[async_trait::async_trait] impl RdpServerDisplayUpdates for DisplayUpdates { - async fn next_update(&mut self) -> Option { + async fn next_update(&mut self) -> anyhow::Result> { sleep(Duration::from_millis(100)).await; let mut rng = rand::rng(); let y: u16 = rng.random_range(0..HEIGHT); - let height = NonZeroU16::new(rng.random_range(1..=HEIGHT.checked_sub(y).unwrap())).unwrap(); + let height = rng.random_range(1..=HEIGHT.checked_sub(y).expect("never underflow")); + let height = NonZeroU16::new(height).expect("never zero"); + let x: u16 = rng.random_range(0..WIDTH); - let width = NonZeroU16::new(rng.random_range(1..=WIDTH.checked_sub(x).unwrap())).unwrap(); + let width = rng.random_range(1..=WIDTH.checked_sub(x).expect("never underflow")); + let width = NonZeroU16::new(width).expect("never zero"); + let capacity = NonZeroUsize::from(width) .checked_mul(NonZeroUsize::from(height)) - .unwrap() + .expect("never overflow") .get() .checked_mul(4) - .unwrap(); - + .expect("never overflow"); let mut data = Vec::with_capacity(capacity); for _ in 0..(data.capacity() / 4) { data.push(rng.random()); @@ -177,7 +180,9 @@ impl RdpServerDisplayUpdates for DisplayUpdates { } info!("get_update +{x}+{y} {width}x{height}"); - let stride = NonZeroUsize::from(width).checked_mul(NonZero::new(4).unwrap()).unwrap(); + let stride = NonZeroUsize::from(width) + .checked_mul(NonZeroUsize::new(4).expect("never zero")) + .expect("never overflow"); let bitmap = BitmapUpdate { x, y, @@ -187,7 +192,7 @@ impl RdpServerDisplayUpdates for DisplayUpdates { data: data.into(), stride, }; - Some(DisplayUpdate::Bitmap(bitmap)) + Ok(Some(DisplayUpdate::Bitmap(bitmap))) } } @@ -230,7 +235,13 @@ struct StubSoundServerFactory { impl ServerEventSender for StubSoundServerFactory { fn set_sender(&mut self, sender: UnboundedSender) { - let mut inner = self.inner.lock().unwrap(); + let mut inner = match self.inner.lock() { + Ok(inner) => inner, + Err(e) => { + warn!("Failed to acquire the mutex: {e:?}"); + return; + } + }; inner.ev_sender = Some(sender); } @@ -337,7 +348,13 @@ impl RdpsndServerHandler for SndHandler { wave.into_iter().flat_map(|value| value.to_le_bytes()).collect() }; - let inner = inner.lock().unwrap(); + let inner = match inner.lock() { + Ok(inner) => inner, + Err(e) => { + warn!("Failed to acquire the mutex: {e:?}"); + return; + } + }; if let Some(sender) = inner.ev_sender.as_ref() { let _ = sender.send(ServerEvent::Rdpsnd(RdpsndServerMessage::Wave(data, ts))); } @@ -418,7 +435,7 @@ async fn run( let mut server = server_builder .with_input_handler(handler.clone()) - .with_display_handler(handler.clone()) + .with_display_handler(handler.clone())? .with_cliprdr_factory(Some(cliprdr)) .with_sound_factory(Some(sound)) .build(); diff --git a/ffi/build.rs b/ffi/build.rs index 4f2cacea7..98bb25679 100644 --- a/ffi/build.rs +++ b/ffi/build.rs @@ -4,7 +4,7 @@ use other::main_stub; use win::main_stub; fn main() { - main_stub(); + main_stub() } #[cfg(target_os = "windows")] @@ -19,7 +19,8 @@ mod win { let company_name = "Devolutions Inc."; let legal_copyright = format!("Copyright 2019-2024 {company_name}"); - let mut cargo_version = env::var("CARGO_PKG_VERSION").unwrap(); + let mut cargo_version = + env::var("CARGO_PKG_VERSION").expect("failed to fetch `CARGO_PKG_VERSION` environment variable"); cargo_version.push_str(".0"); let version_number = cargo_version; @@ -74,14 +75,15 @@ END } pub(crate) fn main_stub() { - let out_dir = env::var("OUT_DIR").unwrap(); + let out_dir = env::var("OUT_DIR").expect("failed to fetch `OUT_DIR` environment variable"); let version_rc_file = format!("{out_dir}/version.rc"); let version_rc_data = generate_version_rc(); - let mut file = File::create(&version_rc_file).expect("cannot create version.rc file"); - file.write_all(version_rc_data.as_bytes()).unwrap(); + let mut file = File::create(&version_rc_file).expect("failed to create version.rc file"); + file.write_all(version_rc_data.as_bytes()) + .expect("failed to write data to version.rc file"); embed_resource::compile(&version_rc_file, embed_resource::NONE) .manifest_required() - .unwrap(); + .expect("failed to compiler the Windows resource file"); } } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index e7b4e6f5b..cf4220282 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -1,6 +1,5 @@ #![allow(clippy::print_stdout)] #![allow(clippy::print_stderr)] -#![allow(clippy::unwrap_used)] #![allow(unreachable_pub)] mod macros; @@ -135,7 +134,7 @@ fn project_root() -> PathBuf { Path::new(&env!("CARGO_MANIFEST_DIR")) .ancestors() .nth(1) - .unwrap() + .expect("failed to retrieve project root path") .to_path_buf() } From 4e99fd5055bc995e3e924f54a0d068d71d540928 Mon Sep 17 00:00:00 2001 From: Alexandr Yusuk Date: Fri, 5 Sep 2025 12:40:34 +0300 Subject: [PATCH 2/8] review: adddress Benoit's comments --- crates/ironrdp-ainput/src/lib.rs | 18 +++++--- crates/ironrdp-client/src/app.rs | 46 ++++++++----------- crates/ironrdp-cliprdr-format/src/bitmap.rs | 4 +- .../ironrdp-graphics/src/color_conversion.rs | 19 +++----- crates/ironrdp-graphics/src/rdp6/rle.rs | 2 +- crates/ironrdp-graphics/src/rlgr.rs | 4 ++ crates/ironrdp-server/src/builder.rs | 8 ++-- crates/ironrdp-server/src/encoder/rfx.rs | 8 +++- crates/ironrdp-web/src/canvas.rs | 4 +- crates/ironrdp-web/src/session.rs | 8 ++-- crates/ironrdp/examples/server.rs | 9 +--- 11 files changed, 59 insertions(+), 71 deletions(-) diff --git a/crates/ironrdp-ainput/src/lib.rs b/crates/ironrdp-ainput/src/lib.rs index c6ad27e2d..8e040ec77 100644 --- a/crates/ironrdp-ainput/src/lib.rs +++ b/crates/ironrdp-ainput/src/lib.rs @@ -99,10 +99,12 @@ pub enum ServerPduType { } impl ServerPduType { + #[expect( + clippy::as_conversions, + reason = "guarantees discriminant layout, and as is the only way to cast enum -> primitive" + )] fn as_u16(&self) -> u16 { - match self { - Self::Version => 0x01, - } + *self as u16 } } @@ -268,10 +270,12 @@ pub enum ClientPduType { } impl ClientPduType { - fn as_u16(&self) -> u16 { - match self { - Self::Mouse => 0x02, - } + #[expect( + clippy::as_conversions, + reason = "guarantees discriminant layout, and as is the only way to cast enum -> primitive" + )] + fn as_u16(self) -> u16 { + self as u16 } } diff --git a/crates/ironrdp-client/src/app.rs b/crates/ironrdp-client/src/app.rs index 6a7e1ff56..48cde764b 100644 --- a/crates/ironrdp-client/src/app.rs +++ b/crates/ironrdp-client/src/app.rs @@ -5,7 +5,7 @@ use core::time::Duration; use std::sync::Arc; use std::time::Instant; -use anyhow::{bail, Context as _}; +use anyhow::Context as _; use raw_window_handle::{DisplayHandle, HasDisplayHandle as _}; use tokio::sync::mpsc; use tracing::{debug, error, trace, warn}; @@ -59,36 +59,28 @@ impl App { }) } - fn send_resize_event(&mut self) -> anyhow::Result<()> { + fn send_resize_event(&mut self) { let Some(size) = self.last_size.take() else { - return Ok(()); + return; }; let Some((window, _)) = self.window.as_mut() else { - return Ok(()); + return; }; let scale_factor = (window.scale_factor() * 100.0) as u32; - let width = u16::try_from(size.width).context("invalid `width`: out of range integral conversion")?; - let height = u16::try_from(size.height).context("invalid `height`: out of range integral conversion")?; - - if self - .input_event_sender - .send(RdpInputEvent::Resize { - width, - height, - scale_factor, - // TODO: it should be possible to get the physical size here, however winit doesn't make it straightforward. - // FreeRDP does it based on DPI reading grabbed via [`SDL_GetDisplayDPI`](https://wiki.libsdl.org/SDL2/SDL_GetDisplayDPI): - // https://github.com/FreeRDP/FreeRDP/blob/ba8cf8cf2158018fb7abbedb51ab245f369be813/client/SDL/sdl_monitor.cpp#L250-L262 - // See also: https://github.com/rust-windowing/winit/issues/826 - physical_size: None, - }) - .is_err() - { - bail!("failed to send resize event"); - }; - - Ok(()) + let width = u16::try_from(size.width).expect("width is too big(more than u16::MAX)"); + let height = u16::try_from(size.height).expect("height is too big(more than u16::MAX)"); + + let _ = self.input_event_sender.send(RdpInputEvent::Resize { + width, + height, + scale_factor, + // TODO: it should be possible to get the physical size here, however winit doesn't make it straightforward. + // FreeRDP does it based on DPI reading grabbed via [`SDL_GetDisplayDPI`](https://wiki.libsdl.org/SDL2/SDL_GetDisplayDPI): + // https://github.com/FreeRDP/FreeRDP/blob/ba8cf8cf2158018fb7abbedb51ab245f369be813/client/SDL/sdl_monitor.cpp#L250-L262 + // See also: https://github.com/rust-windowing/winit/issues/826 + physical_size: None, + }); } fn draw(&mut self) { @@ -110,9 +102,7 @@ impl ApplicationHandler for App { if let Some(timeout) = timeout.checked_duration_since(Instant::now()) { event_loop.set_control_flow(ControlFlow::wait_duration(timeout)); } else { - if let Err(err) = self.send_resize_event() { - error!(?err, "Failed to send resize event"); - }; + self.send_resize_event(); self.resize_timeout = None; event_loop.set_control_flow(ControlFlow::Wait); } diff --git a/crates/ironrdp-cliprdr-format/src/bitmap.rs b/crates/ironrdp-cliprdr-format/src/bitmap.rs index 6fb996cb5..0dfcab860 100644 --- a/crates/ironrdp-cliprdr-format/src/bitmap.rs +++ b/crates/ironrdp-cliprdr-format/src/bitmap.rs @@ -283,14 +283,14 @@ impl BitmapInfoHeader { fn width(&self) -> u16 { let abs = self.width.abs(); debug_assert!(abs <= 10_000); - u16::try_from(abs).expect("Per the invariant on self.width, this cast is infallible") + u16::try_from(abs).expect("per the invariant on self.width, this cast is infallible") } // INVARIANT: output (height) <= 10_000 fn height(&self) -> u16 { let abs = self.height.abs(); debug_assert!(abs <= 10_000); - u16::try_from(abs).expect("Per the invariant on self.height, this cast is infallible") + u16::try_from(abs).expect("per the invariant on self.height, this cast is infallible") } fn is_bottom_up(&self) -> bool { diff --git a/crates/ironrdp-graphics/src/color_conversion.rs b/crates/ironrdp-graphics/src/color_conversion.rs index fe053d6d7..46f78ad67 100644 --- a/crates/ironrdp-graphics/src/color_conversion.rs +++ b/crates/ironrdp-graphics/src/color_conversion.rs @@ -2,7 +2,7 @@ use std::io; use yuv::{ rdp_abgr_to_yuv444, rdp_argb_to_yuv444, rdp_bgra_to_yuv444, rdp_rgba_to_yuv444, rdp_yuv444_to_argb, - rdp_yuv444_to_rgba, BufferStoreMut, YuvPlanarImage, YuvPlanarImageMut, + rdp_yuv444_to_rgba, BufferStoreMut, YuvError, YuvPlanarImage, YuvPlanarImageMut, }; use crate::image_processing::PixelFormat; @@ -43,14 +43,14 @@ pub fn ycbcr_to_rgba(input: YCbCrBuffer<'_>, output: &mut [u8]) -> io::Result<() #[expect(clippy::too_many_arguments)] pub fn to_64x64_ycbcr_tile( input: &[u8], - width: usize, - height: usize, - stride: usize, + width: u32, + height: u32, + stride: u32, format: PixelFormat, y: &mut [i16; 64 * 64], cb: &mut [i16; 64 * 64], cr: &mut [i16; 64 * 64], -) -> io::Result<()> { +) -> Result<(), YuvError> { assert!(width <= 64); assert!(height <= 64); @@ -64,21 +64,16 @@ pub fn to_64x64_ycbcr_tile( u_stride: 64, v_plane, v_stride: 64, - width: width.try_into().map_err(io::Error::other)?, - height: height.try_into().map_err(io::Error::other)?, + width, + height, }; - let stride = u32::try_from(stride).map_err(io::Error::other)?; - match format { PixelFormat::RgbA32 | PixelFormat::RgbX32 => rdp_rgba_to_yuv444(&mut plane, input, stride), PixelFormat::ARgb32 | PixelFormat::XRgb32 => rdp_argb_to_yuv444(&mut plane, input, stride), PixelFormat::BgrA32 | PixelFormat::BgrX32 => rdp_bgra_to_yuv444(&mut plane, input, stride), PixelFormat::ABgr32 | PixelFormat::XBgr32 => rdp_abgr_to_yuv444(&mut plane, input, stride), } - .map_err(io::Error::other)?; - - Ok(()) } /// Convert a 16-bit RDP color to RGB representation. Input value should be represented in diff --git a/crates/ironrdp-graphics/src/rdp6/rle.rs b/crates/ironrdp-graphics/src/rdp6/rle.rs index ae2380334..7f74cb572 100644 --- a/crates/ironrdp-graphics/src/rdp6/rle.rs +++ b/crates/ironrdp-graphics/src/rdp6/rle.rs @@ -249,7 +249,7 @@ macro_rules! ensure_size { (dst: $buf:ident, size: $expected:expr) => {{ let available = $buf.len(); let needed = $expected; - if !(available >= needed) { + if !(needed <= available) { return Err(RleEncodeError::BufferTooSmall); } }}; diff --git a/crates/ironrdp-graphics/src/rlgr.rs b/crates/ironrdp-graphics/src/rlgr.rs index 4fcb496e8..502ef0950 100644 --- a/crates/ironrdp-graphics/src/rlgr.rs +++ b/crates/ironrdp-graphics/src/rlgr.rs @@ -4,6 +4,7 @@ use std::io; use bitvec::field::BitField as _; use bitvec::prelude::*; use ironrdp_pdu::codecs::rfx::EntropyAlgorithm; +use yuv::YuvError; use crate::utils::Bits; @@ -360,6 +361,7 @@ impl From for CompressionMode { #[derive(Debug)] pub enum RlgrError { IoError(io::Error), + YuvError(YuvError), EmptyTile, } @@ -368,6 +370,7 @@ impl core::fmt::Display for RlgrError { match self { Self::IoError(_error) => write!(f, "IO error"), Self::EmptyTile => write!(f, "the input tile is empty"), + Self::YuvError(error) => write!(f, "YUV error: {error}"), } } } @@ -376,6 +379,7 @@ impl core::error::Error for RlgrError { fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { match self { Self::IoError(error) => Some(error), + Self::YuvError(error) => Some(error), Self::EmptyTile => None, } } diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index ad520f949..639981f80 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -1,6 +1,6 @@ use core::net::SocketAddr; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use ironrdp_pdu::rdp::capability_sets::{server_codecs_capabilities, BitmapCodecs}; use tokio_rustls::TlsAcceptor; @@ -125,8 +125,7 @@ impl RdpServerBuilder { display: Box::new(display), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]) - .map_err(|e| anyhow!("failed to generate server codecs capabilities: {e}"))?, + codecs: server_codecs_capabilities(&[]).expect("can't panic"), }, }) } @@ -140,8 +139,7 @@ impl RdpServerBuilder { display: Box::new(NoopDisplay), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]) - .map_err(|e| anyhow!("failed to generate server codecs capabilities: {e}"))?, + codecs: server_codecs_capabilities(&[]).expect("can't panic"), }, }) } diff --git a/crates/ironrdp-server/src/encoder/rfx.rs b/crates/ironrdp-server/src/encoder/rfx.rs index 6b1b3d2ce..0ffba651b 100644 --- a/crates/ironrdp-server/src/encoder/rfx.rs +++ b/crates/ironrdp-server/src/encoder/rfx.rs @@ -1,3 +1,5 @@ +use std::io; + use ironrdp_acceptor::DesktopSize; use ironrdp_core::{cast_int, cast_length, other_err, Encode as _, EncodeResult}; use ironrdp_graphics::color_conversion::to_64x64_ycbcr_tile; @@ -186,14 +188,16 @@ impl<'a> UpdateEncoder<'a> { let x = tile_x * 64; let y = tile_y * 64; - let tile_width = core::cmp::min(width - x, 64); - let tile_height = core::cmp::min(height - y, 64); + let tile_width = u32::try_from(core::cmp::min(width - x, 64)).expect("can always fit in u32"); + let tile_height = u32::try_from(core::cmp::min(height - y, 64)).expect("can always fit in u32"); let stride = self.bitmap.stride.get(); let input = &self.bitmap.data[y * stride + x * bpp..]; + let stride = u32::try_from(stride).map_err(io::Error::other)?; let y = &mut [0i16; 4096]; let cb = &mut [0i16; 4096]; let cr = &mut [0i16; 4096]; + to_64x64_ycbcr_tile(input, tile_width, tile_height, stride, self.bitmap.format, y, cb, cr)?; let (y_data, buf) = buf.split_at_mut(4096); diff --git a/crates/ironrdp-web/src/canvas.rs b/crates/ironrdp-web/src/canvas.rs index 1c8b776d6..54cfbb2a3 100644 --- a/crates/ironrdp-web/src/canvas.rs +++ b/crates/ironrdp-web/src/canvas.rs @@ -79,8 +79,8 @@ impl Canvas { let damage_rect = softbuffer::Rect { x: u32::from(region.left), y: u32::from(region.top), - width: NonZeroU32::new(u32::from(region_width)).expect("Per invariants: 0 < region_width"), - height: NonZeroU32::new(u32::from(region_height)).expect("Per invariants: 0 < region_height"), + width: NonZeroU32::new(u32::from(region_width)).expect("per invariants: 0 < region_width"), + height: NonZeroU32::new(u32::from(region_height)).expect("per invariants: 0 < region_height"), }; dst.present_with_damage(&[damage_rect]).expect("buffer present"); diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 2c7f8dd18..b53730eaa 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -478,10 +478,10 @@ impl iron_remote_desktop::Session for Session { debug!("Initialize canvas"); - let desktop_width = NonZeroU32::new(u32::from(connection_result.desktop_size.width)) - .ok_or_else(|| anyhow!("desktop width is zero"))?; - let desktop_height = NonZeroU32::new(u32::from(connection_result.desktop_size.height)) - .ok_or_else(|| anyhow!("desktop height is zero"))?; + let desktop_width = + NonZeroU32::new(u32::from(connection_result.desktop_size.width)).context("desktop width is zero")?; + let desktop_height = + NonZeroU32::new(u32::from(connection_result.desktop_size.height)).context("desktop height is zero")?; let mut gui = Canvas::new(self.render_canvas.clone(), desktop_width, desktop_height).context("canvas initialization")?; diff --git a/crates/ironrdp/examples/server.rs b/crates/ironrdp/examples/server.rs index 7a346f3dd..b23cac098 100644 --- a/crates/ironrdp/examples/server.rs +++ b/crates/ironrdp/examples/server.rs @@ -235,14 +235,7 @@ struct StubSoundServerFactory { impl ServerEventSender for StubSoundServerFactory { fn set_sender(&mut self, sender: UnboundedSender) { - let mut inner = match self.inner.lock() { - Ok(inner) => inner, - Err(e) => { - warn!("Failed to acquire the mutex: {e:?}"); - return; - } - }; - + let mut inner = self.inner.lock().expect("poisoned"); inner.ev_sender = Some(sender); } } From dc47d82f01ea52ab5d86fd17b82976df44b2302a Mon Sep 17 00:00:00 2001 From: Alexandr Yusuk Date: Fri, 5 Sep 2025 12:59:29 +0300 Subject: [PATCH 3/8] fix build --- crates/ironrdp-bench/benches/bench.rs | 13 ++++++++++++- crates/ironrdp-server/src/builder.rs | 2 +- crates/ironrdp-server/src/encoder/rfx.rs | 3 ++- crates/ironrdp-web/src/session.rs | 2 +- ffi/src/dvc/dvc_pipe_proxy_message_queue.rs | 5 +++-- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/ironrdp-bench/benches/bench.rs b/crates/ironrdp-bench/benches/bench.rs index cf0d7c2e4..0069740bb 100644 --- a/crates/ironrdp-bench/benches/bench.rs +++ b/crates/ironrdp-bench/benches/bench.rs @@ -57,7 +57,18 @@ pub fn to_ycbcr_bench(c: &mut Criterion) { let mut cr = [0i16; WIDTH * HEIGHT]; let format = ironrdp_graphics::image_processing::PixelFormat::ARgb32; c.bench_function("to_ycbcr", |b| { - b.iter(|| to_64x64_ycbcr_tile(&input, WIDTH, HEIGHT, stride, format, &mut y, &mut cb, &mut cr)) + b.iter(|| { + to_64x64_ycbcr_tile( + &input, + WIDTH.try_into().expect("can't panic"), + HEIGHT.try_into().expect("can't panic"), + stride.try_into().expect("can't panic"), + format, + &mut y, + &mut cb, + &mut cr, + ) + }) }); } diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index 639981f80..f55bc6b03 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -1,6 +1,6 @@ use core::net::SocketAddr; -use anyhow::{anyhow, Context, Result}; +use anyhow::Result; use ironrdp_pdu::rdp::capability_sets::{server_codecs_capabilities, BitmapCodecs}; use tokio_rustls::TlsAcceptor; diff --git a/crates/ironrdp-server/src/encoder/rfx.rs b/crates/ironrdp-server/src/encoder/rfx.rs index 0ffba651b..956c8e648 100644 --- a/crates/ironrdp-server/src/encoder/rfx.rs +++ b/crates/ironrdp-server/src/encoder/rfx.rs @@ -198,7 +198,8 @@ impl<'a> UpdateEncoder<'a> { let cb = &mut [0i16; 4096]; let cr = &mut [0i16; 4096]; - to_64x64_ycbcr_tile(input, tile_width, tile_height, stride, self.bitmap.format, y, cb, cr)?; + to_64x64_ycbcr_tile(input, tile_width, tile_height, stride, self.bitmap.format, y, cb, cr) + .map_err(RlgrError::YuvError)?; let (y_data, buf) = buf.split_at_mut(4096); let (cb_data, cr_data) = buf.split_at_mut(4096); diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index b53730eaa..771f367a5 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -5,7 +5,7 @@ use core::time::Duration; use std::borrow::Cow; use std::rc::Rc; -use anyhow::{anyhow, Context as _}; +use anyhow::Context as _; use base64::Engine as _; use futures_channel::mpsc; use futures_util::io::{ReadHalf, WriteHalf}; diff --git a/ffi/src/dvc/dvc_pipe_proxy_message_queue.rs b/ffi/src/dvc/dvc_pipe_proxy_message_queue.rs index ce67b0c15..a57935b92 100644 --- a/ffi/src/dvc/dvc_pipe_proxy_message_queue.rs +++ b/ffi/src/dvc/dvc_pipe_proxy_message_queue.rs @@ -1,12 +1,13 @@ -use ironrdp::svc::SvcMessage; use std::sync::mpsc; +use ironrdp::svc::SvcMessage; + #[diplomat::bridge] pub mod ffi { - use crate::error::ffi::IronRdpError; use std::sync::mpsc; use super::{DvcPipeProxyMessageInner, DvcPipeProxyMessageQueueInner}; + use crate::error::ffi::IronRdpError; #[diplomat::opaque] pub struct DvcPipeProxyMessage(pub DvcPipeProxyMessageInner); From 5c38f6b069a165bbd19ea6472ab6c0d7e920b640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 9 Sep 2025 17:59:40 +0900 Subject: [PATCH 4/8] Code review --- crates/ironrdp-ainput/src/lib.rs | 2 + crates/ironrdp-bench/benches/bench.rs | 16 ++++--- crates/ironrdp-client/src/app.rs | 6 +-- .../src/connection_activation.rs | 2 +- crates/ironrdp-server/src/builder.rs | 10 ++--- crates/ironrdp-server/src/server.rs | 45 +++++++++++-------- crates/ironrdp-web/src/session.rs | 19 ++++---- crates/ironrdp/examples/server.rs | 10 +---- 8 files changed, 59 insertions(+), 51 deletions(-) diff --git a/crates/ironrdp-ainput/src/lib.rs b/crates/ironrdp-ainput/src/lib.rs index 8e040ec77..eba9c4698 100644 --- a/crates/ironrdp-ainput/src/lib.rs +++ b/crates/ironrdp-ainput/src/lib.rs @@ -94,6 +94,7 @@ impl<'de> Decode<'de> for VersionPdu { } #[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)] +#[repr(u16)] pub enum ServerPduType { Version = 0x01, } @@ -265,6 +266,7 @@ impl<'de> Decode<'de> for ClientPdu { } #[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)] +#[repr(u16)] pub enum ClientPduType { Mouse = 0x02, } diff --git a/crates/ironrdp-bench/benches/bench.rs b/crates/ironrdp-bench/benches/bench.rs index 0069740bb..fed6123a0 100644 --- a/crates/ironrdp-bench/benches/bench.rs +++ b/crates/ironrdp-bench/benches/bench.rs @@ -7,13 +7,13 @@ use ironrdp_server::bench::encoder::rfx::{rfx_enc, rfx_enc_tile}; use ironrdp_server::BitmapUpdate; pub fn rfx_enc_tile_bench(c: &mut Criterion) { - let quant = rfx::Quant::default(); - let algo = rfx::EntropyAlgorithm::Rlgr3; - const WIDTH: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero"); const HEIGHT: NonZeroU16 = NonZeroU16::new(64).expect("value is guaranteed to be non-zero"); const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero"); + let quant = rfx::Quant::default(); + let algo = rfx::EntropyAlgorithm::Rlgr3; + let bitmap = BitmapUpdate { x: 0, y: 0, @@ -27,14 +27,14 @@ pub fn rfx_enc_tile_bench(c: &mut Criterion) { } pub fn rfx_enc_bench(c: &mut Criterion) { - let quant = rfx::Quant::default(); - let algo = rfx::EntropyAlgorithm::Rlgr3; - const WIDTH: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero"); const HEIGHT: NonZeroU16 = NonZeroU16::new(2048).expect("value is guaranteed to be non-zero"); - // QUESTION: It looks like we have a bug here, don't we? The stride value should be 2048 * 4. + // FIXME/QUESTION: It looks like we have a bug here, don't we? The stride value should be 2048 * 4. const STRIDE: NonZeroUsize = NonZeroUsize::new(64 * 4).expect("value is guaranteed to be non-zero"); + let quant = rfx::Quant::default(); + let algo = rfx::EntropyAlgorithm::Rlgr3; + let bitmap = BitmapUpdate { x: 0, y: 0, @@ -50,12 +50,14 @@ pub fn rfx_enc_bench(c: &mut Criterion) { pub fn to_ycbcr_bench(c: &mut Criterion) { const WIDTH: usize = 64; const HEIGHT: usize = 64; + let input = vec![0; WIDTH * HEIGHT * 4]; let stride = WIDTH * 4; let mut y = [0i16; WIDTH * HEIGHT]; let mut cb = [0i16; WIDTH * HEIGHT]; let mut cr = [0i16; WIDTH * HEIGHT]; let format = ironrdp_graphics::image_processing::PixelFormat::ARgb32; + c.bench_function("to_ycbcr", |b| { b.iter(|| { to_64x64_ycbcr_tile( diff --git a/crates/ironrdp-client/src/app.rs b/crates/ironrdp-client/src/app.rs index 48cde764b..9f579c178 100644 --- a/crates/ironrdp-client/src/app.rs +++ b/crates/ironrdp-client/src/app.rs @@ -68,8 +68,8 @@ impl App { }; let scale_factor = (window.scale_factor() * 100.0) as u32; - let width = u16::try_from(size.width).expect("width is too big(more than u16::MAX)"); - let height = u16::try_from(size.height).expect("height is too big(more than u16::MAX)"); + let width = u16::try_from(size.width).expect("reasonable width"); + let height = u16::try_from(size.height).expect("reasonable height"); let _ = self.input_event_sender.send(RdpInputEvent::Resize { width, @@ -169,7 +169,7 @@ impl ApplicationHandler for App { let scancode = match u16::try_from(scancode) { Ok(scancode) => scancode, Err(_) => { - warn!("Unsupported scancode: `{scancode:#X}`. Keyboard event will be ignored."); + warn!("Unsupported scancode: `{scancode:#X}`; ignored"); return; } }; diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index fb7895b67..86104bc0b 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -368,7 +368,7 @@ fn create_client_confirm_active( }), CapabilitySet::BitmapCodecs(match config.bitmap.as_ref().map(|b| b.codecs.clone()) { Some(codecs) => codecs, - None => client_codecs_capabilities(&[]).map_err(|e| reason_err!("client codecs capabilities", "{e}"))?, + None => client_codecs_capabilities(&[]).expect("can't panic for &[]"), }), CapabilitySet::FrameAcknowledge(FrameAcknowledge { // FIXME(#447): Revert this to 2 per FreeRDP. diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index f55bc6b03..40bdca80b 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -113,11 +113,11 @@ impl RdpServerBuilder { } impl RdpServerBuilder { - pub fn with_display_handler(self, display: D) -> Result> + pub fn with_display_handler(self, display: D) -> RdpServerBuilder where D: RdpServerDisplay + 'static, { - Ok(RdpServerBuilder { + RdpServerBuilder { state: BuilderDone { addr: self.state.addr, security: self.state.security, @@ -125,9 +125,9 @@ impl RdpServerBuilder { display: Box::new(display), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]).expect("can't panic"), + codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), }, - }) + } } pub fn with_no_display(self) -> Result> { @@ -139,7 +139,7 @@ impl RdpServerBuilder { display: Box::new(NoopDisplay), sound_factory: None, cliprdr_factory: None, - codecs: server_codecs_capabilities(&[]).expect("can't panic"), + codecs: server_codecs_capabilities(&[]).expect("can't panic for &[]"), }, }) } diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 26d174bc5..a0da55f92 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -602,28 +602,35 @@ impl RdpServer { let dispatch_display = async move { let mut buffer = vec![0u8; 4096]; + loop { - if let Some(update) = display_updates.next_update().await? { - match Self::dispatch_display_update( - update, - &mut display_writer, - user_channel_id, - io_channel_id, - &mut buffer, - encoder, - ) - .await? - { - (RunState::Continue, enc) => { - encoder = enc; - continue; - } - (state, _) => { - break Ok(state); + match display_updates.next_update().await { + Ok(Some(update)) => { + match Self::dispatch_display_update( + update, + &mut display_writer, + user_channel_id, + io_channel_id, + &mut buffer, + encoder, + ) + .await? + { + (RunState::Continue, enc) => { + encoder = enc; + continue; + } + (state, _) => { + break Ok(state); + } } } - } else { - break Ok(RunState::Disconnect); + Ok(None) => { + break Ok(RunState::Disconnect); + } + Err(error) => { + warn!(error = format!("{error:#}"), "next_updated failed"); + } } } }; diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 771f367a5..e5933bad3 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -280,8 +280,7 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { info!("Connect to RDP host"); - let mut config = - build_config(username, password, server_domain, client_name, desktop_size).context("build config")?; + let mut config = build_config(username, password, server_domain, client_name, desktop_size); let enable_credssp = self.0.borrow().enable_credssp; config.enable_credssp = enable_credssp; @@ -849,8 +848,8 @@ fn build_config( domain: Option, client_name: String, desktop_size: DesktopSize, -) -> anyhow::Result { - Ok(connector::Config { +) -> connector::Config { + connector::Config { credentials: Credentials::UsernamePassword { username, password }, domain, // TODO(#327): expose these options from the WASM module. @@ -869,12 +868,16 @@ fn build_config( bitmap: Some(connector::BitmapConfig { color_depth: 16, lossy_compression: true, - codecs: client_codecs_capabilities(&[]).map_err(|err| anyhow::anyhow!("client codecs capabilities error: {err}"))?, + codecs: client_codecs_capabilities(&[]).expect("can't panic for &[]"), }), - #[expect(clippy::arithmetic_side_effects)] // fine unless we end up with an insanely big version + #[expect( + clippy::arithmetic_side_effects, + reason = "fine unless we end up with an insanely big version" + )] client_build: semver::Version::parse(env!("CARGO_PKG_VERSION")) .map_or(0, |version| version.major * 100 + version.minor * 10 + version.patch) - .pipe(u32::try_from).context("cargo package version")?, + .pipe(u32::try_from) + .expect("fine until major ~42949672"), client_name, // NOTE: hardcode this value like in freerdp // https://github.com/FreeRDP/FreeRDP/blob/4e24b966c86fdf494a782f0dfcfc43a057a2ea60/libfreerdp/core/settings.c#LL49C34-L49C70 @@ -890,7 +893,7 @@ fn build_config( hardware_id: None, license_cache: None, timezone_info: TimezoneInfo::default(), - }) + } } async fn writer_task( diff --git a/crates/ironrdp/examples/server.rs b/crates/ironrdp/examples/server.rs index b23cac098..dde826ba1 100644 --- a/crates/ironrdp/examples/server.rs +++ b/crates/ironrdp/examples/server.rs @@ -341,13 +341,7 @@ impl RdpsndServerHandler for SndHandler { wave.into_iter().flat_map(|value| value.to_le_bytes()).collect() }; - let inner = match inner.lock() { - Ok(inner) => inner, - Err(e) => { - warn!("Failed to acquire the mutex: {e:?}"); - return; - } - }; + let inner = inner.lock().expect("poisoned"); if let Some(sender) = inner.ev_sender.as_ref() { let _ = sender.send(ServerEvent::Rdpsnd(RdpsndServerMessage::Wave(data, ts))); } @@ -428,7 +422,7 @@ async fn run( let mut server = server_builder .with_input_handler(handler.clone()) - .with_display_handler(handler.clone())? + .with_display_handler(handler.clone()) .with_cliprdr_factory(Some(cliprdr)) .with_sound_factory(Some(sound)) .build(); From 2b2ccec0daee626ba5bad3daa3fb685596cd5f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Cortier?= <3809077+CBenoit@users.noreply.github.com> Date: Tue, 9 Sep 2025 05:54:38 -0400 Subject: [PATCH 5/8] Apply suggestion from @CBenoit --- crates/ironrdp-testsuite-extra/tests/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ironrdp-testsuite-extra/tests/tests.rs b/crates/ironrdp-testsuite-extra/tests/tests.rs index c8da8e36f..7127fa5f1 100644 --- a/crates/ironrdp-testsuite-extra/tests/tests.rs +++ b/crates/ironrdp-testsuite-extra/tests/tests.rs @@ -178,7 +178,6 @@ where .with_display_handler(TestDisplay { rx: Arc::new(Mutex::new(display_rx)), }) - .unwrap() .build(); server.set_credentials(Some(server::Credentials { username: USERNAME.into(), From 1aef9144344e041478a363c696238ca0c3fc023a Mon Sep 17 00:00:00 2001 From: Alex Yusiuk <55661041+RRRadicalEdward@users.noreply.github.com> Date: Thu, 11 Sep 2025 14:15:42 +0300 Subject: [PATCH 6/8] Update crates/ironrdp-connector/src/connection_activation.rs --- crates/ironrdp-connector/src/connection_activation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index 86104bc0b..a403bd28a 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -5,7 +5,7 @@ use ironrdp_pdu::rdp::{self}; use tracing::{debug, warn}; use crate::{ - general_err, legacy, reason_err, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, + general_err, legacy, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, State, Written, }; From e91e1e2d46196a02f53975e571033c1a56710442 Mon Sep 17 00:00:00 2001 From: Alexandr Yusuk Date: Thu, 11 Sep 2025 14:17:14 +0300 Subject: [PATCH 7/8] run cargo fmt --- crates/ironrdp-connector/src/connection_activation.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index a403bd28a..d268f9672 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -5,8 +5,7 @@ use ironrdp_pdu::rdp::{self}; use tracing::{debug, warn}; use crate::{ - general_err, legacy, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, - State, Written, + general_err, legacy, Config, ConnectionFinalizationSequence, ConnectorResult, DesktopSize, Sequence, State, Written, }; /// Represents the Capability Exchange and Connection Finalization phases From 66f36ec37c323ed6372792b171514edf862d085e Mon Sep 17 00:00:00 2001 From: Alexandr Yusuk Date: Thu, 11 Sep 2025 14:24:26 +0300 Subject: [PATCH 8/8] fix doc test --- crates/ironrdp-server/src/server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index a0da55f92..101ea4066 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -202,7 +202,7 @@ impl DisplayControlHandler for DisplayControlBackend { /// .with_addr(([127, 0, 0, 1], 3389)) /// .with_tls(tls_acceptor) /// .with_input_handler(input_handler) -/// .with_display_handler(display_handler)? +/// .with_display_handler(display_handler) /// .build(); /// /// server.run().await;