refactor(ironrdp-pdu)!: fix as_conversions clippy lint warnings (#967)

Co-authored-by: Benoît CORTIER <git.divisible626@passmail.com>

Author: RRRadicalEdward

Cargo.toml

@@ -93,6 +93,7 @@ fn_to_numeric_cast_any = "warn"
 ptr_cast_constness = "warn"
 
 # == Correctness == #
+#as_conversions = "warn"
 cast_lossless = "warn"
 cast_possible_truncation = "warn"
 cast_possible_wrap = "warn"

crates/ironrdp-pdu/src/basic_output/bitmap/mod.rs

@@ -7,8 +7,8 @@ use core::fmt::{self, Debug};
 
 use bitflags::bitflags;
 use ironrdp_core::{
- ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor,
- WriteCursor,
+ cast_length, ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult,
+ ReadCursor, WriteCursor,
 };
 
 use crate::geometry::InclusiveRectangle;
@@ -41,11 +41,9 @@ impl Encode for BitmapUpdateData<'_> {
  fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
  ensure_size!(in: dst, size: self.size());
 
- if self.rectangles.len() > u16::MAX as usize {
- return Err(invalid_field_err!("numberRectangles", "rectangle count is too big"));
- }
+ let rectangle_count = cast_length!("number of rectangles", self.rectangles.len())?;
 
- Self::encode_header(self.rectangles.len() as u16, dst)?;
+ Self::encode_header(rectangle_count, dst)?;
 
  for bitmap_data in self.rectangles.iter() {
  bitmap_data.encode(dst)?;
@@ -74,10 +72,10 @@ impl<'de> Decode<'de> for BitmapUpdateData<'de> {
  return Err(invalid_field_err!("updateType", "invalid update type"));
  }
 
- let rectangles_number = src.read_u16() as usize;
- let mut rectangles = Vec::with_capacity(rectangles_number);
+ let rectangle_count = usize::from(src.read_u16());
+ let mut rectangles = Vec::with_capacity(rectangle_count);
 
- for _ in 0..rectangles_number {
+ for _ in 0..rectangle_count {
  rectangles.push(BitmapData::decode(src)?);
  }
 
@@ -111,16 +109,14 @@ impl Encode for BitmapData<'_> {
  ensure_size!(in: dst, size: self.size());
 
  let encoded_bitmap_data_length = self.encoded_bitmap_data_length();
- if encoded_bitmap_data_length > u16::MAX as usize {
- return Err(invalid_field_err!("bitmapLength", "bitmap data length is too big"));
- }
+ let encoded_bitmap_data_length = cast_length!("bitmap data length", encoded_bitmap_data_length)?;
 
  self.rectangle.encode(dst)?;
  dst.write_u16(self.width);
  dst.write_u16(self.height);
  dst.write_u16(self.bits_per_pixel);
  dst.write_u16(self.compression_flags.bits());
- dst.write_u16(encoded_bitmap_data_length as u16);
+ dst.write_u16(encoded_bitmap_data_length);
  if let Some(compressed_data_header) = &self.compressed_data_header {
  compressed_data_header.encode(dst)?;
  };
@@ -150,25 +146,25 @@ impl<'de> Decode<'de> for BitmapData<'de> {
 
  // A 16-bit, unsigned integer. The size in bytes of the data in the bitmapComprHdr
  // and bitmapDataStream fields.
- let encoded_bitmap_data_length = src.read_u16();
+ let encoded_bitmap_data_length = usize::from(src.read_u16());
 
- ensure_size!(in: src, size: encoded_bitmap_data_length as usize);
+ ensure_size!(in: src, size: encoded_bitmap_data_length);
 
  let (compressed_data_header, buffer_length) = if compression_flags.contains(Compression::BITMAP_COMPRESSION)
  && !compression_flags.contains(Compression::NO_BITMAP_COMPRESSION_HDR)
  {
  // Check if encoded_bitmap_data_length is at least CompressedDataHeader::ENCODED_SIZE
- if encoded_bitmap_data_length < CompressedDataHeader::ENCODED_SIZE as u16 {
+ if encoded_bitmap_data_length < CompressedDataHeader::ENCODED_SIZE {
  return Err(invalid_field_err!(
  "cbCompEncodedBitmapDataLength",
  "length is less than CompressedDataHeader::ENCODED_SIZE"
  ));
  }
 
- let buffer_length = encoded_bitmap_data_length as usize - CompressedDataHeader::ENCODED_SIZE;
+ let buffer_length = encoded_bitmap_data_length - CompressedDataHeader::ENCODED_SIZE;
  (Some(CompressedDataHeader::decode(src)?), buffer_length)
  } else {
- (None, encoded_bitmap_data_length as usize)
+ (None, encoded_bitmap_data_length)
  };
 
  let bitmap_data = src.read_slice(buffer_length);

crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs

@@ -56,7 +56,7 @@ impl Encode for BitmapStreamHeader {
  fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
  ensure_size!(in: dst, size: self.size());
 
- let mut header = ((self.enable_rle_compression as u8) << 4) | ((!self.use_alpha as u8) << 5);
+ let mut header = (u8::from(self.enable_rle_compression) << 4) | (u8::from(!self.use_alpha) << 5);
 
  match self.color_plane_definition {
  ColorPlaneDefinition::Argb => {
@@ -68,7 +68,7 @@ impl Encode for BitmapStreamHeader {
  ..
  } => {
  // Add cll and cs flags to header
- header |= (color_loss_level & 0x07) | ((use_chroma_subsampling as u8) << 3);
+ header |= (color_loss_level & 0x07) | (u8::from(use_chroma_subsampling) << 3);
  }
  }
 

crates/ironrdp-pdu/src/basic_output/fast_path/mod.rs

@@ -4,8 +4,8 @@ mod tests;
 use bit_field::BitField as _;
 use bitflags::bitflags;
 use ironrdp_core::{
- decode_cursor, ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeError, DecodeResult, Encode,
- EncodeResult, InvalidFieldErr as _, ReadCursor, WriteCursor,
+ cast_length, decode_cursor, ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeError,
+ DecodeResult, Encode, EncodeResult, InvalidFieldErr as _, ReadCursor, WriteCursor,
 };
 use num_derive::FromPrimitive;
 use num_traits::FromPrimitive as _;
@@ -42,7 +42,7 @@ impl FastPathHeader {
  // it may then be +2 if > 0x7f
  let len = self.data_length + Self::FIXED_PART_SIZE + 1;
 
- Self::FIXED_PART_SIZE + per::sizeof_length(len as u16)
+ Self::FIXED_PART_SIZE + per::sizeof_length(len)
  }
 }
 
@@ -56,15 +56,13 @@ impl Encode for FastPathHeader {
  dst.write_u8(header);
 
  let length = self.data_length + self.size();
- if length > u16::MAX as usize {
- return Err(invalid_field_err!("length", "fastpath PDU length is too big"));
- }
+ let length = cast_length!("length", length)?;
 
  if self.forced_long_length {
  // Preserve same layout for header as received
- per::write_long_length(dst, length as u16);
+ per::write_long_length(dst, length);
  } else {
- per::write_length(dst, length as u16);
+ per::write_length(dst, length);
  }
 
  Ok(())
@@ -93,14 +91,15 @@ impl<'de> Decode<'de> for FastPathHeader {
  let (length, sizeof_length) = per::read_length(src).map_err(|e| {
  DecodeError::invalid_field("", "length", "Invalid encoded fast path PDU length").with_source(e)
  })?;
- if (length as usize) < sizeof_length + Self::FIXED_PART_SIZE {
+ let length = usize::from(length);
+ if length < sizeof_length + Self::FIXED_PART_SIZE {
  return Err(invalid_field_err!(
  "length",
  "received fastpath PDU length is smaller than header size"
  ));
  }
- let data_length = length as usize - sizeof_length - Self::FIXED_PART_SIZE;
- // Detect case, when received packet has non-optimal packet length packing
+ let data_length = length - sizeof_length - Self::FIXED_PART_SIZE;
+ // Detect case, when received packet has non-optimal packet length packing.
  let forced_long_length = per::sizeof_length(length) != sizeof_length;
 
  Ok(FastPathHeader {
@@ -131,9 +130,7 @@ impl Encode for FastPathUpdatePdu<'_> {
  fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
  ensure_size!(in: dst, size: self.size());
 
- if self.data.len() > u16::MAX as usize {
- return Err(invalid_field_err!("data", "fastpath PDU data is too big"));
- }
+ let data_len = cast_length!("data length", self.data.len())?;
 
  let mut header = 0u8;
  header.set_bits(0..4, self.update_code.as_u8());
@@ -148,7 +145,7 @@ impl Encode for FastPathUpdatePdu<'_> {
  dst.write_u8(compression_flags_with_type);
  }
 
- dst.write_u16(self.data.len() as u16);
+ dst.write_u16(data_len);
  dst.write_slice(self.data);
 
  Ok(())
@@ -200,7 +197,7 @@ impl<'de> Decode<'de> for FastPathUpdatePdu<'de> {
  (None, None)
  };
 
- let data_length = src.read_u16() as usize;
+ let data_length = usize::from(src.read_u16());
  ensure_size!(in: src, size: data_length);
  let data = src.read_slice(data_length);
 

crates/ironrdp-pdu/src/basic_output/pointer/mod.rs

@@ -1,6 +1,6 @@
 use ironrdp_core::{
- ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor,
- WriteCursor,
+ cast_int, cast_length, ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode,
+ EncodeResult, ReadCursor, WriteCursor,
 };
 
 // Represents `TS_POINT16` described in [MS-RDPBCGR] 2.2.9.1.1.4.1
@@ -69,21 +69,24 @@ macro_rules! check_masks_alignment {
  ($and_mask:expr, $xor_mask:expr, $pointer_height:expr, $large_ptr:expr) => {{
  const AND_MASK_SIZE_FIELD: &str = "lengthAndMask";
  const XOR_MASK_SIZE_FIELD: &str = "lengthXorMask";
+ const U32_MAX: usize = 0xFFFFFFFF;
+
+ let pointer_height: usize = cast_int!("pointer height", $pointer_height)?;
 
  let check_mask = |mask: &[u8], field: &'static str| {
  if $pointer_height == 0 {
  return Err(invalid_field_err!(field, "pointer height cannot be zero"));
  }
- if $large_ptr && (mask.len() > u32::MAX as usize) {
+ if $large_ptr && (mask.len() > U32_MAX) {
  return Err(invalid_field_err!(field, "pointer mask is too big for u32 size"));
  }
- if !$large_ptr && (mask.len() > u16::MAX as usize) {
+ if !$large_ptr && (mask.len() > usize::from(u16::MAX)) {
  return Err(invalid_field_err!(field, "pointer mask is too big for u16 size"));
  }
- if (mask.len() % $pointer_height as usize) != 0 {
+ if (mask.len() % pointer_height) != 0 {
  return Err(invalid_field_err!(field, "pointer mask have incomplete scanlines"));
  }
- if (mask.len() / $pointer_height as usize) % 2 != 0 {
+ if (mask.len() / pointer_height) % 2 != 0 {
  return Err(invalid_field_err!(
  field,
  "pointer mask scanlines should be aligned to 16 bits"
@@ -108,8 +111,8 @@ impl Encode for ColorPointerAttribute<'_> {
  dst.write_u16(self.width);
  dst.write_u16(self.height);
 
- dst.write_u16(self.and_mask.len() as u16);
- dst.write_u16(self.xor_mask.len() as u16);
+ dst.write_u16(cast_length!("and mask length", self.and_mask.len())?);
+ dst.write_u16(cast_length!("xor mask length", self.xor_mask.len())?);
  // Note that masks are written in reverse order. It is not a mistake, that is how the
  // message is defined in [MS-RDPBCGR]
  dst.write_slice(self.xor_mask);
@@ -135,15 +138,15 @@ impl<'a> Decode<'a> for ColorPointerAttribute<'a> {
  let hot_spot = Point16::decode(src)?;
  let width = src.read_u16();
  let height = src.read_u16();
- let length_and_mask = src.read_u16();
- let length_xor_mask = src.read_u16();
-
  // Convert to usize during the addition to prevent overflow and match expected type
- let expected_masks_size = (length_and_mask as usize) + (length_xor_mask as usize);
+ let length_and_mask = usize::from(src.read_u16());
+ let length_xor_mask = usize::from(src.read_u16());
+
+ let expected_masks_size = length_and_mask + length_xor_mask;
  ensure_size!(in: src, size: expected_masks_size);
 
- let xor_mask = src.read_slice(length_xor_mask as usize);
- let and_mask = src.read_slice(length_and_mask as usize);
+ let xor_mask = src.read_slice(length_xor_mask);
+ let and_mask = src.read_slice(length_and_mask);
 
  check_masks_alignment!(and_mask, xor_mask, height, false)?;
 
@@ -270,8 +273,8 @@ impl Encode for LargePointerAttribute<'_> {
  dst.write_u16(self.width);
  dst.write_u16(self.height);
 
- dst.write_u32(self.and_mask.len() as u32);
- dst.write_u32(self.xor_mask.len() as u32);
+ dst.write_u32(cast_length!("and mask length", self.and_mask.len())?);
+ dst.write_u32(cast_length!("xor mask length", self.xor_mask.len())?);
  // See comment in `ColorPointerAttribute::encode` about encoding order
  dst.write_slice(self.xor_mask);
  dst.write_slice(self.and_mask);
@@ -298,8 +301,8 @@ impl<'a> Decode<'a> for LargePointerAttribute<'a> {
  let width = src.read_u16();
  let height = src.read_u16();
  // Convert to usize to prevent overflow during addition
- let length_and_mask = src.read_u32() as usize;
- let length_xor_mask = src.read_u32() as usize;
+ let length_and_mask = cast_length!("and mask length", src.read_u32())?;
+ let length_xor_mask = cast_length!("xor mask length", src.read_u32())?;
 
  let expected_masks_size = length_and_mask + length_xor_mask;
  ensure_size!(in: src, size: expected_masks_size);

crates/ironrdp-pdu/src/basic_output/surface_commands/mod.rs

@@ -3,10 +3,10 @@ mod tests;
 
 use bitflags::bitflags;
 use ironrdp_core::{
- ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult, ReadCursor,
- WriteCursor,
+ cast_length, ensure_fixed_part_size, ensure_size, invalid_field_err, Decode, DecodeResult, Encode, EncodeResult,
+ ReadCursor, WriteCursor,
 };
-use num_derive::{FromPrimitive, ToPrimitive};
+use num_derive::FromPrimitive;
 use num_traits::FromPrimitive as _;
 
 use crate::geometry::ExclusiveRectangle;
@@ -126,7 +126,7 @@ impl Encode for FrameMarkerPdu {
  fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
  ensure_fixed_part_size!(in: dst);
 
- dst.write_u16(self.frame_action as u16);
+ dst.write_u16(self.frame_action.as_u16());
  dst.write_u32(self.frame_id.unwrap_or(0));
 
  Ok(())
@@ -197,9 +197,7 @@ impl Encode for ExtendedBitmapDataPdu<'_> {
  fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
  ensure_size!(in: dst, size: self.size());
 
- if self.data.len() > u32::MAX as usize {
- return Err(invalid_field_err!("bitmapDataLength", "bitmap data is too big"));
- }
+ let data_len = cast_length!("bitmap data length", self.data.len())?;
 
  dst.write_u8(self.bpp);
  let flags = if self.header.is_some() {
@@ -212,7 +210,7 @@ impl Encode for ExtendedBitmapDataPdu<'_> {
  dst.write_u8(self.codec_id);
  dst.write_u16(self.width);
  dst.write_u16(self.height);
- dst.write_u32(self.data.len() as u32);
+ dst.write_u32(data_len);
  if let Some(header) = &self.header {
  header.encode(dst)?;
  }
@@ -240,7 +238,7 @@ impl<'de> Decode<'de> for ExtendedBitmapDataPdu<'de> {
  let codec_id = src.read_u8();
  let width = src.read_u16();
  let height = src.read_u16();
- let data_length = src.read_u32() as usize;
+ let data_length = cast_length!("bitmap data length", src.read_u32())?;
 
  let expected_remaining_size = if flags.contains(BitmapDataFlags::COMPRESSED_BITMAP_HEADER_PRESENT) {
  data_length + BitmapDataHeader::ENCODED_SIZE
@@ -352,13 +350,23 @@ impl From<&SurfaceCommand<'_>> for SurfaceCommandType {
  }
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive)]
 #[repr(u16)]
 pub enum FrameAction {
  Begin = 0x00,
  End = 0x01,
 }
 
+impl FrameAction {
+ #[expect(
+ clippy::as_conversions,
+ reason = "guarantees discriminant layout, and as is the only way to cast enum -> primitive"
+ )]
+ pub fn as_u16(self) -> u16 {
+ self as u16
+ }
+}
+
 bitflags! {
  #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
  struct BitmapDataFlags: u8 {