From f34a9f25008bd4f48d1522ef8ba8d5f190ee92f9 Mon Sep 17 00:00:00 2001 From: Alex Yusiuk <55661041+RRRadicalEdward@users.noreply.github.com> Date: Tue, 26 Aug 2025 17:52:01 +0300 Subject: [PATCH] refactor: add `panic` clippy correctness lint (#934) --- Cargo.toml | 1 + clippy.toml | 1 + crates/ironrdp-acceptor/src/connection.rs | 12 +++++------ crates/ironrdp-connector/src/connection.rs | 15 ++++++++----- crates/ironrdp-graphics/src/pointer.rs | 24 ++++++++++++++------- crates/ironrdp-server/src/server.rs | 2 +- crates/ironrdp-session/src/legacy.rs | 7 +++--- crates/ironrdp-testsuite-core/tests/main.rs | 2 +- 8 files changed, 39 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bfc1decb..05352f5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,6 +107,7 @@ mem_forget = "warn" mixed_read_write_in_expression = "warn" needless_raw_strings = "warn" non_ascii_literal = "warn" +panic = "warn" # == Style, readability == # semicolon_outside_block = "warn" # With semicolon-outside-block-ignore-multiline = true diff --git a/clippy.toml b/clippy.toml index d468261c..7fc7b2d0 100644 --- a/clippy.toml +++ b/clippy.toml @@ -2,3 +2,4 @@ msrv = "1.84" semicolon-outside-block-ignore-multiline = true accept-comment-above-statement = true accept-comment-above-attributes = true +allow-panic-in-tests = true diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index 246e65af..68e214b7 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -1,8 +1,8 @@ use core::mem; use ironrdp_connector::{ - encode_x224_packet, reason_err, ConnectorError, ConnectorErrorExt as _, ConnectorResult, DesktopSize, Sequence, - State, Written, + encode_x224_packet, general_err, reason_err, ConnectorError, ConnectorErrorExt as _, ConnectorResult, DesktopSize, + Sequence, State, Written, }; use ironrdp_core::{decode, WriteBuf}; use ironrdp_pdu as pdu; @@ -72,13 +72,13 @@ impl Acceptor { mut consumed: Acceptor, static_channels: StaticChannelSet, desktop_size: DesktopSize, - ) -> Self { + ) -> ConnectorResult { let AcceptorState::CapabilitiesSendServer { early_capability, channels, } = consumed.saved_for_reactivation else { - panic!("invalid acceptor state"); + return Err(general_err!("invalid acceptor state")); }; for cap in consumed.server_capabilities.iter_mut() { @@ -95,7 +95,7 @@ impl Acceptor { early_capability, channels, }; - Self { + Ok(Self { security: consumed.security, state, user_channel_id: consumed.user_channel_id, @@ -106,7 +106,7 @@ impl Acceptor { saved_for_reactivation, creds: consumed.creds, reactivation: true, - } + }) } pub fn attach_static_channel(&mut self, channel: T) diff --git a/crates/ironrdp-connector/src/connection.rs b/crates/ironrdp-connector/src/connection.rs index baaa621e..6852ce79 100644 --- a/crates/ironrdp-connector/src/connection.rs +++ b/crates/ironrdp-connector/src/connection.rs @@ -325,7 +325,7 @@ impl Sequence for ClientConnector { debug!("Basic Settings Exchange"); let client_gcc_blocks = - create_gcc_blocks(&self.config, selected_protocol, self.static_channels.values()); + create_gcc_blocks(&self.config, selected_protocol, self.static_channels.values())?; let connect_initial = mcs::ConnectInitial::with_gcc_blocks(client_gcc_blocks); @@ -608,7 +608,7 @@ fn create_gcc_blocks<'a>( config: &Config, selected_protocol: nego::SecurityProtocol, static_channels: impl Iterator, -) -> gcc::ClientGccBlocks { +) -> ConnectorResult { use ironrdp_pdu::gcc::{ ClientCoreData, ClientCoreOptionalData, ClientEarlyCapabilityFlags, ClientGccBlocks, ClientNetworkData, ClientSecurityData, ColorDepth, ConnectionType, EncryptionMethod, HighColorDepth, MonitorOrientation, @@ -622,14 +622,19 @@ fn create_gcc_blocks<'a>( 16 => SupportedColorDepths::BPP16, 24 => SupportedColorDepths::BPP24, 32 => SupportedColorDepths::BPP32 | SupportedColorDepths::BPP16, - _ => panic!("Unsupported color depth: {max_color_depth}"), + _ => { + return Err(reason_err!( + "create gcc blocks", + "unsupported color depth: {max_color_depth}" + )) + } }; let channels = static_channels .map(ironrdp_svc::make_channel_definition) .collect::>(); - ClientGccBlocks { + Ok(ClientGccBlocks { core: ClientCoreData { version: RdpVersion::V5_PLUS, desktop_width: config.desktop_size.width, @@ -698,7 +703,7 @@ fn create_gcc_blocks<'a>( // TODO(#140): support for Some(MultiTransportChannelData { flags: MultiTransportFlags::empty(), }) multi_transport_channel: None, monitor_extended: None, - } + }) } fn create_client_info_pdu(config: &Config, client_addr: &SocketAddr) -> rdp::ClientInfoPdu { diff --git a/crates/ironrdp-graphics/src/pointer.rs b/crates/ironrdp-graphics/src/pointer.rs index 779024e5..48b17927 100644 --- a/crates/ironrdp-graphics/src/pointer.rs +++ b/crates/ironrdp-graphics/src/pointer.rs @@ -24,6 +24,8 @@ use ironrdp_pdu::pointer::{ColorPointerAttribute, LargePointerAttribute, Pointer use crate::color_conversion::rdp_16bit_to_rgb; +const SUPPORTED_COLOR_BPP: [u16; 4] = [1, 16, 24, 32]; + #[derive(Debug)] pub enum PointerError { InvalidXorMaskSize { expected: usize, actual: usize }, @@ -179,8 +181,6 @@ impl DecodedPointer { } fn decode_pointer(data: PointerData<'_>, target: PointerBitmapTarget) -> Result { - const SUPPORTED_COLOR_BPP: [u16; 4] = [1, 16, 24, 32]; - if data.width == 0 || data.height == 0 { return Ok(Self::new_invisible()); } @@ -230,7 +230,7 @@ impl DecodedPointer { (xor_stride_cursor, and_stride_cursor) }; - let mut color_reader = ColorStrideReader::new(data.xor_bpp, xor_stride); + let mut color_reader = ColorStrideReader::new(data.xor_bpp, xor_stride)?; let mut bitmask_reader = BitmaskStrideReader::new(and_stride); let compute_inverted_pixel = if target.should_invert_pixels_using_check_pattern() { @@ -340,6 +340,7 @@ impl BitmaskStrideReader { enum ColorStrideReader { Color { + /// INVARIANT: `bpp == 16 || bpp == 24 || bpp == 32` bpp: u16, read_stide_bytes: usize, stride_data_bytes: usize, @@ -349,16 +350,23 @@ enum ColorStrideReader { } impl ColorStrideReader { - fn new(bpp: u16, stride: Stride) -> Self { - match bpp { + fn new(bpp: u16, stride: Stride) -> Result { + Ok(match bpp { 1 => Self::Bitmask(BitmaskStrideReader::new(stride)), bpp => Self::Color { - bpp, + bpp: { + // Enforce the bpp == 16 || bpp == 24 || bpp == 32 invariant. + if !SUPPORTED_COLOR_BPP[1..].contains(&bpp) { + return Err(PointerError::NotSupportedBpp { bpp }); + } + + bpp + }, read_stide_bytes: 0, stride_data_bytes: stride.data_bytes, stride_padding: stride.padding, }, - } + }) } fn next_pixel(&mut self, cursor: &mut ReadCursor<'_>) -> [u8; 4] { @@ -392,7 +400,7 @@ impl ColorStrideReader { let color_32bit = cursor.read_array::<4>(); [color_32bit[2], color_32bit[1], color_32bit[0], color_32bit[3]] } - _ => panic!("BUG: should be validated in the calling code"), + _ => unreachable!("per the invariant on self.bpp, this path is unreachable"), } } ColorStrideReader::Bitmask(bitask) => { diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 84edff84..190d6a81 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -969,7 +969,7 @@ impl RdpServer { acceptor, core::mem::take(&mut self.static_channels), desktop_size, - ); + )?; framed = unsplit_tokio_framed(reader, writer); continue; } diff --git a/crates/ironrdp-session/src/legacy.rs b/crates/ironrdp-session/src/legacy.rs index 3df112cc..44e1399f 100644 --- a/crates/ironrdp-session/src/legacy.rs +++ b/crates/ironrdp-session/src/legacy.rs @@ -5,10 +5,9 @@ use crate::SessionError; impl From for crate::SessionErrorKind { fn from(value: ironrdp_connector::ConnectorErrorKind) -> Self { match value { - ironrdp_connector::ConnectorErrorKind::Credssp(_) => panic!("unexpected"), - ironrdp_connector::ConnectorErrorKind::AccessDenied => panic!("unexpected"), - ironrdp_connector::ConnectorErrorKind::General => crate::SessionErrorKind::General, - ironrdp_connector::ConnectorErrorKind::Custom => crate::SessionErrorKind::Custom, + ironrdp_connector::ConnectorErrorKind::Custom | ironrdp_connector::ConnectorErrorKind::Credssp(_) => { + crate::SessionErrorKind::Custom + } _ => crate::SessionErrorKind::General, } } diff --git a/crates/ironrdp-testsuite-core/tests/main.rs b/crates/ironrdp-testsuite-core/tests/main.rs index ad9456bc..246feea8 100644 --- a/crates/ironrdp-testsuite-core/tests/main.rs +++ b/crates/ironrdp-testsuite-core/tests/main.rs @@ -1,5 +1,5 @@ #![allow(unused_crate_dependencies)] // false positives because there is both a library and a binary - +#![allow(clippy::panic, reason = "panic is acceptable in tests")] //! Integration Tests (IT) //! //! Integration tests are all contained in this single crate, and organized in modules.