refactor: add panic clippy correctness lint (#934)

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

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

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<Self> {
         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<T>(&mut self, channel: T)

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<Item = &'a StaticVirtualChannel>,
-) -> gcc::ClientGccBlocks {
+) -> ConnectorResult<gcc::ClientGccBlocks> {
     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::<Vec<_>>();
 
-    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 {

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<Self, PointerError> {
-        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<Self, PointerError> {
+        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) => {

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;
                 }

crates/ironrdp-session/src/legacy.rs

@@ -5,10 +5,9 @@ use crate::SessionError;
 impl From<ironrdp_connector::ConnectorErrorKind> 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,
         }
     }

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.