From cb99c82c7de057b2e06aa073302eac3ce1778f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 23 Jul 2025 16:46:26 +0400 Subject: [PATCH] refactor(graphics): hand-implement Error trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau --- Cargo.lock | 1 - crates/ironrdp-graphics/Cargo.toml | 1 - crates/ironrdp-graphics/src/pointer.rs | 49 +++++++++++++--- .../src/rdp6/bitmap_stream/decoder.rs | 47 +++++++++++++--- .../src/rdp6/bitmap_stream/encoder.rs | 23 ++++++-- crates/ironrdp-graphics/src/rdp6/rle.rs | 56 +++++++++++++------ crates/ironrdp-graphics/src/rlgr.rs | 31 ++++++++-- crates/ironrdp-graphics/src/zgfx/mod.rs | 50 +++++++++++++---- fuzz/Cargo.lock | 2 - 9 files changed, 205 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 20080294..7714eb54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2590,7 +2590,6 @@ dependencies = [ "lazy_static", "num-derive", "num-traits", - "thiserror 1.0.69", "yuv", ] diff --git a/crates/ironrdp-graphics/Cargo.toml b/crates/ironrdp-graphics/Cargo.toml index df12d7f3..ab5cc81d 100644 --- a/crates/ironrdp-graphics/Cargo.toml +++ b/crates/ironrdp-graphics/Cargo.toml @@ -25,7 +25,6 @@ byteorder = "1.5" # TODO: remove lazy_static.workspace = true # Legacy crate; prefer std::sync::LazyLock or LazyCell num-derive.workspace = true # TODO: remove num-traits.workspace = true # TODO: remove -thiserror = "1" # FIXME: handwrite the Error trait implementations. yuv = { version = "0.8", features = ["rdp"] } [dev-dependencies] diff --git a/crates/ironrdp-graphics/src/pointer.rs b/crates/ironrdp-graphics/src/pointer.rs index 2d4b6c0d..779024e5 100644 --- a/crates/ironrdp-graphics/src/pointer.rs +++ b/crates/ironrdp-graphics/src/pointer.rs @@ -21,20 +21,55 @@ use ironrdp_core::ReadCursor; use ironrdp_pdu::pointer::{ColorPointerAttribute, LargePointerAttribute, PointerAttribute}; -use thiserror::Error; use crate::color_conversion::rdp_16bit_to_rgb; -#[derive(Debug, Error)] +#[derive(Debug)] pub enum PointerError { - #[error("invalid pointer xorMask size. Expected: {expected}, actual: {actual}")] InvalidXorMaskSize { expected: usize, actual: usize }, - #[error("invalid pointer andMask size. Expected: {expected}, actual: {actual}")] InvalidAndMaskSize { expected: usize, actual: usize }, - #[error("not supported pointer bpp: {bpp}")] NotSupportedBpp { bpp: u16 }, - #[error(transparent)] - Pdu(#[from] ironrdp_pdu::PduError), + Pdu(ironrdp_pdu::PduError), +} + +impl core::fmt::Display for PointerError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + PointerError::InvalidXorMaskSize { expected, actual } => { + write!( + f, + "invalid pointer xorMask size. Expected: {expected}, actual: {actual}" + ) + } + PointerError::InvalidAndMaskSize { expected, actual } => { + write!( + f, + "invalid pointer andMask size. Expected: {expected}, actual: {actual}" + ) + } + PointerError::NotSupportedBpp { bpp } => { + write!(f, "not supported pointer bpp: {bpp}") + } + PointerError::Pdu(err) => err.fmt(f), + } + } +} + +impl core::error::Error for PointerError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + PointerError::InvalidXorMaskSize { .. } => None, + PointerError::InvalidAndMaskSize { .. } => None, + PointerError::NotSupportedBpp { .. } => None, + PointerError::Pdu(error) => error.source(), + } + } +} + +impl From for PointerError { + fn from(error: ironrdp_pdu::PduError) -> Self { + PointerError::Pdu(error) + } } /// Represents RDP pointer in decoded form (color channels stored as RGBA pre-multiplied values) diff --git a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs index 59f3a7b0..0e26231e 100644 --- a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs +++ b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs @@ -1,20 +1,53 @@ use ironrdp_core::{decode, DecodeError}; use ironrdp_pdu::bitmap::rdp6::{BitmapStream as BitmapStreamPdu, ColorPlaneDefinition}; -use thiserror::Error; use crate::color_conversion::Rgb; use crate::rdp6::rle::{decompress_8bpp_plane, RleDecodeError}; -#[derive(Debug, Error)] +#[derive(Debug)] pub enum BitmapDecodeError { - #[error("failed to decode RDP6 bitmap stream PDU: {0}")] - Decode(#[from] DecodeError), - #[error("failed to perform RLE decompression of RDP6 bitmap stream: {0}")] - Rle(#[from] RleDecodeError), - #[error("color plane data size provided in PDU is not sufficient to reconstruct the bitmap")] + Decode(DecodeError), + Rle(RleDecodeError), InvalidUncompressedDataSize, } +impl core::fmt::Display for BitmapDecodeError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + BitmapDecodeError::Decode(_error) => write!(f, "failed to decode RDP6 bitmap stream PDU"), + BitmapDecodeError::Rle(_error) => { + write!(f, "failed to perform RLE decompression of RDP6 bitmap stream") + } + BitmapDecodeError::InvalidUncompressedDataSize => write!( + f, + "color plane data size provided in PDU is not sufficient to reconstruct the bitmap" + ), + } + } +} + +impl core::error::Error for BitmapDecodeError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + BitmapDecodeError::Decode(err) => Some(err), + BitmapDecodeError::Rle(err) => Some(err), + BitmapDecodeError::InvalidUncompressedDataSize => None, + } + } +} + +impl From for BitmapDecodeError { + fn from(err: DecodeError) -> Self { + BitmapDecodeError::Decode(err) + } +} + +impl From for BitmapDecodeError { + fn from(err: RleDecodeError) -> Self { + BitmapDecodeError::Rle(err) + } +} + /// Implements decoding of RDP6 bitmap stream PDU (see [`BitmapStreamPdu`]) #[derive(Debug, Default)] pub struct BitmapStreamDecoder { diff --git a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/encoder.rs b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/encoder.rs index 73270aae..14d5306f 100644 --- a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/encoder.rs +++ b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/encoder.rs @@ -1,17 +1,32 @@ use ironrdp_core::{not_enough_bytes_err, EncodeError, WriteCursor}; use ironrdp_pdu::bitmap::rdp6::{BitmapStreamHeader, ColorPlaneDefinition}; -use thiserror::Error; use crate::rdp6::rle::{compress_8bpp_plane, RleEncodeError}; -#[derive(Debug, Error)] +#[derive(Debug)] pub enum BitmapEncodeError { - #[error("failed to rle compress")] Rle(RleEncodeError), - #[error("failed to encode pdu")] Encode(EncodeError), } +impl core::fmt::Display for BitmapEncodeError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + BitmapEncodeError::Rle(_error) => write!(f, "failed to rle compress"), + BitmapEncodeError::Encode(_error) => write!(f, "failed to encode pdu"), + } + } +} + +impl core::error::Error for BitmapEncodeError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + BitmapEncodeError::Rle(error) => Some(error), + BitmapEncodeError::Encode(error) => Some(error), + } + } +} + pub trait ColorChannels { const STRIDE: usize; const R: usize; diff --git a/crates/ironrdp-graphics/src/rdp6/rle.rs b/crates/ironrdp-graphics/src/rdp6/rle.rs index f480c318..1c415adf 100644 --- a/crates/ironrdp-graphics/src/rdp6/rle.rs +++ b/crates/ironrdp-graphics/src/rdp6/rle.rs @@ -3,36 +3,60 @@ use std::io::{Read, Write}; use byteorder::ReadBytesExt; use ironrdp_core::WriteCursor; -use thiserror::Error; /// Maximum possible segment size is 47 (run_length = 2, raw_bytes_count = 15), which is treated as /// special mode segment, which repeats last decoded byte in scanline 32 + raw_bytes_count times const MAX_DECODED_SEGMENT_SIZE: usize = 47; -#[derive(Debug, Error)] +#[derive(Debug)] pub enum RleDecodeError { - #[error("failed to read RLE-compressed data: {0}")] - ReadCompressedData(#[source] std::io::Error), - - #[error("failed to write decompressed data: {0}")] - WriteDecompressedData(#[source] std::io::Error), - - #[error("invalid RLE segment header")] + ReadCompressedData(std::io::Error), + WriteDecompressedData(std::io::Error), InvalidSegmentHeader, - - #[error("decoded scanline segments length exceeds scanline length")] SegmentDoNotFitScanline, } -#[derive(Debug, Error)] -pub enum RleEncodeError { - #[error("not enough data to compress")] - NotEnoughBytes, +impl core::fmt::Display for RleDecodeError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + RleDecodeError::ReadCompressedData(_error) => write!(f, "failed to read RLE-compressed data"), + RleDecodeError::WriteDecompressedData(_error) => write!(f, "failed to write decompressed data"), + RleDecodeError::InvalidSegmentHeader => write!(f, "invalid RLE segment header"), + RleDecodeError::SegmentDoNotFitScanline => { + write!(f, "decoded scanline segments length exceeds scanline length") + } + } + } +} - #[error("destination buffer is too small")] +impl core::error::Error for RleDecodeError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + RleDecodeError::ReadCompressedData(error) => Some(error), + RleDecodeError::WriteDecompressedData(error) => Some(error), + RleDecodeError::InvalidSegmentHeader => None, + RleDecodeError::SegmentDoNotFitScanline => None, + } + } +} + +#[derive(Debug)] +pub enum RleEncodeError { + NotEnoughBytes, BufferTooSmall, } +impl core::fmt::Display for RleEncodeError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + RleEncodeError::NotEnoughBytes => write!(f, "not enough data to compress"), + RleEncodeError::BufferTooSmall => write!(f, "destination buffer is too small"), + } + } +} + +impl core::error::Error for RleEncodeError {} + /// RLE-encoded color plane decoder implementation for RDP6 bitmap stream #[derive(Debug)] struct RlePlaneDecoder { diff --git a/crates/ironrdp-graphics/src/rlgr.rs b/crates/ironrdp-graphics/src/rlgr.rs index 05db9d23..38c1f454 100644 --- a/crates/ironrdp-graphics/src/rlgr.rs +++ b/crates/ironrdp-graphics/src/rlgr.rs @@ -6,7 +6,6 @@ use bitvec::order::Msb0; use bitvec::prelude::*; use bitvec::slice::BitSlice; use ironrdp_pdu::codecs::rfx::EntropyAlgorithm; -use thiserror::Error; use crate::utils::Bits; @@ -355,10 +354,32 @@ impl From for CompressionMode { } } -#[derive(Debug, Error)] +#[derive(Debug)] pub enum RlgrError { - #[error("IO error: {0}")] - IoError(#[from] io::Error), - #[error("the input tile is empty")] + IoError(io::Error), EmptyTile, } + +impl core::fmt::Display for RlgrError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::IoError(_error) => write!(f, "IO error"), + Self::EmptyTile => write!(f, "the input tile is empty"), + } + } +} + +impl core::error::Error for RlgrError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::IoError(error) => Some(error), + Self::EmptyTile => None, + } + } +} + +impl From for RlgrError { + fn from(err: io::Error) -> Self { + Self::IoError(err) + } +} diff --git a/crates/ironrdp-graphics/src/zgfx/mod.rs b/crates/ironrdp-graphics/src/zgfx/mod.rs index cf978252..d140b374 100644 --- a/crates/ironrdp-graphics/src/zgfx/mod.rs +++ b/crates/ironrdp-graphics/src/zgfx/mod.rs @@ -10,7 +10,6 @@ use bitvec::field::BitField as _; use bitvec::order::Msb0; use bitvec::slice::BitSlice; use byteorder::WriteBytesExt; -use thiserror::Error; use self::circular_buffer::FixedCircularBuffer; use self::control_messages::{BulkEncodedData, CompressionFlags, SegmentedDataPdu}; @@ -426,27 +425,54 @@ lazy_static::lazy_static! { ]; } -#[derive(Debug, Error)] +#[derive(Debug)] pub enum ZgfxError { - #[error("IO error")] - IOError(#[from] io::Error), - #[error("invalid compression type")] + IOError(io::Error), InvalidCompressionType, - #[error("invalid segmented descriptor")] InvalidSegmentedDescriptor, - #[error( - "decompressed size of segments ({}) does not equal to uncompressed size ({})", - decompressed_size, - uncompressed_size - )] InvalidDecompressedSize { decompressed_size: usize, uncompressed_size: usize, }, - #[error("token bits not found")] TokenBitsNotFound, } +impl core::fmt::Display for ZgfxError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::IOError(_error) => write!(f, "IO error"), + Self::InvalidCompressionType => write!(f, "invalid compression type"), + Self::InvalidSegmentedDescriptor => write!(f, "invalid segmented descriptor"), + Self::InvalidDecompressedSize { + decompressed_size, + uncompressed_size, + } => write!( + f, + "decompressed size of segments ({decompressed_size}) does not equal to uncompressed size ({uncompressed_size})", + ), + Self::TokenBitsNotFound => write!(f, "token bits not found"), + } + } +} + +impl core::error::Error for ZgfxError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::IOError(error) => Some(error), + Self::InvalidCompressionType => None, + Self::InvalidSegmentedDescriptor => None, + Self::InvalidDecompressedSize { .. } => None, + Self::TokenBitsNotFound => None, + } + } +} + +impl From for ZgfxError { + fn from(err: io::Error) -> Self { + Self::IOError(err) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 69b8c80e..ebd898ee 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -292,7 +292,6 @@ dependencies = [ "ironrdp-core", "ironrdp-pdu", "ironrdp-svc", - "thiserror", "tracing", ] @@ -374,7 +373,6 @@ dependencies = [ "lazy_static", "num-derive", "num-traits", - "thiserror", "yuv", ]