From d62a6896850ae6d6bafc400e4ac2cca41c254c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 21:47:15 -0400 Subject: [PATCH 01/14] refactor: check for additional lints --- Cargo.lock | 19 ---- clippy.toml | 1 + crates/ironrdp-acceptor/src/connection.rs | 9 +- crates/ironrdp-acceptor/src/lib.rs | 7 +- crates/ironrdp-acceptor/src/util.rs | 4 +- crates/ironrdp-async/Cargo.toml | 1 - crates/ironrdp-async/src/framed.rs | 3 +- crates/ironrdp-async/src/lib.rs | 6 +- crates/ironrdp-blocking/Cargo.toml | 1 - crates/ironrdp-blocking/src/framed.rs | 3 +- crates/ironrdp-blocking/src/lib.rs | 6 +- crates/ironrdp-client/Cargo.toml | 3 - crates/ironrdp-client/src/gui.rs | 20 ++++- crates/ironrdp-client/src/lib.rs | 9 ++ crates/ironrdp-client/src/main.rs | 6 +- crates/ironrdp-client/src/rdp.rs | 9 +- crates/ironrdp-cliprdr-native/Cargo.toml | 5 +- crates/ironrdp-cliprdr-native/src/lib.rs | 3 +- crates/ironrdp-cliprdr/src/lib.rs | 8 +- .../ironrdp-cliprdr/src/pdu/capabilities.rs | 15 ++-- .../src/pdu/client_temporary_directory.rs | 18 ++-- .../ironrdp-cliprdr/src/pdu/file_contents.rs | 4 +- .../src/pdu/format_data/file_list.rs | 18 ++-- .../src/pdu/format_data/metafile.rs | 16 ++-- crates/ironrdp-cliprdr/src/pdu/format_list.rs | 1 + crates/ironrdp-cliprdr/src/pdu/mod.rs | 10 +-- crates/ironrdp-connector/Cargo.toml | 1 - .../src/channel_connection.rs | 2 +- crates/ironrdp-connector/src/connection.rs | 37 ++++---- crates/ironrdp-connector/src/legacy.rs | 2 +- crates/ironrdp-connector/src/lib.rs | 8 +- crates/ironrdp-dvc/src/lib.rs | 5 +- crates/ironrdp-error/src/lib.rs | 7 +- crates/ironrdp-fuzzing/src/lib.rs | 7 ++ crates/ironrdp-fuzzing/src/oracles/mod.rs | 2 +- .../ironrdp-graphics/src/image_processing.rs | 6 +- crates/ironrdp-graphics/src/lib.rs | 6 ++ .../src/rdp6/bitmap_stream/decoder.rs | 2 +- crates/ironrdp-graphics/src/rdp6/rle.rs | 25 +++--- .../src/rectangle_processing.rs | 1 + crates/ironrdp-graphics/src/rle.rs | 6 +- crates/ironrdp-graphics/src/utils.rs | 10 ++- .../src/zgfx/circular_buffer.rs | 6 +- .../src/zgfx/control_messages.rs | 12 +-- crates/ironrdp-graphics/src/zgfx/mod.rs | 4 +- crates/ironrdp-input/src/lib.rs | 8 +- crates/ironrdp-pdu-generators/Cargo.toml | 4 +- crates/ironrdp-pdu-generators/src/lib.rs | 7 +- crates/ironrdp-pdu/src/basic_output/bitmap.rs | 6 +- .../src/basic_output/bitmap/rdp6.rs | 2 +- .../ironrdp-pdu/src/basic_output/fast_path.rs | 8 +- .../ironrdp-pdu/src/basic_output/pointer.rs | 12 +-- .../src/basic_output/surface_commands.rs | 12 +-- crates/ironrdp-pdu/src/ber.rs | 42 ++++----- crates/ironrdp-pdu/src/codecs/rfx.rs | 7 +- .../src/codecs/rfx/data_messages.rs | 12 +-- .../src/codecs/rfx/header_messages.rs | 64 +++++++++++--- crates/ironrdp-pdu/src/crypto/rc4.rs | 6 +- crates/ironrdp-pdu/src/crypto/rsa.rs | 2 +- crates/ironrdp-pdu/src/cursor.rs | 4 + crates/ironrdp-pdu/src/gcc.rs | 4 +- crates/ironrdp-pdu/src/gcc/core_data.rs | 4 +- crates/ironrdp-pdu/src/geometry.rs | 1 + crates/ironrdp-pdu/src/lib.rs | 10 ++- crates/ironrdp-pdu/src/mcs.rs | 37 +++----- crates/ironrdp-pdu/src/rdp/capability_sets.rs | 2 +- .../src/rdp/capability_sets/bitmap_codecs.rs | 2 +- crates/ironrdp-pdu/src/rdp/server_license.rs | 4 +- .../client_new_license_request.rs | 4 +- .../client_new_license_request/tests.rs | 2 +- .../client_platform_challenge_response.rs | 14 +-- .../test.rs | 6 +- .../server_license/server_license_request.rs | 1 + crates/ironrdp-pdu/src/rdp/vc/dvc.rs | 6 +- crates/ironrdp-pdu/src/rdp/vc/dvc/display.rs | 14 +-- .../src/rdp/vc/dvc/gfx/graphics_messages.rs | 13 +-- .../dvc/gfx/graphics_messages/avc_messages.rs | 4 +- .../vc/dvc/gfx/graphics_messages/client.rs | 10 +-- .../vc/dvc/gfx/graphics_messages/server.rs | 2 +- crates/ironrdp-pdu/src/utils.rs | 1 + crates/ironrdp-rdcleanpath/src/lib.rs | 10 ++- crates/ironrdp-rdpdr/README.md | 5 +- crates/ironrdp-rdpdr/src/lib.rs | 12 ++- crates/ironrdp-rdpdr/src/pdu/efs.rs | 88 +++++++++---------- crates/ironrdp-rdpdr/src/pdu/mod.rs | 4 +- crates/ironrdp-rdpsnd/Cargo.toml | 1 - crates/ironrdp-server/src/builder.rs | 5 +- crates/ironrdp-server/src/capabilities.rs | 2 +- crates/ironrdp-server/src/encoder/bitmap.rs | 38 ++++---- crates/ironrdp-server/src/encoder/mod.rs | 18 ++-- crates/ironrdp-server/src/handler.rs | 2 + crates/ironrdp-server/src/lib.rs | 2 + crates/ironrdp-session-generators/Cargo.toml | 6 +- crates/ironrdp-session-generators/src/lib.rs | 7 +- crates/ironrdp-session/Cargo.toml | 3 +- crates/ironrdp-session/src/active_stage.rs | 8 +- crates/ironrdp-session/src/fast_path.rs | 24 +++-- crates/ironrdp-session/src/image.rs | 4 + crates/ironrdp-session/src/lib.rs | 6 +- crates/ironrdp-session/src/rfx.rs | 18 ++-- crates/ironrdp-session/src/x224/display.rs | 2 +- crates/ironrdp-session/src/x224/gfx.rs | 6 +- crates/ironrdp-session/src/x224/mod.rs | 81 ++++++++--------- crates/ironrdp-svc/README.md | 3 + crates/ironrdp-svc/src/lib.rs | 32 ++++--- crates/ironrdp-testsuite-core/Cargo.toml | 4 +- crates/ironrdp-testsuite-core/src/lib.rs | 7 ++ .../ironrdp-testsuite-core/tests/pdu/rfx.rs | 4 +- crates/ironrdp-tokio/Cargo.toml | 1 - crates/ironrdp-web/src/canvas.rs | 6 +- crates/ironrdp-web/src/image.rs | 4 +- crates/ironrdp-web/src/lib.rs | 2 + crates/ironrdp-web/src/network_client.rs | 2 +- crates/ironrdp-web/src/session.rs | 14 ++- xtask/src/check.rs | 82 ++++++++++++++++- xtask/src/cov.rs | 22 ++++- xtask/src/main.rs | 5 ++ xtask/src/section.rs | 2 +- 118 files changed, 729 insertions(+), 504 deletions(-) create mode 100644 clippy.toml create mode 100644 crates/ironrdp-svc/README.md diff --git a/Cargo.lock b/Cargo.lock index fb27b927f..bc8f034eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1836,7 +1836,6 @@ dependencies = [ "bytes", "ironrdp-connector", "ironrdp-pdu", - "tap", "tracing", ] @@ -1847,7 +1846,6 @@ dependencies = [ "bytes", "ironrdp-connector", "ironrdp-pdu", - "tap", "tracing", ] @@ -1856,14 +1854,11 @@ name = "ironrdp-client" version = "0.1.0" dependencies = [ "anyhow", - "chrono", "clap", "exitcode", "inquire", "ironrdp", "ironrdp-cliprdr-native", - "ironrdp-rdpdr", - "ironrdp-rdpsnd", "ironrdp-tls", "ironrdp-tokio", "semver", @@ -1910,7 +1905,6 @@ dependencies = [ "ironrdp-pdu", "ironrdp-svc", "rand_core 0.6.4", - "rstest", "sspi", "tracing", ] @@ -2002,10 +1996,6 @@ dependencies = [ [[package]] name = "ironrdp-pdu-generators" version = "0.0.0" -dependencies = [ - "ironrdp-pdu", - "proptest", -] [[package]] name = "ironrdp-rdcleanpath" @@ -2030,7 +2020,6 @@ version = "0.1.0" dependencies = [ "ironrdp-pdu", "ironrdp-svc", - "tracing", ] [[package]] @@ -2055,23 +2044,16 @@ version = "0.1.0" dependencies = [ "bitflags 2.4.0", "ironrdp-connector", - "ironrdp-dvc", "ironrdp-error", "ironrdp-graphics", "ironrdp-pdu", "ironrdp-svc", - "sspi", "tracing", ] [[package]] name = "ironrdp-session-generators" version = "0.0.0" -dependencies = [ - "ironrdp-pdu-generators", - "ironrdp-session", - "proptest", -] [[package]] name = "ironrdp-svc" @@ -2123,7 +2105,6 @@ dependencies = [ "bytes", "ironrdp-async", "tokio", - "tokio-util", ] [[package]] diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..0ac765717 --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +semicolon-outside-block-ignore-multiline = true diff --git a/crates/ironrdp-acceptor/src/connection.rs b/crates/ironrdp-acceptor/src/connection.rs index ccca436cc..c352464af 100644 --- a/crates/ironrdp-acceptor/src/connection.rs +++ b/crates/ironrdp-acceptor/src/connection.rs @@ -227,6 +227,7 @@ impl Sequence for Acceptor { .optional_data .early_capability_flags; + #[allow(clippy::arithmetic_side_effects)] // IO channel ID is not big enough for overflowing let channels = settings_initial .conference_create_request .gcc_blocks @@ -236,10 +237,10 @@ impl Sequence for Acceptor { .channels .into_iter() .enumerate() - .map(|(i, c)| (i as u16 + self.io_channel_id + 1, c)) + .map(|(i, c)| (u16::try_from(i).unwrap() + self.io_channel_id + 1, c)) .collect() }) - .unwrap_or(Vec::new()); + .unwrap_or_default(); ( Written::Nothing, @@ -400,8 +401,8 @@ impl Sequence for Acceptor { monitors: vec![gcc::Monitor { left: 0, top: 0, - right: self.desktop_size.width as i32, - bottom: self.desktop_size.height as i32, + right: i32::from(self.desktop_size.width), + bottom: i32::from(self.desktop_size.height), flags: gcc::MonitorFlags::PRIMARY, }], }); diff --git a/crates/ironrdp-acceptor/src/lib.rs b/crates/ironrdp-acceptor/src/lib.rs index 853e4b497..470a46cc8 100644 --- a/crates/ironrdp-acceptor/src/lib.rs +++ b/crates/ironrdp-acceptor/src/lib.rs @@ -3,15 +3,18 @@ extern crate tracing; use ironrdp_async::{Framed, FramedRead, FramedWrite, StreamWrapper}; use ironrdp_connector::{custom_err, ConnectorResult, Sequence, Written}; +use ironrdp_pdu::write_buf::WriteBuf; mod channel_connection; mod connection; mod finalization; mod util; -pub use connection::{Acceptor, AcceptorResult}; pub use ironrdp_connector::DesktopSize; -use ironrdp_pdu::write_buf::WriteBuf; + +pub use self::channel_connection::{ChannelConnectionSequence, ChannelConnectionState}; +pub use self::connection::{Acceptor, AcceptorResult, AcceptorState}; +pub use self::finalization::{FinalizationSequence, FinalizationState}; pub enum BeginResult where diff --git a/crates/ironrdp-acceptor/src/util.rs b/crates/ironrdp-acceptor/src/util.rs index 3bd3eaa21..834a30561 100644 --- a/crates/ironrdp-acceptor/src/util.rs +++ b/crates/ironrdp-acceptor/src/util.rs @@ -4,7 +4,7 @@ use ironrdp_connector::{ConnectorError, ConnectorErrorExt, ConnectorResult}; use ironrdp_pdu::write_buf::WriteBuf; use ironrdp_pdu::{rdp, PduParsing}; -pub fn encode_send_data_indication( +pub(crate) fn encode_send_data_indication( initiator_id: u16, channel_id: u16, user_msg: &T, @@ -30,7 +30,7 @@ where Ok(written) } -pub fn wrap_share_data(pdu: rdp::headers::ShareDataPdu, io_channel_id: u16) -> rdp::headers::ShareControlHeader { +pub(crate) fn wrap_share_data(pdu: rdp::headers::ShareDataPdu, io_channel_id: u16) -> rdp::headers::ShareControlHeader { rdp::headers::ShareControlHeader { share_id: 0, pdu_source: io_channel_id, diff --git a/crates/ironrdp-async/Cargo.toml b/crates/ironrdp-async/Cargo.toml index 89705d1bf..f600fff63 100644 --- a/crates/ironrdp-async/Cargo.toml +++ b/crates/ironrdp-async/Cargo.toml @@ -20,5 +20,4 @@ bytes = "1" ironrdp-connector.workspace = true ironrdp-pdu.workspace = true # ironrdp-session.workspace = true -tap = "1" tracing.workspace = true diff --git a/crates/ironrdp-async/src/framed.rs b/crates/ironrdp-async/src/framed.rs index b1478ffbe..383dffbb4 100644 --- a/crates/ironrdp-async/src/framed.rs +++ b/crates/ironrdp-async/src/framed.rs @@ -108,7 +108,8 @@ where if self.buf.len() >= length { return Ok(self.buf.split_to(length)); } else { - self.buf.reserve(length - self.buf.len()); + self.buf + .reserve(length.checked_sub(self.buf.len()).expect("length > self.buf.len()")); } let len = self.read().await?; diff --git a/crates/ironrdp-async/src/lib.rs b/crates/ironrdp-async/src/lib.rs index d7dfd19a2..5a04fd17f 100644 --- a/crates/ironrdp-async/src/lib.rs +++ b/crates/ironrdp-async/src/lib.rs @@ -5,6 +5,6 @@ mod connector; mod framed; mod session; -pub use connector::*; -pub use framed::*; -pub use session::*; +pub use self::connector::*; +pub use self::framed::*; +// pub use self::session::*; diff --git a/crates/ironrdp-blocking/Cargo.toml b/crates/ironrdp-blocking/Cargo.toml index f7fbddbba..f3df84782 100644 --- a/crates/ironrdp-blocking/Cargo.toml +++ b/crates/ironrdp-blocking/Cargo.toml @@ -20,5 +20,4 @@ bytes = "1" ironrdp-connector.workspace = true ironrdp-pdu.workspace = true # ironrdp-session.workspace = true -tap = "1" tracing.workspace = true diff --git a/crates/ironrdp-blocking/src/framed.rs b/crates/ironrdp-blocking/src/framed.rs index 3aa6cfd36..3cf4cfd12 100644 --- a/crates/ironrdp-blocking/src/framed.rs +++ b/crates/ironrdp-blocking/src/framed.rs @@ -49,7 +49,8 @@ where if self.buf.len() >= length { return Ok(self.buf.split_to(length)); } else { - self.buf.reserve(length - self.buf.len()); + self.buf + .reserve(length.checked_sub(self.buf.len()).expect("length > self.buf.len()")); } let len = self.read()?; diff --git a/crates/ironrdp-blocking/src/lib.rs b/crates/ironrdp-blocking/src/lib.rs index d7dfd19a2..5a04fd17f 100644 --- a/crates/ironrdp-blocking/src/lib.rs +++ b/crates/ironrdp-blocking/src/lib.rs @@ -5,6 +5,6 @@ mod connector; mod framed; mod session; -pub use connector::*; -pub use framed::*; -pub use session::*; +pub use self::connector::*; +pub use self::framed::*; +// pub use self::session::*; diff --git a/crates/ironrdp-client/Cargo.toml b/crates/ironrdp-client/Cargo.toml index 3fa386d6c..a33476ccc 100644 --- a/crates/ironrdp-client/Cargo.toml +++ b/crates/ironrdp-client/Cargo.toml @@ -32,8 +32,6 @@ ironrdp = { workspace = true, features = ["input", "graphics", "dvc", "rdpdr", " ironrdp-cliprdr-native.workspace = true ironrdp-tls.workspace = true ironrdp-tokio.workspace = true -ironrdp-rdpsnd.workspace = true -ironrdp-rdpdr.workspace = true sspi = { workspace = true, features = ["network_client", "dns_resolver"] } # enable additional features # Windowing and rendering @@ -53,7 +51,6 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } tokio = { version = "1", features = ["full"]} # Utils -chrono = "0.4" whoami = "1.4" anyhow = "1" smallvec = "1.10" diff --git a/crates/ironrdp-client/src/gui.rs b/crates/ironrdp-client/src/gui.rs index 9a8876e0f..2de9b67d0 100644 --- a/crates/ironrdp-client/src/gui.rs +++ b/crates/ironrdp-client/src/gui.rs @@ -1,18 +1,20 @@ +#![allow(clippy::print_stderr, clippy::print_stdout)] // allowed in this module only + use std::num::NonZeroU32; use anyhow::Context as _; use tokio::sync::mpsc; use winit::dpi::LogicalPosition; use winit::event::{self, Event, WindowEvent}; -use winit::event_loop::{ControlFlow, EventLoop, EventLoopBuilder}; +use winit::event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy}; use winit::window::{Window, WindowBuilder}; use crate::rdp::{RdpInputEvent, RdpOutputEvent}; pub struct GuiContext { - pub window: Window, - pub event_loop: EventLoop, - pub context: softbuffer::Context, + window: Window, + event_loop: EventLoop, + context: softbuffer::Context, } impl GuiContext { @@ -24,6 +26,7 @@ impl GuiContext { .build(&event_loop) .context("unable to create winit Window")?; + // SAFETY: both the context and the window are held by the GuiContext let context = unsafe { softbuffer::Context::new(&window) } .map_err(|e| anyhow::Error::msg(format!("unable to initialize softbuffer context: {e}")))?; @@ -34,6 +37,14 @@ impl GuiContext { }) } + pub fn window(&self) -> &Window { + &self.window + } + + pub fn create_event_proxy(&self) -> EventLoopProxy { + self.event_loop.create_proxy() + } + pub fn run(self, input_event_sender: mpsc::UnboundedSender) -> ! { let Self { window, @@ -41,6 +52,7 @@ impl GuiContext { context, } = self; + // SAFETY: both the context and the window are kept alive until the end of this function’s scope let mut surface = unsafe { softbuffer::Surface::new(&context, &window) }.expect("surface"); let mut input_database = ironrdp::input::Database::new(); diff --git a/crates/ironrdp-client/src/lib.rs b/crates/ironrdp-client/src/lib.rs index f3428ae7e..b9199ded4 100644 --- a/crates/ironrdp-client/src/lib.rs +++ b/crates/ironrdp-client/src/lib.rs @@ -1,3 +1,12 @@ +#![allow(unused_crate_dependencies)] // false positives because there is both a library and a binary + +// No need to be as strict as in production libraries +#![allow(clippy::arithmetic_side_effects)] +#![allow(clippy::cast_lossless)] +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] + #[macro_use] extern crate tracing; diff --git a/crates/ironrdp-client/src/main.rs b/crates/ironrdp-client/src/main.rs index 756590894..ada59e57a 100644 --- a/crates/ironrdp-client/src/main.rs +++ b/crates/ironrdp-client/src/main.rs @@ -1,3 +1,5 @@ +#![allow(unused_crate_dependencies)] // false positives because there is both a library and a binary + #[macro_use] extern crate tracing; @@ -16,11 +18,11 @@ fn main() -> anyhow::Result<()> { let gui = GuiContext::init().context("unable to initialize GUI context")?; debug!("GUI context initialized"); - let window_size = gui.window.inner_size(); + let window_size = gui.window().inner_size(); config.connector.desktop_size.width = u16::try_from(window_size.width).unwrap(); config.connector.desktop_size.height = u16::try_from(window_size.height).unwrap(); - let event_loop_proxy = gui.event_loop.create_proxy(); + let event_loop_proxy = gui.create_event_proxy(); 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 023ec5c6c..875fb4921 100644 --- a/crates/ironrdp-client/src/rdp.rs +++ b/crates/ironrdp-client/src/rdp.rs @@ -1,12 +1,11 @@ use ironrdp::cliprdr::backend::{ClipboardMessage, CliprdrBackendFactory}; -use ironrdp::cliprdr::Cliprdr; use ironrdp::connector::sspi::network_client::reqwest_network_client::RequestClientFactory; use ironrdp::connector::{ConnectionResult, ConnectorResult}; use ironrdp::graphics::image_processing::PixelFormat; use ironrdp::pdu::input::fast_path::FastPathInputEvent; use ironrdp::session::image::DecodedImage; use ironrdp::session::{ActiveStage, ActiveStageOutput, SessionResult}; -use ironrdp::{connector, session}; +use ironrdp::{cliprdr, connector, rdpdr, rdpsnd, session}; use smallvec::SmallVec; use tokio::net::TcpStream; use tokio::sync::mpsc; @@ -108,13 +107,13 @@ async fn connect( .with_server_name(&config.destination) .with_credssp_network_client(RequestClientFactory) // .with_static_channel(ironrdp::dvc::Drdynvc::new()) // FIXME: drdynvc is not working - .with_static_channel(ironrdp::rdpsnd::Rdpsnd::new()) - .with_static_channel(ironrdp_rdpdr::Rdpdr::new("IronRDP".to_string()).with_smartcard(0)); + .with_static_channel(rdpsnd::Rdpsnd::new()) + .with_static_channel(rdpdr::Rdpdr::new("IronRDP".to_owned()).with_smartcard(0)); if let Some(builder) = cliprdr_factory { let backend = builder.build_cliprdr_backend(); - let cliprdr = Cliprdr::new(backend); + let cliprdr = cliprdr::Cliprdr::new(backend); connector.attach_static_channel(cliprdr); } diff --git a/crates/ironrdp-cliprdr-native/Cargo.toml b/crates/ironrdp-cliprdr-native/Cargo.toml index 4b3c17b0c..2114df012 100644 --- a/crates/ironrdp-cliprdr-native/Cargo.toml +++ b/crates/ironrdp-cliprdr-native/Cargo.toml @@ -16,14 +16,11 @@ categories.workspace = true doctest = false test = false -[dependencies] +[target.'cfg(windows)'.dependencies] ironrdp-cliprdr.workspace = true ironrdp-svc.workspace = true thiserror.workspace = true tracing.workspace = true - -[target.'cfg(windows)'.dependencies] - windows = { version = "0.48.0", features = [ "Win32_Foundation", "Win32_System_DataExchange", diff --git a/crates/ironrdp-cliprdr-native/src/lib.rs b/crates/ironrdp-cliprdr-native/src/lib.rs index 9e5818f3a..924f18c5d 100644 --- a/crates/ironrdp-cliprdr-native/src/lib.rs +++ b/crates/ironrdp-cliprdr-native/src/lib.rs @@ -8,8 +8,7 @@ #![warn(clippy::as_ptr_cast_mut)] #![warn(clippy::cast_ptr_alignment)] #![warn(clippy::fn_to_numeric_cast_any)] -// TODO(@pacmancoder): Enable after toolchain update -// #![warn(clippy::ptr_cast_constness)] +#![warn(clippy::ptr_cast_constness)] #[cfg(windows)] mod windows; diff --git a/crates/ironrdp-cliprdr/src/lib.rs b/crates/ironrdp-cliprdr/src/lib.rs index e1f50ae65..e06bf0821 100644 --- a/crates/ironrdp-cliprdr/src/lib.rs +++ b/crates/ironrdp-cliprdr/src/lib.rs @@ -1,4 +1,9 @@ #![doc = include_str!("../README.md")] +#![allow(clippy::arithmetic_side_effects)] // FIXME: remove +#![allow(clippy::cast_lossless)] // FIXME: remove +#![allow(clippy::cast_possible_truncation)] // FIXME: remove +#![allow(clippy::cast_possible_wrap)] // FIXME: remove +#![allow(clippy::cast_sign_loss)] // FIXME: remove pub mod backend; pub mod pdu; @@ -17,6 +22,7 @@ use pdu::{ use thiserror::Error; use tracing::{error, info}; +#[rustfmt::skip] // do not reorder use crate::pdu::FormatList; /// PDUs for sending to the server on the CLIPRDR channel. @@ -253,7 +259,7 @@ impl StaticVirtualChannelProcessor for Cliprdr { } } - fn compression_condition(&self) -> ironrdp_svc::CompressionCondition { + fn compression_condition(&self) -> CompressionCondition { CompressionCondition::WhenRdpDataIsCompressed } } diff --git a/crates/ironrdp-cliprdr/src/pdu/capabilities.rs b/crates/ironrdp-cliprdr/src/pdu/capabilities.rs index 7f4b6ee65..876acc8b6 100644 --- a/crates/ironrdp-cliprdr/src/pdu/capabilities.rs +++ b/crates/ironrdp-cliprdr/src/pdu/capabilities.rs @@ -226,11 +226,12 @@ pub enum ClipboardProtocolVersion { } impl ClipboardProtocolVersion { - const VERSION_VALUE_V1: u32 = 0x00000001; - const VERSION_VALUE_V2: u32 = 0x00000002; + const VERSION_VALUE_V1: u32 = 0x0000_0001; + const VERSION_VALUE_V2: u32 = 0x0000_0002; const NAME: &str = "CLIPRDR_CAPS_VERSION"; + #[must_use] pub fn downgrade(self, other: Self) -> Self { if self != other { return Self::V1; @@ -271,23 +272,23 @@ bitflags! { /// Short Format Name variant MUST be used. If this flag is set by both /// protocol endpoints, then the Long Format Name variant MUST be /// used. - const USE_LONG_FORMAT_NAMES = 0x00000002; + const USE_LONG_FORMAT_NAMES = 0x0000_0002; /// File copy and paste using stream-based operations are supported /// using the File Contents Request PDU and File Contents Response /// PDU. - const STREAM_FILECLIP_ENABLED = 0x00000004; + const STREAM_FILECLIP_ENABLED = 0x0000_0004; /// Indicates that any description of files to copy and paste MUST NOT /// include the source path of the files. - const FILECLIP_NO_FILE_PATHS = 0x00000008; + const FILECLIP_NO_FILE_PATHS = 0x0000_0008; /// Locking and unlocking of File Stream data on the clipboard is /// supported using the Lock Clipboard Data PDU and Unlock Clipboard /// Data PDU. - const CAN_LOCK_CLIPDATA = 0x00000010; + const CAN_LOCK_CLIPDATA = 0x0000_0010; /// Indicates support for transferring files that are larger than /// 4,294,967,295 bytes in size. If this flag is not set, then only files of /// size less than or equal to 4,294,967,295 bytes can be exchanged /// using the File Contents Request PDU and File Contents /// Response PDU. - const HUGE_FILE_SUPPORT_ENABLED = 0x00000020; + const HUGE_FILE_SUPPORT_ENABLED = 0x0000_0020; } } diff --git a/crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs b/crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs index f942fcc7d..9b071992a 100644 --- a/crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs +++ b/crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs @@ -2,9 +2,7 @@ use std::borrow::Cow; use ironrdp_pdu::cursor::{ReadCursor, WriteCursor}; use ironrdp_pdu::utils::{read_string_from_cursor, write_string_to_cursor, CharacterSet}; -use ironrdp_pdu::{ - cast_int, ensure_fixed_part_size, ensure_size, invalid_message_err, PduDecode, PduEncode, PduResult, -}; +use ironrdp_pdu::{cast_int, ensure_size, invalid_message_err, PduDecode, PduEncode, PduResult}; use crate::pdu::PartialHeader; @@ -18,7 +16,7 @@ impl ClientTemporaryDirectory<'_> { const PATH_BUFFER_SIZE: usize = 520; const NAME: &str = "CLIPRDR_TEMP_DIRECTORY"; - const FIXED_PART_SIZE: usize = Self::PATH_BUFFER_SIZE; + const INNER_SIZE: usize = Self::PATH_BUFFER_SIZE; /// Creates new `ClientTemporaryDirectory` and encodes given path to UTF-16 representation. pub fn new(path: &str) -> PduResult { @@ -41,18 +39,14 @@ impl ClientTemporaryDirectory<'_> { read_string_from_cursor(&mut cursor, CharacterSet::Unicode, true) .map_err(|_| invalid_message_err!("wszTempDir", "failed to decode temp dir path")) } - - fn inner_size(&self) -> usize { - Self::FIXED_PART_SIZE - } } impl PduEncode for ClientTemporaryDirectory<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { - let header = PartialHeader::new(cast_int!("dataLen", self.inner_size())?); + let header = PartialHeader::new(cast_int!("dataLen", Self::INNER_SIZE)?); header.encode(dst)?; - ensure_size!(in: dst, size: self.inner_size()); + ensure_size!(in: dst, size: Self::INNER_SIZE); dst.write_slice(&self.path_buffer); Ok(()) @@ -63,7 +57,7 @@ impl PduEncode for ClientTemporaryDirectory<'_> { } fn size(&self) -> usize { - PartialHeader::SIZE + self.inner_size() + PartialHeader::SIZE + Self::INNER_SIZE } } @@ -71,7 +65,7 @@ impl<'de> PduDecode<'de> for ClientTemporaryDirectory<'de> { fn decode(src: &mut ReadCursor<'de>) -> PduResult { let _header = PartialHeader::decode(src)?; - ensure_fixed_part_size!(in: src); + ensure_size!(in: src, size: Self::INNER_SIZE); let buffer = src.read_slice(Self::PATH_BUFFER_SIZE); Ok(Self { diff --git a/crates/ironrdp-cliprdr/src/pdu/file_contents.rs b/crates/ironrdp-cliprdr/src/pdu/file_contents.rs index b570f1b5a..79acdc338 100644 --- a/crates/ironrdp-cliprdr/src/pdu/file_contents.rs +++ b/crates/ironrdp-cliprdr/src/pdu/file_contents.rs @@ -15,12 +15,12 @@ bitflags! { /// returned as a 64-bit, unsigned integer. The cbRequested field MUST be set to /// 0x00000008 and both the nPositionLow and nPositionHigh fields MUST be /// set to 0x00000000. - const SIZE = 0x00000001; + const SIZE = 0x0000_0001; /// A request for the data present in the file identified by the lindex field. The data /// to be retrieved is extracted starting from the offset given by the nPositionLow /// and nPositionHigh fields. The maximum number of bytes to extract is specified /// by the cbRequested field. - const DATA = 0x00000002; + const DATA = 0x0000_0002; } } diff --git a/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs b/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs index babb128eb..4515a38da 100644 --- a/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs +++ b/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs @@ -8,11 +8,11 @@ bitflags! { #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ClipboardFileFlags: u32 { /// The fileAttributes field contains valid data. - const ATTRIBUTES = 0x00000004; + const ATTRIBUTES = 0x0000_0004; /// The fileSizeHigh and fileSizeLow fields contain valid data. - const FILE_SIZE = 0x00000040; + const FILE_SIZE = 0x0000_0040; /// The lastWriteTime field contains valid data. - const LAST_WRITE_TIME = 0x00000020; + const LAST_WRITE_TIME = 0x0000_0020; } } @@ -22,21 +22,21 @@ bitflags! { pub struct ClipboardFileAttributes: u32 { /// A file that is read-only. Applications can read the file, but cannot write to /// it or delete it - const READONLY = 0x00000001; + const READONLY = 0x0000_0001; /// The file or directory is hidden. It is not included in an ordinary directory /// listing. - const HIDDEN = 0x00000002; + const HIDDEN = 0x0000_0002; /// A file or directory that the operating system uses a part of, or uses /// exclusively. - const SYSTEM = 0x00000004; + const SYSTEM = 0x0000_0004; /// Identifies a directory. - const DIRECTORY = 0x00000010; + const DIRECTORY = 0x0000_0010; /// A file or directory that is an archive file or directory. Applications typically /// use this attribute to mark files for backup or removal - const ARCHIVE = 0x00000020; + const ARCHIVE = 0x0000_0020; /// A file that does not have other attributes set. This attribute is valid only /// when used alone. - const NORMAL = 0x00000080; + const NORMAL = 0x0000_0080; } } diff --git a/crates/ironrdp-cliprdr/src/pdu/format_data/metafile.rs b/crates/ironrdp-cliprdr/src/pdu/format_data/metafile.rs index 736abdd0a..a0488b71a 100644 --- a/crates/ironrdp-cliprdr/src/pdu/format_data/metafile.rs +++ b/crates/ironrdp-cliprdr/src/pdu/format_data/metafile.rs @@ -10,25 +10,25 @@ bitflags! { pub struct PackedMetafileMappingMode: u32 { /// Each logical unit is mapped to one device pixel. Positive x is to the right; positive /// y is down. - const TEXT = 0x00000001; + const TEXT = 0x0000_0001; /// Each logical unit is mapped to 0.1 millimeter. Positive x is to the right; positive /// y is up. - const LO_METRIC = 0x00000002; + const LO_METRIC = 0x0000_0002; /// Each logical unit is mapped to 0.01 millimeter. Positive x is to the right; positive /// y is up. - const HI_METRIC = 0x00000003; + const HI_METRIC = 0x0000_0003; /// Each logical unit is mapped to 0.01 inch. Positive x is to the right; positive y is up. - const LO_ENGLISH = 0x00000004; + const LO_ENGLISH = 0x0000_0004; /// Each logical unit is mapped to 0.001 inch. Positive x is to the right; positive y is up. - const HI_ENGLISH = 0x00000005; + const HI_ENGLISH = 0x0000_0005; /// Each logical unit is mapped to 1/20 of a printer's point (1/1440 of an inch), also /// called a twip. Positive x is to the right; positive y is up. - const TWIPS = 0x00000006; + const TWIPS = 0x0000_0006; /// Logical units are mapped to arbitrary units with equally scaled axes; one unit along /// the x-axis is equal to one unit along the y-axis. - const ISOTROPIC = 0x00000007; + const ISOTROPIC = 0x0000_0007; /// Logical units are mapped to arbitrary units with arbitrarily scaled axes. - const ANISOTROPIC = 0x00000008; + const ANISOTROPIC = 0x0000_0008; } } diff --git a/crates/ironrdp-cliprdr/src/pdu/format_list.rs b/crates/ironrdp-cliprdr/src/pdu/format_list.rs index 3a4e653d9..0f5837114 100644 --- a/crates/ironrdp-cliprdr/src/pdu/format_list.rs +++ b/crates/ironrdp-cliprdr/src/pdu/format_list.rs @@ -178,6 +178,7 @@ impl ClipboardFormat { /// /// This is typically used for custom/OS-specific formats where a name must be associated to /// the `ClipboardFormatId` in order to distinguish between vendors. + #[must_use] pub fn with_name(self, name: ClipboardFormatName) -> Self { if name.0.is_empty() { return Self { diff --git a/crates/ironrdp-cliprdr/src/pdu/mod.rs b/crates/ironrdp-cliprdr/src/pdu/mod.rs index e668ceaee..6878feb39 100644 --- a/crates/ironrdp-cliprdr/src/pdu/mod.rs +++ b/crates/ironrdp-cliprdr/src/pdu/mod.rs @@ -38,8 +38,8 @@ pub const FORMAT_NAME_FILE_LIST: &str = "FileGroupDescriptorW"; /// Header without message type included struct PartialHeader { - pub message_flags: ClipboardPduFlags, - pub data_length: u32, + pub(crate) message_flags: ClipboardPduFlags, + pub(crate) data_length: u32, } impl PartialHeader { @@ -47,18 +47,18 @@ impl PartialHeader { const FIXED_PART_SIZE: usize = std::mem::size_of::() + std::mem::size_of::(); const SIZE: usize = Self::FIXED_PART_SIZE; - pub fn new(inner_data_length: u32) -> Self { + pub(crate) fn new(inner_data_length: u32) -> Self { Self::new_with_flags(inner_data_length, ClipboardPduFlags::empty()) } - pub fn new_with_flags(data_length: u32, message_flags: ClipboardPduFlags) -> Self { + pub(crate) fn new_with_flags(data_length: u32, message_flags: ClipboardPduFlags) -> Self { Self { message_flags, data_length, } } - pub fn data_length(&self) -> usize { + pub(crate) fn data_length(&self) -> usize { usize::try_from(self.data_length).expect("BUG: Upcasting u32 -> usize should be infallible") } } diff --git a/crates/ironrdp-connector/Cargo.toml b/crates/ironrdp-connector/Cargo.toml index 85e52f8c6..f1e6d28d6 100644 --- a/crates/ironrdp-connector/Cargo.toml +++ b/crates/ironrdp-connector/Cargo.toml @@ -24,6 +24,5 @@ ironrdp-svc.workspace = true ironrdp-error.workspace = true ironrdp-pdu = { workspace = true, features = ["std"] } rand_core = { version = "0.6.4", features = ["std"] } # TODO: dependency injection? -rstest.workspace = true sspi.workspace = true tracing.workspace = true diff --git a/crates/ironrdp-connector/src/channel_connection.rs b/crates/ironrdp-connector/src/channel_connection.rs index 70a3820dd..8c53e9571 100644 --- a/crates/ironrdp-connector/src/channel_connection.rs +++ b/crates/ironrdp-connector/src/channel_connection.rs @@ -180,7 +180,7 @@ impl Sequence for ChannelConnectionSequence { return Err(general_err!("received bad MCS Channel Join Confirm")); } - let next_index = index + 1; + let next_index = index.checked_add(1).unwrap(); let next_state = if next_index == self.channel_ids.len() { ChannelConnectionState::AllJoined { user_channel_id } diff --git a/crates/ironrdp-connector/src/connection.rs b/crates/ironrdp-connector/src/connection.rs index 427d475a8..31c31ca0a 100644 --- a/crates/ironrdp-connector/src/connection.rs +++ b/crates/ironrdp-connector/src/connection.rs @@ -16,10 +16,10 @@ use crate::{ }; #[derive(Clone, Copy, Debug)] -pub struct CredsspTsRequestHint; +struct CredsspTsRequestHint; -pub const DEFAULT_POINTER_CACHE_SIZE: u16 = 32; -pub const CREDSSP_TS_REQUEST_HINT: CredsspTsRequestHint = CredsspTsRequestHint; +const DEFAULT_POINTER_CACHE_SIZE: u16 = 32; +const CREDSSP_TS_REQUEST_HINT: CredsspTsRequestHint = CredsspTsRequestHint; impl PduHint for CredsspTsRequestHint { fn find_size(&self, bytes: &[u8]) -> ironrdp_pdu::PduResult> { @@ -32,9 +32,9 @@ impl PduHint for CredsspTsRequestHint { } #[derive(Clone, Copy, Debug)] -pub struct CredsspEarlyUserAuthResultHint; +struct CredsspEarlyUserAuthResultHint; -pub const CREDSSP_EARLY_USER_AUTH_RESULT_HINT: CredsspEarlyUserAuthResultHint = CredsspEarlyUserAuthResultHint; +const CREDSSP_EARLY_USER_AUTH_RESULT_HINT: CredsspEarlyUserAuthResultHint = CredsspEarlyUserAuthResultHint; impl PduHint for CredsspEarlyUserAuthResultHint { fn find_size(&self, _: &[u8]) -> ironrdp_pdu::PduResult> { @@ -163,7 +163,7 @@ impl State for ClientConnectorState { pub struct ClientConnector { pub config: Config, pub state: ClientConnectorState, - pub server_addr: Option, + pub server_addr: Option, pub server_name: Option, pub network_client_factory: Option>, pub server_public_key: Option>, @@ -184,17 +184,19 @@ impl ClientConnector { } /// Must be set to the actual target server address (as opposed to the proxy) - pub fn with_server_addr(mut self, addr: std::net::SocketAddr) -> Self { + #[must_use] + pub fn with_server_addr(mut self, addr: SocketAddr) -> Self { self.server_addr = Some(addr); self } /// Must be set to the actual target server address (as opposed to the proxy) - pub fn attach_server_addr(&mut self, addr: std::net::SocketAddr) { + pub fn attach_server_addr(&mut self, addr: SocketAddr) { self.server_addr = Some(addr); } /// Must be set to the actual target server hostname (as opposed to the proxy) + #[must_use] pub fn with_server_name(mut self, name: impl Into) -> Self { self.server_name = Some(name.into()); self @@ -205,6 +207,7 @@ impl ClientConnector { self.server_name = Some(name.into()); } + #[must_use] pub fn with_credssp_network_client(mut self, network_client_factory: T) -> Self where T: sspi::network_client::NetworkClientFactory + 'static, @@ -213,19 +216,20 @@ impl ClientConnector { self } - pub fn with_static_channel(mut self, channel: T) -> Self + pub fn attach_credssp_network_client(&mut self, network_client_factory: T) where - T: StaticVirtualChannelProcessor + 'static, + T: sspi::network_client::NetworkClientFactory + 'static, { - self.static_channels.insert(channel); - self + self.network_client_factory = Some(Box::new(network_client_factory)); } - pub fn attach_credssp_network_client(&mut self, network_client_factory: T) + #[must_use] + pub fn with_static_channel(mut self, channel: T) -> Self where - T: sspi::network_client::NetworkClientFactory + 'static, + T: StaticVirtualChannelProcessor + 'static, { - self.network_client_factory = Some(Box::new(network_client_factory)); + self.static_channels.insert(channel); + self } pub fn attach_static_channel(&mut self, channel: T) @@ -793,6 +797,7 @@ impl Sequence for ClientConnector { } } +#[allow(single_use_lifetimes)] // anonymous lifetimes in `impl Trait` are unstable fn create_gcc_blocks<'a>( config: &Config, selected_protocol: nego::SecurityProtocol, @@ -1056,7 +1061,7 @@ fn create_client_confirm_active( } } -fn write_credssp_request(ts_request: credssp::TsRequest, output: &mut WriteBuf) -> crate::ConnectorResult { +fn write_credssp_request(ts_request: credssp::TsRequest, output: &mut WriteBuf) -> ConnectorResult { let length = usize::from(ts_request.buffer_len()); let unfilled_buffer = output.unfilled_to(length); diff --git a/crates/ironrdp-connector/src/legacy.rs b/crates/ironrdp-connector/src/legacy.rs index 4148aa546..50b60311f 100644 --- a/crates/ironrdp-connector/src/legacy.rs +++ b/crates/ironrdp-connector/src/legacy.rs @@ -7,7 +7,7 @@ use ironrdp_pdu::{rdp, x224, PduParsing}; use crate::{ConnectorError, ConnectorErrorExt as _, ConnectorResult}; -pub fn encode_x224_packet(x224_msg: &T, buf: &mut WriteBuf) -> ConnectorResult +pub fn encode_x224_packet(x224_msg: &T, buf: &mut WriteBuf) -> ConnectorResult where T: PduParsing, ConnectorError: From, diff --git a/crates/ironrdp-connector/src/lib.rs b/crates/ironrdp-connector/src/lib.rs index b0dcdad44..7c2196286 100644 --- a/crates/ironrdp-connector/src/lib.rs +++ b/crates/ironrdp-connector/src/lib.rs @@ -100,7 +100,7 @@ impl State for () { true } - fn as_any(&self) -> &dyn core::any::Any { + fn as_any(&self) -> &dyn Any { self } } @@ -116,7 +116,7 @@ impl Written { pub fn from_size(value: usize) -> ConnectorResult { core::num::NonZeroUsize::new(value) .map(Self::Size) - .ok_or(ConnectorError::general("invalid written length (can’t be zero)")) + .ok_or_else(|| ConnectorError::general("invalid written length (can’t be zero)")) } #[inline] @@ -148,7 +148,7 @@ pub trait Sequence: Send + Sync { ironrdp_pdu::assert_obj_safe!(Sequence); -pub type ConnectorResult = std::result::Result; +pub type ConnectorResult = Result; #[non_exhaustive] #[derive(Debug)] @@ -220,7 +220,9 @@ impl ConnectorErrorExt for ConnectorError { } pub trait ConnectorResultExt { + #[must_use] fn with_context(self, context: &'static str) -> Self; + #[must_use] fn with_source(self, source: E) -> Self where E: std::error::Error + Sync + Send + 'static; diff --git a/crates/ironrdp-dvc/src/lib.rs b/crates/ironrdp-dvc/src/lib.rs index f3f3e7e63..233b4656a 100644 --- a/crates/ironrdp-dvc/src/lib.rs +++ b/crates/ironrdp-dvc/src/lib.rs @@ -80,6 +80,7 @@ impl Drdynvc { // FIXME: it’s likely we want to enable adding dynamic channels at any point during the session (message passing? other approach?) + #[must_use] pub fn with_dynamic_channel(mut self, channel: T) -> Self where T: DynamicVirtualChannel + 'static, @@ -250,8 +251,8 @@ impl StaticVirtualChannelProcessor for Drdynvc { } struct DynamicChannelCtx<'a> { - pub dvc_pdu: vc::dvc::ServerPdu, - pub dvc_data: &'a [u8], + dvc_pdu: vc::dvc::ServerPdu, + dvc_data: &'a [u8], } fn decode_dvc_message(user_data: &[u8]) -> PduResult> { diff --git a/crates/ironrdp-error/src/lib.rs b/crates/ironrdp-error/src/lib.rs index 4318874a1..f7a2b5bd1 100644 --- a/crates/ironrdp-error/src/lib.rs +++ b/crates/ironrdp-error/src/lib.rs @@ -16,13 +16,14 @@ pub struct Error { pub context: &'static str, pub kind: Kind, #[cfg(feature = "std")] - source: Option>, + source: Option>, #[cfg(all(not(feature = "std"), feature = "alloc"))] source: Option>, } impl Error { #[cold] + #[must_use] pub fn new(context: &'static str, kind: Kind) -> Self { Self { context, @@ -34,6 +35,7 @@ impl Error { #[cfg(feature = "std")] #[cold] + #[must_use] pub fn with_source(mut self, source: E) -> Self where E: std::error::Error + Sync + Send + 'static, @@ -44,6 +46,7 @@ impl Error { #[cfg(all(not(feature = "std"), feature = "alloc"))] #[cold] + #[must_use] pub fn with_source(mut self, source: E) -> Self where E: fmt::Display + fmt::Debug + Sync + Send + 'static, @@ -117,7 +120,7 @@ where Kind: std::error::Error + Send + Sync + 'static, { fn from(error: Error) -> Self { - std::io::Error::new(std::io::ErrorKind::Other, error) + Self::new(std::io::ErrorKind::Other, error) } } diff --git a/crates/ironrdp-fuzzing/src/lib.rs b/crates/ironrdp-fuzzing/src/lib.rs index 7ce9430df..a52abc938 100644 --- a/crates/ironrdp-fuzzing/src/lib.rs +++ b/crates/ironrdp-fuzzing/src/lib.rs @@ -1,3 +1,10 @@ +// No need to be as strict as in production libraries +#![allow(clippy::arithmetic_side_effects)] +#![allow(clippy::cast_lossless)] +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] + #[macro_use] extern crate arbitrary; diff --git a/crates/ironrdp-fuzzing/src/oracles/mod.rs b/crates/ironrdp-fuzzing/src/oracles/mod.rs index 1077998df..6f9849804 100644 --- a/crates/ironrdp-fuzzing/src/oracles/mod.rs +++ b/crates/ironrdp-fuzzing/src/oracles/mod.rs @@ -67,7 +67,7 @@ pub fn pdu_decode(data: &[u8]) { let _ = codecs::rfx::SyncPdu::from_buffer(data); let _ = codecs::rfx::CodecVersionsPdu::from_buffer(data); let _ = codecs::rfx::ChannelsPdu::from_buffer(data); - let _ = codecs::rfx::Channel::from_buffer(data); + let _ = codecs::rfx::RfxChannel::from_buffer(data); let _ = input::InputEventPdu::from_buffer(data); let _ = input::InputEvent::from_buffer(data); diff --git a/crates/ironrdp-graphics/src/image_processing.rs b/crates/ironrdp-graphics/src/image_processing.rs index a415d9ea7..0fdbafc2e 100644 --- a/crates/ironrdp-graphics/src/image_processing.rs +++ b/crates/ironrdp-graphics/src/image_processing.rs @@ -22,7 +22,7 @@ pub struct ImageRegion<'a> { pub data: &'a [u8], } -impl<'a> ImageRegion<'a> { +impl ImageRegion<'_> { pub fn copy_to(&self, other: &mut ImageRegionMut<'_>) -> io::Result<()> { let width = usize::from(other.region.width()); let height = usize::from(other.region.height()); @@ -256,8 +256,8 @@ impl PixelFormat { } struct Point { - pub x: usize, - pub y: usize, + x: usize, + y: usize, } pub struct Rgba { diff --git a/crates/ironrdp-graphics/src/lib.rs b/crates/ironrdp-graphics/src/lib.rs index c0a1bcaa3..5e28a858e 100644 --- a/crates/ironrdp-graphics/src/lib.rs +++ b/crates/ironrdp-graphics/src/lib.rs @@ -1,3 +1,9 @@ +#![allow(clippy::arithmetic_side_effects)] // FIXME: remove +#![allow(clippy::cast_lossless)] // FIXME: remove +#![allow(clippy::cast_possible_truncation)] // FIXME: remove +#![allow(clippy::cast_possible_wrap)] // FIXME: remove +#![allow(clippy::cast_sign_loss)] // FIXME: remove + pub mod color_conversion; pub mod dwt; pub mod image_processing; diff --git a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs index ac622fc1a..d10ae2914 100644 --- a/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs +++ b/crates/ironrdp-graphics/src/rdp6/bitmap_stream/decoder.rs @@ -42,7 +42,7 @@ struct AYCoCgParams { } impl<'a> BitmapStreamDecoderImpl<'a> { - pub fn init(bitmap: BitmapStreamPdu<'a>, image_width: usize, image_height: usize) -> Self { + fn init(bitmap: BitmapStreamPdu<'a>, image_width: usize, image_height: usize) -> Self { let (chroma_width, chroma_height) = if bitmap.has_subsampled_chroma() { // When image is subsampled, chroma plane has half the size of the luma plane, however // its size is rounded up to the nearest greater integer, to take into account odd image diff --git a/crates/ironrdp-graphics/src/rdp6/rle.rs b/crates/ironrdp-graphics/src/rdp6/rle.rs index a210332b9..ba4f5aceb 100644 --- a/crates/ironrdp-graphics/src/rdp6/rle.rs +++ b/crates/ironrdp-graphics/src/rdp6/rle.rs @@ -47,7 +47,7 @@ struct RlePlaneDecoder { } impl RlePlaneDecoder { - pub fn new(width: usize, height: usize) -> Self { + fn new(width: usize, height: usize) -> Self { Self { last_decoded_byte: 0, width, @@ -132,7 +132,7 @@ impl RlePlaneDecoder { }); } - pub fn decode(mut self, src: &[u8], dst: &mut [u8]) -> Result { + fn decode(mut self, src: &[u8], dst: &mut [u8]) -> Result { let mut read_bytes = 0; read_bytes += self.decode_scanline(src, dst)?; @@ -157,7 +157,12 @@ impl RlePlaneDecoder { /// Size of data written to dst buffer is exactly equal to `width * height`. /// /// Returns number of bytes consumed from src buffer. -pub fn decompress_8bpp_plane(src: &[u8], dst: &mut [u8], width: usize, height: usize) -> Result { +pub(crate) fn decompress_8bpp_plane( + src: &[u8], + dst: &mut [u8], + width: usize, + height: usize, +) -> Result { RlePlaneDecoder::new(width, height).decode(src, dst) } @@ -176,7 +181,7 @@ impl RleEncoderScanlineIterator { } } - fn delta_value(&self, prev: u8, next: u8) -> u8 { + fn delta_value(prev: u8, next: u8) -> u8 { let mut result = (next as i16 - prev as i16) as u8; // bit magic from 3.1.9.2.1 of [MS-RDPEGDI]. @@ -200,7 +205,7 @@ impl> Iterator for RleEncoderScanlineIterator { let prev = std::mem::replace(&mut self.prev_scanline[idx % self.width], next); if idx >= self.width { - next = self.delta_value(prev, next); + next = Self::delta_value(prev, next); } Some(next) @@ -228,15 +233,11 @@ macro_rules! ensure_size { } impl RlePlaneEncoder { - pub fn new(width: usize, height: usize) -> Self { + fn new(width: usize, height: usize) -> Self { Self { width, height } } - pub fn encode( - &self, - mut src: impl Iterator, - dst: &mut WriteCursor<'_>, - ) -> Result { + fn encode(&self, mut src: impl Iterator, dst: &mut WriteCursor<'_>) -> Result { let mut written = 0; for _ in 0..self.height { @@ -355,7 +356,7 @@ impl RlePlaneEncoder { /// Destination slice must have enough space for the compressed data. /// /// Returns number of bytes written to the dst buffer. -pub fn compress_8bpp_plane( +pub(crate) fn compress_8bpp_plane( src: impl Iterator, dst: &mut WriteCursor<'_>, width: usize, diff --git a/crates/ironrdp-graphics/src/rectangle_processing.rs b/crates/ironrdp-graphics/src/rectangle_processing.rs index 01925c92d..c5e3677b3 100644 --- a/crates/ironrdp-graphics/src/rectangle_processing.rs +++ b/crates/ironrdp-graphics/src/rectangle_processing.rs @@ -62,6 +62,7 @@ impl Region { } } + #[must_use] pub fn intersect_rectangle(&self, rectangle: &InclusiveRectangle) -> Self { match self.rectangles.len() { 0 => Self::new(), diff --git a/crates/ironrdp-graphics/src/rle.rs b/crates/ironrdp-graphics/src/rle.rs index 8925850f1..81221a6c6 100644 --- a/crates/ironrdp-graphics/src/rle.rs +++ b/crates/ironrdp-graphics/src/rle.rs @@ -461,7 +461,7 @@ impl Code { } /// Extract the run length of a compression order. - pub fn extract_run_length(self, header: u8, src: &mut Buf) -> Result { + fn extract_run_length(self, header: u8, src: &mut Buf) -> Result { match self { Self::REGULAR_FGBG_IMAGE => extract_run_length_fg_bg(header, MASK_REGULAR_RUN_LENGTH, src), @@ -736,11 +736,11 @@ impl DepthMode for Mode24Bpp { const PIXEL_FORMAT: RlePixelFormat = RlePixelFormat::Rgb24; - const BLACK_PIXEL: Self::Pixel = 0x000000; + const BLACK_PIXEL: Self::Pixel = 0x00_0000; // 8 bits per RGB component: // 1111 1111 1111 1111 1111 1111 (binary) - const WHITE_PIXEL: Self::Pixel = 0xFFFFFF; + const WHITE_PIXEL: Self::Pixel = 0xFF_FFFF; fn write_pixel(dst: &mut BufMut, pixel: Self::Pixel) { dst.write_u24(pixel); diff --git a/crates/ironrdp-graphics/src/utils.rs b/crates/ironrdp-graphics/src/utils.rs index e854808e8..909620fe0 100644 --- a/crates/ironrdp-graphics/src/utils.rs +++ b/crates/ironrdp-graphics/src/utils.rs @@ -2,27 +2,29 @@ use std::ops; use bitvec::prelude::{BitSlice, Msb0}; -pub struct Bits<'a> { +// FIXME: check if this should be deleted in favor of something else + +pub(crate) struct Bits<'a> { bits_slice: &'a BitSlice, remaining_bits_of_last_byte: usize, } impl<'a> Bits<'a> { - pub fn new(bits_slice: &'a BitSlice) -> Self { + pub(crate) fn new(bits_slice: &'a BitSlice) -> Self { Self { bits_slice, remaining_bits_of_last_byte: 0, } } - pub fn split_to(&mut self, at: usize) -> &'a BitSlice { + pub(crate) fn split_to(&mut self, at: usize) -> &'a BitSlice { let (value, new_bits) = self.bits_slice.split_at(at); self.bits_slice = new_bits; self.remaining_bits_of_last_byte = (self.remaining_bits_of_last_byte + at) % 8; value } - pub fn remaining_bits_of_last_byte(&self) -> usize { + pub(crate) fn remaining_bits_of_last_byte(&self) -> usize { self.remaining_bits_of_last_byte } } diff --git a/crates/ironrdp-graphics/src/zgfx/circular_buffer.rs b/crates/ironrdp-graphics/src/zgfx/circular_buffer.rs index b68d2fbf9..2f8156214 100644 --- a/crates/ironrdp-graphics/src/zgfx/circular_buffer.rs +++ b/crates/ironrdp-graphics/src/zgfx/circular_buffer.rs @@ -1,20 +1,20 @@ use std::cmp::min; use std::io; -pub struct FixedCircularBuffer { +pub(crate) struct FixedCircularBuffer { buffer: Vec, position: usize, } impl FixedCircularBuffer { - pub fn new(size: usize) -> Self { + pub(crate) fn new(size: usize) -> Self { Self { buffer: vec![0; size], position: 0, } } - pub fn read_with_offset(&self, offset: usize, length: usize, mut output: impl io::Write) -> io::Result<()> { + pub(crate) fn read_with_offset(&self, offset: usize, length: usize, mut output: impl io::Write) -> io::Result<()> { let position = (self.buffer.len() + self.position - offset) % self.buffer.len(); // will take the offset if the destination length is greater than the offset, diff --git a/crates/ironrdp-graphics/src/zgfx/control_messages.rs b/crates/ironrdp-graphics/src/zgfx/control_messages.rs index c2d5428a0..affe93468 100644 --- a/crates/ironrdp-graphics/src/zgfx/control_messages.rs +++ b/crates/ironrdp-graphics/src/zgfx/control_messages.rs @@ -7,7 +7,7 @@ use num_traits::FromPrimitive as _; use super::ZgfxError; #[derive(Debug, Clone, PartialEq, Eq)] -pub enum SegmentedDataPdu<'a> { +pub(crate) enum SegmentedDataPdu<'a> { Single(BulkEncodedData<'a>), Multipart { uncompressed_size: usize, @@ -16,7 +16,7 @@ pub enum SegmentedDataPdu<'a> { } impl<'a> SegmentedDataPdu<'a> { - pub fn from_buffer(mut buffer: &'a [u8]) -> Result { + pub(crate) fn from_buffer(mut buffer: &'a [u8]) -> Result { let descriptor = SegmentedDescriptor::from_u8(buffer.read_u8()?).ok_or(ZgfxError::InvalidSegmentedDescriptor)?; @@ -45,13 +45,13 @@ impl<'a> SegmentedDataPdu<'a> { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct BulkEncodedData<'a> { - pub compression_flags: CompressionFlags, - pub data: &'a [u8], +pub(crate) struct BulkEncodedData<'a> { + pub(crate) compression_flags: CompressionFlags, + pub(crate) data: &'a [u8], } impl<'a> BulkEncodedData<'a> { - pub fn from_buffer(mut buffer: &'a [u8]) -> Result { + pub(crate) fn from_buffer(mut buffer: &'a [u8]) -> Result { let compression_type_and_flags = buffer.read_u8()?; let _compression_type = CompressionType::from_u8(compression_type_and_flags.get_bits(..4)) .ok_or(ZgfxError::InvalidCompressionType)?; diff --git a/crates/ironrdp-graphics/src/zgfx/mod.rs b/crates/ironrdp-graphics/src/zgfx/mod.rs index 2205d3410..5b507e918 100644 --- a/crates/ironrdp-graphics/src/zgfx/mod.rs +++ b/crates/ironrdp-graphics/src/zgfx/mod.rs @@ -204,8 +204,8 @@ fn read_encoded_bytes( } struct Token { - pub prefix: &'static BitSlice, - pub ty: TokenType, + prefix: &'static BitSlice, + ty: TokenType, } enum TokenType { diff --git a/crates/ironrdp-input/src/lib.rs b/crates/ironrdp-input/src/lib.rs index dda534aca..4d2198ad5 100644 --- a/crates/ironrdp-input/src/lib.rs +++ b/crates/ironrdp-input/src/lib.rs @@ -74,13 +74,16 @@ impl Scancode { pub const fn from_u16(scancode: u16) -> Self { let extended = scancode & 0xE000 == 0xE000; + + #[allow(clippy::cast_possible_truncation)] // truncating on purpose let code = scancode as u8; + Self { code, extended } } pub fn as_idx(self) -> usize { if self.extended { - usize::from(self.code) + 256 + usize::from(self.code).checked_add(256).expect("never overflow") } else { usize::from(self.code) } @@ -322,7 +325,8 @@ impl Database { for idx in self.keyboard.iter_ones() { let (scancode, extended) = if idx >= 256 { - (u8::try_from(idx - 256).unwrap(), true) + let extended_code = idx.checked_sub(256).expect("never underflow"); + (u8::try_from(extended_code).unwrap(), true) } else { (u8::try_from(idx).unwrap(), false) }; diff --git a/crates/ironrdp-pdu-generators/Cargo.toml b/crates/ironrdp-pdu-generators/Cargo.toml index e815a1d2c..714331c08 100644 --- a/crates/ironrdp-pdu-generators/Cargo.toml +++ b/crates/ironrdp-pdu-generators/Cargo.toml @@ -10,5 +10,5 @@ doctest = false test = false [dependencies] -ironrdp-pdu.workspace = true -proptest.workspace = true +# ironrdp-pdu.workspace = true +# proptest.workspace = true diff --git a/crates/ironrdp-pdu-generators/src/lib.rs b/crates/ironrdp-pdu-generators/src/lib.rs index 8b1378917..d4bd0bf76 100644 --- a/crates/ironrdp-pdu-generators/src/lib.rs +++ b/crates/ironrdp-pdu-generators/src/lib.rs @@ -1 +1,6 @@ - +// No need to be as strict as in production libraries +#![allow(clippy::arithmetic_side_effects)] +#![allow(clippy::cast_lossless)] +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] diff --git a/crates/ironrdp-pdu/src/basic_output/bitmap.rs b/crates/ironrdp-pdu/src/basic_output/bitmap.rs index c5fe58ea7..6ee7a399b 100644 --- a/crates/ironrdp-pdu/src/basic_output/bitmap.rs +++ b/crates/ironrdp-pdu/src/basic_output/bitmap.rs @@ -35,7 +35,7 @@ impl BitmapUpdateData<'_> { } } -impl<'en> PduEncode for BitmapUpdateData<'en> { +impl PduEncode for BitmapUpdateData<'_> { fn encode(&self, dst: &mut crate::cursor::WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -95,7 +95,7 @@ pub struct BitmapData<'a> { pub bitmap_data: &'a [u8], } -impl<'a> BitmapData<'a> { +impl BitmapData<'_> { const NAME: &str = "TS_BITMAP_DATA"; const FIXED_PART_SIZE: usize = InclusiveRectangle::ENCODED_SIZE + core::mem::size_of::() * 5; @@ -104,7 +104,7 @@ impl<'a> BitmapData<'a> { } } -impl<'en> PduEncode for BitmapData<'en> { +impl PduEncode for BitmapData<'_> { fn encode(&self, dst: &mut crate::cursor::WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); diff --git a/crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs b/crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs index 623c27120..bf50cc896 100644 --- a/crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs +++ b/crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs @@ -137,7 +137,7 @@ impl<'a> PduDecode<'a> for BitmapStream<'a> { } } -impl<'a> PduEncode for BitmapStream<'a> { +impl PduEncode for BitmapStream<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); diff --git a/crates/ironrdp-pdu/src/basic_output/fast_path.rs b/crates/ironrdp-pdu/src/basic_output/fast_path.rs index 23653e723..1d2577838 100644 --- a/crates/ironrdp-pdu/src/basic_output/fast_path.rs +++ b/crates/ironrdp-pdu/src/basic_output/fast_path.rs @@ -120,7 +120,7 @@ impl FastPathUpdatePdu<'_> { const FIXED_PART_SIZE: usize = std::mem::size_of::(); } -impl<'en> PduEncode for FastPathUpdatePdu<'en> { +impl PduEncode for FastPathUpdatePdu<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -242,7 +242,7 @@ impl<'a> FastPathUpdate<'a> { UpdateCode::DefaultPointer => Ok(Self::Pointer(PointerUpdateData::SetDefault)), UpdateCode::PositionPointer => Ok(Self::Pointer(PointerUpdateData::SetPosition(decode_cursor(src)?))), UpdateCode::ColorPointer => { - let color = crate::decode_cursor(src)?; + let color = decode_cursor(src)?; Ok(Self::Pointer(PointerUpdateData::Color(color))) } UpdateCode::CachedPointer => Ok(Self::Pointer(PointerUpdateData::Cached(decode_cursor(src)?))), @@ -261,7 +261,7 @@ impl<'a> FastPathUpdate<'a> { } } -impl<'en> PduEncode for FastPathUpdate<'en> { +impl PduEncode for FastPathUpdate<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -325,7 +325,7 @@ pub enum UpdateCode { LargePointer = 0xc, } -impl<'a> From<&FastPathUpdate<'a>> for UpdateCode { +impl From<&FastPathUpdate<'_>> for UpdateCode { fn from(update: &FastPathUpdate<'_>) -> Self { match update { FastPathUpdate::SurfaceCommands(_) => Self::SurfaceCommands, diff --git a/crates/ironrdp-pdu/src/basic_output/pointer.rs b/crates/ironrdp-pdu/src/basic_output/pointer.rs index a64a6a4b4..d7130c907 100644 --- a/crates/ironrdp-pdu/src/basic_output/pointer.rs +++ b/crates/ironrdp-pdu/src/basic_output/pointer.rs @@ -57,7 +57,7 @@ pub struct ColorPointerAttribute<'a> { pub and_mask: &'a [u8], } -impl<'a> ColorPointerAttribute<'a> { +impl ColorPointerAttribute<'_> { const NAME: &'static str = "TS_COLORPOINTERATTRIBUTE"; const FIXED_PART_SIZE: usize = core::mem::size_of::() * 5 + Point16::FIXED_PART_SIZE; @@ -89,7 +89,7 @@ impl<'a> ColorPointerAttribute<'a> { } } -impl<'a> PduEncode for ColorPointerAttribute<'a> { +impl PduEncode for ColorPointerAttribute<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -156,12 +156,12 @@ pub struct PointerAttribute<'a> { pub color_pointer: ColorPointerAttribute<'a>, } -impl<'a> PointerAttribute<'a> { +impl PointerAttribute<'_> { const NAME: &'static str = "TS_POINTERATTRIBUTE"; const FIXED_PART_SIZE: usize = core::mem::size_of::(); } -impl<'a> PduEncode for PointerAttribute<'a> { +impl PduEncode for PointerAttribute<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -242,13 +242,13 @@ pub struct LargePointerAttribute<'a> { pub and_mask: &'a [u8], } -impl<'a> LargePointerAttribute<'a> { +impl LargePointerAttribute<'_> { const NAME: &'static str = "TS_FP_LARGEPOINTERATTRIBUTE"; const FIXED_PART_SIZE: usize = core::mem::size_of::() * 2 + core::mem::size_of::() * 4 + core::mem::size_of::(); } -impl<'a> PduEncode for LargePointerAttribute<'a> { +impl PduEncode for LargePointerAttribute<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); diff --git a/crates/ironrdp-pdu/src/basic_output/surface_commands.rs b/crates/ironrdp-pdu/src/basic_output/surface_commands.rs index 0d00de6a2..ea89bbf80 100644 --- a/crates/ironrdp-pdu/src/basic_output/surface_commands.rs +++ b/crates/ironrdp-pdu/src/basic_output/surface_commands.rs @@ -24,7 +24,7 @@ impl SurfaceCommand<'_> { const FIXED_PART_SIZE: usize = std::mem::size_of::(); } -impl<'en> PduEncode for SurfaceCommand<'en> { +impl PduEncode for SurfaceCommand<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -79,7 +79,7 @@ impl SurfaceBitsPdu<'_> { const NAME: &str = "TS_SURFCMD_x_SURFACE_BITS_PDU"; } -impl<'en> PduEncode for SurfaceBitsPdu<'en> { +impl PduEncode for SurfaceBitsPdu<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { self.destination.encode(dst)?; self.extended_bitmap_data.encode(dst)?; @@ -179,7 +179,7 @@ impl ExtendedBitmapDataPdu<'_> { core::mem::size_of::() * 4 + core::mem::size_of::() * 2 + core::mem::size_of::(); } -impl<'en> PduEncode for ExtendedBitmapDataPdu<'en> { +impl PduEncode for ExtendedBitmapDataPdu<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { ensure_size!(in: dst, size: self.size()); @@ -292,8 +292,8 @@ impl PduEncode for BitmapDataHeader { } } -impl<'de> PduDecode<'de> for BitmapDataHeader { - fn decode(src: &mut ReadCursor<'de>) -> PduResult { +impl PduDecode<'_> for BitmapDataHeader { + fn decode(src: &mut ReadCursor<'_>) -> PduResult { ensure_fixed_part_size!(in: src); let high_unique_id = src.read_u32(); @@ -318,7 +318,7 @@ enum SurfaceCommandType { StreamSurfaceBits = 0x06, } -impl<'a> From<&SurfaceCommand<'a>> for SurfaceCommandType { +impl From<&SurfaceCommand<'_>> for SurfaceCommandType { fn from(command: &SurfaceCommand<'_>) -> Self { match command { SurfaceCommand::SetSurfaceBits(_) => Self::SetSurfaceBits, diff --git a/crates/ironrdp-pdu/src/ber.rs b/crates/ironrdp-pdu/src/ber.rs index 45fe62809..d7a2b88fa 100644 --- a/crates/ironrdp-pdu/src/ber.rs +++ b/crates/ironrdp-pdu/src/ber.rs @@ -4,7 +4,7 @@ use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; #[repr(u8)] #[allow(unused)] -pub enum Pc { +pub(crate) enum Pc { Primitive = 0x00, Construct = 0x20, } @@ -31,26 +31,26 @@ enum Tag { Sequence = 0x10, } -pub const SIZEOF_ENUMERATED: u16 = 3; -pub const SIZEOF_BOOL: u16 = 3; +pub(crate) const SIZEOF_ENUMERATED: u16 = 3; +pub(crate) const SIZEOF_BOOL: u16 = 3; const TAG_MASK: u8 = 0x1F; -pub fn sizeof_application_tag(tagnum: u8, length: u16) -> u16 { +pub(crate) fn sizeof_application_tag(tagnum: u8, length: u16) -> u16 { let tag_len = if tagnum > 0x1E { 2 } else { 1 }; sizeof_length(length) + tag_len } -pub fn sizeof_sequence_tag(length: u16) -> u16 { +pub(crate) fn sizeof_sequence_tag(length: u16) -> u16 { 1 + sizeof_length(length) } -pub fn sizeof_octet_string(length: u16) -> u16 { +pub(crate) fn sizeof_octet_string(length: u16) -> u16 { 1 + sizeof_length(length) + length } -pub fn sizeof_integer(value: u32) -> u16 { +pub(crate) fn sizeof_integer(value: u32) -> u16 { if value < 0x0000_0080 { 3 } else if value < 0x0000_8000 { @@ -62,12 +62,12 @@ pub fn sizeof_integer(value: u32) -> u16 { } } -pub fn write_sequence_tag(mut stream: impl io::Write, length: u16) -> io::Result { +pub(crate) fn write_sequence_tag(mut stream: impl io::Write, length: u16) -> io::Result { write_universal_tag(&mut stream, Tag::Sequence, Pc::Construct)?; write_length(stream, length).map(|length| length + 1) } -pub fn read_sequence_tag(mut stream: impl io::Read) -> io::Result { +pub(crate) fn read_sequence_tag(mut stream: impl io::Read) -> io::Result { let identifier = stream.read_u8()?; if identifier != Class::Universal as u8 | Pc::Construct as u8 | (TAG_MASK & Tag::Sequence as u8) { @@ -80,7 +80,7 @@ pub fn read_sequence_tag(mut stream: impl io::Read) -> io::Result { } } -pub fn write_application_tag(mut stream: impl io::Write, tagnum: u8, length: u16) -> io::Result { +pub(crate) fn write_application_tag(mut stream: impl io::Write, tagnum: u8, length: u16) -> io::Result { let taglen = if tagnum > 0x1E { stream.write_u8(Class::Application as u8 | Pc::Construct as u8 | TAG_MASK)?; stream.write_u8(tagnum)?; @@ -93,7 +93,7 @@ pub fn write_application_tag(mut stream: impl io::Write, tagnum: u8, length: u16 write_length(stream, length).map(|length| length + taglen) } -pub fn read_application_tag(mut stream: impl io::Read, tagnum: u8) -> io::Result { +pub(crate) fn read_application_tag(mut stream: impl io::Read, tagnum: u8) -> io::Result { let identifier = stream.read_u8()?; if tagnum > 0x1E { @@ -119,7 +119,7 @@ pub fn read_application_tag(mut stream: impl io::Read, tagnum: u8) -> io::Result read_length(stream) } -pub fn write_enumerated(mut stream: impl io::Write, enumerated: u8) -> io::Result { +pub(crate) fn write_enumerated(mut stream: impl io::Write, enumerated: u8) -> io::Result { let mut size = 0; size += write_universal_tag(&mut stream, Tag::Enumerated, Pc::Primitive)?; size += write_length(&mut stream, 1)?; @@ -129,7 +129,7 @@ pub fn write_enumerated(mut stream: impl io::Write, enumerated: u8) -> io::Resul Ok(size) } -pub fn read_enumerated(mut stream: impl io::Read, count: u8) -> io::Result { +pub(crate) fn read_enumerated(mut stream: impl io::Read, count: u8) -> io::Result { read_universal_tag(&mut stream, Tag::Enumerated, Pc::Primitive)?; let length = read_length(&mut stream)?; @@ -145,7 +145,7 @@ pub fn read_enumerated(mut stream: impl io::Read, count: u8) -> io::Result { Ok(enumerated) } -pub fn write_integer(mut stream: impl io::Write, value: u32) -> io::Result { +pub(crate) fn write_integer(mut stream: impl io::Write, value: u32) -> io::Result { write_universal_tag(&mut stream, Tag::Integer, Pc::Primitive)?; if value < 0x0000_0080 { @@ -172,7 +172,7 @@ pub fn write_integer(mut stream: impl io::Write, value: u32) -> io::Result io::Result { +pub(crate) fn read_integer(mut stream: impl io::Read) -> io::Result { read_universal_tag(&mut stream, Tag::Integer, Pc::Primitive)?; let length = read_length(&mut stream)?; @@ -194,7 +194,7 @@ pub fn read_integer(mut stream: impl io::Read) -> io::Result { } } -pub fn write_bool(mut stream: impl io::Write, value: bool) -> io::Result { +pub(crate) fn write_bool(mut stream: impl io::Write, value: bool) -> io::Result { let mut size = 0; size += write_universal_tag(&mut stream, Tag::Boolean, Pc::Primitive)?; size += write_length(&mut stream, 1)?; @@ -204,7 +204,7 @@ pub fn write_bool(mut stream: impl io::Write, value: bool) -> io::Result Ok(size) } -pub fn read_bool(mut stream: impl io::Read) -> io::Result { +pub(crate) fn read_bool(mut stream: impl io::Read) -> io::Result { read_universal_tag(&mut stream, Tag::Boolean, Pc::Primitive)?; let length = read_length(&mut stream)?; @@ -215,18 +215,18 @@ pub fn read_bool(mut stream: impl io::Read) -> io::Result { Ok(stream.read_u8()? != 0) } -pub fn write_octet_string(mut stream: impl io::Write, value: &[u8]) -> io::Result { +pub(crate) fn write_octet_string(mut stream: impl io::Write, value: &[u8]) -> io::Result { let tag_size = write_octet_string_tag(&mut stream, value.len() as u16)?; stream.write_all(value)?; Ok(tag_size + value.len()) } -pub fn write_octet_string_tag(mut stream: impl io::Write, length: u16) -> io::Result { +pub(crate) fn write_octet_string_tag(mut stream: impl io::Write, length: u16) -> io::Result { write_universal_tag(&mut stream, Tag::OctetString, Pc::Primitive)?; write_length(&mut stream, length).map(|length| length + 1) } -pub fn read_octet_string(mut stream: impl io::Read) -> io::Result> { +pub(crate) fn read_octet_string(mut stream: impl io::Read) -> io::Result> { let length = read_octet_string_tag(&mut stream)?; let mut buffer = vec![0; length as usize]; @@ -235,7 +235,7 @@ pub fn read_octet_string(mut stream: impl io::Read) -> io::Result> { Ok(buffer) } -pub fn read_octet_string_tag(mut stream: impl io::Read) -> io::Result { +pub(crate) fn read_octet_string_tag(mut stream: impl io::Read) -> io::Result { read_universal_tag(&mut stream, Tag::OctetString, Pc::Primitive)?; read_length(stream) } diff --git a/crates/ironrdp-pdu/src/codecs/rfx.rs b/crates/ironrdp-pdu/src/codecs/rfx.rs index 657a9a337..f9455d087 100644 --- a/crates/ironrdp-pdu/src/codecs/rfx.rs +++ b/crates/ironrdp-pdu/src/codecs/rfx.rs @@ -12,7 +12,7 @@ pub use self::data_messages::{ ContextPdu, EntropyAlgorithm, FrameBeginPdu, FrameEndPdu, OperatingMode, Quant, RegionPdu, RfxRectangle, Tile, TileSetPdu, }; -pub use self::header_messages::{Channel, ChannelsPdu, CodecVersionsPdu, SyncPdu}; +pub use self::header_messages::{ChannelsPdu, CodecVersionsPdu, RfxChannel, SyncPdu}; use crate::{PduBufferParsing, PduParsing}; const BLOCK_HEADER_SIZE: usize = 6; @@ -133,6 +133,7 @@ impl CodecChannelHeader { Ok(Self) } + #[allow(clippy::unused_self)] // for symmetry fn to_buffer_consume_with_type(&self, buffer: &mut &mut [u8], ty: BlockType) -> Result<(), RfxError> { buffer.write_u8(CODEC_ID)?; @@ -249,6 +250,10 @@ pub enum RfxError { InvalidSubtype(u16), #[error("Got invalid IT flag of TileSet: {0}")] InvalidItFlag(bool), + #[error("Got invalid channel width: {0}")] + InvalidChannelWidth(i16), + #[error("Got invalid channel height: {0}")] + InvalidChannelHeight(i16), } #[cfg(feature = "std")] diff --git a/crates/ironrdp-pdu/src/codecs/rfx/data_messages.rs b/crates/ironrdp-pdu/src/codecs/rfx/data_messages.rs index 8841e8fbe..02d0f9900 100644 --- a/crates/ironrdp-pdu/src/codecs/rfx/data_messages.rs +++ b/crates/ironrdp-pdu/src/codecs/rfx/data_messages.rs @@ -75,7 +75,7 @@ impl ContextPdu { } } -impl<'a> PduBufferParsing<'a> for ContextPdu { +impl PduBufferParsing<'_> for ContextPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -120,7 +120,7 @@ pub struct FrameBeginPdu { pub number_of_regions: i16, } -impl<'a> PduBufferParsing<'a> for FrameBeginPdu { +impl PduBufferParsing<'_> for FrameBeginPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -161,7 +161,7 @@ impl<'a> PduBufferParsing<'a> for FrameBeginPdu { #[derive(Debug, Clone, PartialEq, Eq)] pub struct FrameEndPdu; -impl<'a> PduBufferParsing<'a> for FrameEndPdu { +impl PduBufferParsing<'_> for FrameEndPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -194,7 +194,7 @@ pub struct RegionPdu { pub rectangles: Vec, } -impl<'a> PduBufferParsing<'a> for RegionPdu { +impl PduBufferParsing<'_> for RegionPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -415,7 +415,7 @@ pub struct RfxRectangle { pub height: u16, } -impl<'a> PduBufferParsing<'a> for RfxRectangle { +impl PduBufferParsing<'_> for RfxRectangle { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -455,7 +455,7 @@ pub struct Quant { pub hh1: u8, } -impl<'a> PduBufferParsing<'a> for Quant { +impl PduBufferParsing<'_> for Quant { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { diff --git a/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs b/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs index 430095688..b91bfd589 100644 --- a/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs +++ b/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs @@ -16,7 +16,7 @@ const CHANNEL_SIZE: usize = 5; #[derive(Debug, Clone, PartialEq, Eq)] pub struct SyncPdu; -impl<'a> PduBufferParsing<'a> for SyncPdu { +impl PduBufferParsing<'_> for SyncPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -71,7 +71,7 @@ impl CodecVersionsPdu { } } -impl<'a> PduBufferParsing<'a> for CodecVersionsPdu { +impl PduBufferParsing<'_> for CodecVersionsPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -98,7 +98,7 @@ impl<'a> PduBufferParsing<'a> for CodecVersionsPdu { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ChannelsPdu(pub Vec); +pub struct ChannelsPdu(pub Vec); impl ChannelsPdu { pub fn from_buffer_consume_with_header(buffer: &mut &[u8], header: BlockHeader) -> Result { @@ -114,14 +114,14 @@ impl ChannelsPdu { } let channels = (0..channels_number) - .map(|_| Channel::from_buffer_consume(&mut buffer)) + .map(|_| RfxChannel::from_buffer_consume(&mut buffer)) .collect::, _>>()?; Ok(Self(channels)) } } -impl<'a> PduBufferParsing<'a> for ChannelsPdu { +impl PduBufferParsing<'_> for ChannelsPdu { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -151,13 +151,44 @@ impl<'a> PduBufferParsing<'a> for ChannelsPdu { } } +/// TS_RFX_CHANNELT #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct Channel { - pub width: i16, - pub height: i16, +pub struct RfxChannel { + pub width: RfxChannelWidth, + pub height: RfxChannelHeight, } -impl<'a> PduBufferParsing<'a> for Channel { +/// A 16-bit, signed integer within the range of 1 to 4096 +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[repr(transparent)] +pub struct RfxChannelWidth(i16); + +impl RfxChannelWidth { + pub fn as_u16(self) -> u16 { + u16::try_from(self.0).expect("integer within the range of 1 to 4096") + } + + pub fn get(self) -> i16 { + self.0 + } +} + +/// A 16-bit, signed integer within the range of 1 to 2048 +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[repr(transparent)] +pub struct RfxChannelHeight(i16); + +impl RfxChannelHeight { + pub fn as_u16(self) -> u16 { + u16::try_from(self.0).expect("integer within the range of 1 to 2048") + } + + pub fn get(self) -> i16 { + self.0 + } +} + +impl PduBufferParsing<'_> for RfxChannel { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { @@ -167,15 +198,24 @@ impl<'a> PduBufferParsing<'a> for Channel { } let width = buffer.read_i16::()?; + if width < 1 && width > 4096 { + return Err(RfxError::InvalidChannelWidth(width)); + } + let width = RfxChannelWidth(width); + let height = buffer.read_i16::()?; + if height < 1 && height > 2048 { + return Err(RfxError::InvalidChannelHeight(height)); + } + let height = RfxChannelHeight(height); Ok(Self { width, height }) } fn to_buffer_consume(&self, buffer: &mut &mut [u8]) -> Result<(), Self::Error> { buffer.write_u8(CHANNEL_ID)?; - buffer.write_i16::(self.width)?; - buffer.write_i16::(self.height)?; + buffer.write_i16::(self.width.get())?; + buffer.write_i16::(self.height.get())?; Ok(()) } @@ -188,7 +228,7 @@ impl<'a> PduBufferParsing<'a> for Channel { #[derive(Debug, Clone, PartialEq)] struct CodecVersion; -impl<'a> PduBufferParsing<'a> for CodecVersion { +impl PduBufferParsing<'_> for CodecVersion { type Error = RfxError; fn from_buffer_consume(buffer: &mut &[u8]) -> Result { diff --git a/crates/ironrdp-pdu/src/crypto/rc4.rs b/crates/ironrdp-pdu/src/crypto/rc4.rs index b4c0dcb7b..90589f46b 100644 --- a/crates/ironrdp-pdu/src/crypto/rc4.rs +++ b/crates/ironrdp-pdu/src/crypto/rc4.rs @@ -1,14 +1,14 @@ use std::{fmt, ops}; #[derive(Debug, Clone)] -pub struct Rc4 { +pub(crate) struct Rc4 { i: usize, j: usize, state: State, } impl Rc4 { - pub fn new(key: &[u8]) -> Self { + pub(crate) fn new(key: &[u8]) -> Self { // key scheduling let mut state = State::default(); for (i, item) in state.iter_mut().enumerate().take(256) { @@ -23,7 +23,7 @@ impl Rc4 { Self { i: 0, j: 0, state } } - pub fn process(&mut self, message: &[u8]) -> Vec { + pub(crate) fn process(&mut self, message: &[u8]) -> Vec { // PRGA let mut output = Vec::with_capacity(message.len()); while output.capacity() > output.len() { diff --git a/crates/ironrdp-pdu/src/crypto/rsa.rs b/crates/ironrdp-pdu/src/crypto/rsa.rs index b6afaa5b8..d16e16321 100644 --- a/crates/ironrdp-pdu/src/crypto/rsa.rs +++ b/crates/ironrdp-pdu/src/crypto/rsa.rs @@ -3,7 +3,7 @@ use std::io; use der_parser::parse_der; use num_bigint::BigUint; -pub fn encrypt_with_public_key(message: &[u8], public_key_der: &[u8]) -> io::Result> { +pub(crate) fn encrypt_with_public_key(message: &[u8], public_key_der: &[u8]) -> io::Result> { let (_, der_object) = parse_der(public_key_der).map_err(|err| { io::Error::new( io::ErrorKind::InvalidData, diff --git a/crates/ironrdp-pdu/src/cursor.rs b/crates/ironrdp-pdu/src/cursor.rs index d59ed097d..ae77a85ce 100644 --- a/crates/ironrdp-pdu/src/cursor.rs +++ b/crates/ironrdp-pdu/src/cursor.rs @@ -185,6 +185,7 @@ impl<'a> ReadCursor<'a> { self.pos += len; } + #[must_use] pub const fn advanced(&'a self, len: usize) -> ReadCursor<'a> { ReadCursor { inner: self.inner, @@ -196,6 +197,7 @@ impl<'a> ReadCursor<'a> { self.pos -= len; } + #[must_use] pub const fn rewinded(&'a self, len: usize) -> ReadCursor<'a> { ReadCursor { inner: self.inner, @@ -300,6 +302,7 @@ impl<'a> WriteCursor<'a> { self.pos += len; } + #[must_use] pub fn advanced(&'a mut self, len: usize) -> WriteCursor<'a> { WriteCursor { inner: self.inner, @@ -311,6 +314,7 @@ impl<'a> WriteCursor<'a> { self.pos -= len; } + #[must_use] pub fn rewinded(&'a mut self, len: usize) -> WriteCursor<'a> { WriteCursor { inner: self.inner, diff --git a/crates/ironrdp-pdu/src/gcc.rs b/crates/ironrdp-pdu/src/gcc.rs index 57c4647b0..7c0ba7d83 100644 --- a/crates/ironrdp-pdu/src/gcc.rs +++ b/crates/ironrdp-pdu/src/gcc.rs @@ -74,7 +74,7 @@ pub struct ClientGccBlocks { } impl ClientGccBlocks { - pub fn channel_names(&self) -> Option> { + pub fn channel_names(&self) -> Option> { self.network.as_ref().map(|network| network.channels.clone()) } } @@ -321,7 +321,7 @@ pub struct UserDataHeader { impl UserDataHeader { fn from_gcc_block(block_type: T, gcc_block: &B) -> Result where - GccError: std::convert::From<::Error>, + GccError: From<::Error>, { let mut block_data = Vec::with_capacity(gcc_block.buffer_length()); gcc_block.to_buffer(&mut block_data)?; diff --git a/crates/ironrdp-pdu/src/gcc/core_data.rs b/crates/ironrdp-pdu/src/gcc/core_data.rs index 8392fc7e9..dc6cafd07 100644 --- a/crates/ironrdp-pdu/src/gcc/core_data.rs +++ b/crates/ironrdp-pdu/src/gcc/core_data.rs @@ -1,5 +1,5 @@ -pub mod client; -pub mod server; +pub(crate) mod client; +pub(crate) mod server; use std::io; diff --git a/crates/ironrdp-pdu/src/geometry.rs b/crates/ironrdp-pdu/src/geometry.rs index c29feee5f..8aefbc491 100644 --- a/crates/ironrdp-pdu/src/geometry.rs +++ b/crates/ironrdp-pdu/src/geometry.rs @@ -70,6 +70,7 @@ pub trait Rectangle: RectangleImpl { } } + #[must_use] fn union(&self, other: &Self) -> Self { let a = self.to_base(); let b = other.to_base(); diff --git a/crates/ironrdp-pdu/src/lib.rs b/crates/ironrdp-pdu/src/lib.rs index 4e3e28fe5..1fa4b17d9 100644 --- a/crates/ironrdp-pdu/src/lib.rs +++ b/crates/ironrdp-pdu/src/lib.rs @@ -1,3 +1,9 @@ +#![allow(clippy::arithmetic_side_effects)] // FIXME: remove +#![allow(clippy::cast_lossless)] // FIXME: remove +#![allow(clippy::cast_possible_truncation)] // FIXME: remove +#![allow(clippy::cast_possible_wrap)] // FIXME: remove +#![allow(clippy::cast_sign_loss)] // FIXME: remove + use core::fmt; use cursor::WriteCursor; @@ -34,7 +40,7 @@ pub(crate) mod per; pub use crate::basic_output::{bitmap, fast_path, pointer, surface_commands}; pub use crate::rdp::vc::dvc; -pub type PduResult = core::result::Result; +pub type PduResult = Result; pub type PduError = ironrdp_error::Error; @@ -249,7 +255,7 @@ pub enum Action { } impl Action { - pub fn from_fp_output_header(fp_output_header: u8) -> core::result::Result { + pub fn from_fp_output_header(fp_output_header: u8) -> Result { match fp_output_header & 0b11 { 0x00 => Ok(Self::FastPath), 0x03 => Ok(Self::X224), diff --git a/crates/ironrdp-pdu/src/mcs.rs b/crates/ironrdp-pdu/src/mcs.rs index 9516fb570..3f98d13b7 100644 --- a/crates/ironrdp-pdu/src/mcs.rs +++ b/crates/ironrdp-pdu/src/mcs.rs @@ -239,21 +239,15 @@ impl DomainMcsPdu { fn read_mcspdu_header(src: &mut ReadCursor<'_>, ctx: &'static str) -> PduResult { let choice = src.try_read_u8(ctx)?; - DomainMcsPdu::from_choice(choice).ok_or(PduError::invalid_message( - ctx, - "domain-mcspdu", - "unexpected application tag for CHOICE", - )) + DomainMcsPdu::from_choice(choice) + .ok_or_else(|| PduError::invalid_message(ctx, "domain-mcspdu", "unexpected application tag for CHOICE")) } fn peek_mcspdu_header(src: &mut ReadCursor<'_>, ctx: &'static str) -> PduResult { let choice = src.try_peek_u8(ctx)?; - DomainMcsPdu::from_choice(choice).ok_or(PduError::invalid_message( - ctx, - "domain-mcspdu", - "unexpected application tag for CHOICE", - )) + DomainMcsPdu::from_choice(choice) + .ok_or_else(|| PduError::invalid_message(ctx, "domain-mcspdu", "unexpected application tag for CHOICE")) } fn write_mcspdu_header(dst: &mut WriteCursor<'_>, domain_mcspdu: DomainMcsPdu, options: u8) { @@ -806,19 +800,14 @@ impl<'de> McsPdu<'de> for DisconnectProviderUltimatum { let reason = (b1 & 0x03) << 1 | (b2 >> 7); DomainMcsPdu::from_u8(domain_mcspdu_choice) - .ok_or(PduError::invalid_message( - Self::MCS_NAME, - "domain-mcspdu", - "unexpected application tag for CHOICE", - ))? + .ok_or_else(|| { + PduError::invalid_message(Self::MCS_NAME, "domain-mcspdu", "unexpected application tag for CHOICE") + })? .check_expected(Self::MCS_NAME, DomainMcsPdu::DisconnectProviderUltimatum)?; Ok(Self { - reason: DisconnectReason::from_u8(reason).ok_or(PduError::invalid_message( - Self::MCS_NAME, - "reason", - "unknown variant", - ))?, + reason: DisconnectReason::from_u8(reason) + .ok_or_else(|| PduError::invalid_message(Self::MCS_NAME, "reason", "unknown variant"))?, }) } @@ -972,7 +961,7 @@ mod legacy { impl PduParsing for ConnectInitial { type Error = McsError; - fn from_buffer(mut stream: impl io::Read) -> std::result::Result { + fn from_buffer(mut stream: impl io::Read) -> Result { ber::read_application_tag(&mut stream, MCS_TYPE_CONNECT_INITIAL)?; let calling_domain_selector = ber::read_octet_string(&mut stream)?; let called_domain_selector = ber::read_octet_string(&mut stream)?; @@ -994,7 +983,7 @@ mod legacy { }) } - fn to_buffer(&self, mut stream: impl io::Write) -> std::result::Result<(), McsError> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), McsError> { ber::write_application_tag(&mut stream, MCS_TYPE_CONNECT_INITIAL, self.fields_buffer_ber_length())?; ber::write_octet_string(&mut stream, self.calling_domain_selector.as_ref())?; ber::write_octet_string(&mut stream, self.called_domain_selector.as_ref())?; @@ -1028,7 +1017,7 @@ mod legacy { impl PduParsing for ConnectResponse { type Error = McsError; - fn from_buffer(mut stream: impl io::Read) -> std::result::Result { + fn from_buffer(mut stream: impl io::Read) -> Result { ber::read_application_tag(&mut stream, MCS_TYPE_CONNECT_RESPONSE)?; ber::read_enumerated(&mut stream, RESULT_ENUM_LENGTH)?; let called_connect_id = ber::read_integer(&mut stream)? as u32; @@ -1043,7 +1032,7 @@ mod legacy { }) } - fn to_buffer(&self, mut stream: impl io::Write) -> std::result::Result<(), McsError> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), McsError> { ber::write_application_tag(&mut stream, MCS_TYPE_CONNECT_RESPONSE, self.fields_buffer_ber_length())?; ber::write_enumerated(&mut stream, 0)?; ber::write_integer(&mut stream, self.called_connect_id)?; diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets.rs b/crates/ironrdp-pdu/src/rdp/capability_sets.rs index dab8f6267..939edc363 100644 --- a/crates/ironrdp-pdu/src/rdp/capability_sets.rs +++ b/crates/ironrdp-pdu/src/rdp/capability_sets.rs @@ -137,7 +137,7 @@ impl PduParsing for DemandActive { stream.read_exact(source_descriptor_buffer.as_mut())?; let source_descriptor = String::from_utf8(source_descriptor_buffer)? .trim_end_matches(NULL_TERMINATOR) - .to_string(); + .to_owned(); let capability_sets_count = stream.read_u16::()? as usize; let _padding = stream.read_u16::()?; diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs index 304edd3f5..b03adedf1 100644 --- a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs +++ b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs @@ -82,7 +82,7 @@ impl PduParsing for Guid { } fn buffer_length(&self) -> usize { - 16_usize + 16usize } } diff --git a/crates/ironrdp-pdu/src/rdp/server_license.rs b/crates/ironrdp-pdu/src/rdp/server_license.rs index f6bf0d1f7..f80412904 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license.rs @@ -23,9 +23,9 @@ mod server_upgrade_license; pub use self::client_new_license_request::{ClientNewLicenseRequest, PLATFORM_ID}; pub use self::client_platform_challenge_response::ClientPlatformChallengeResponse; pub use self::licensing_error_message::{LicenseErrorCode, LicensingErrorMessage, LicensingStateTransition}; -pub use self::server_license_request::{InitialMessageType, InitialServerLicenseMessage, ServerLicenseRequest}; +pub use self::server_license_request::{cert, InitialMessageType, InitialServerLicenseMessage, ServerLicenseRequest}; pub use self::server_platform_challenge::ServerPlatformChallenge; -pub use self::server_upgrade_license::ServerUpgradeLicense; +pub use self::server_upgrade_license::{NewLicenseInformation, ServerUpgradeLicense}; pub const PREAMBLE_SIZE: usize = 4; pub const PREMASTER_SECRET_SIZE: usize = 48; diff --git a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs index d4abd659d..37394010f 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs @@ -107,8 +107,8 @@ impl ClientNewLicenseRequest { license_header, client_random: Vec::from(client_random), encrypted_premaster_secret, - client_username: client_username.to_string(), - client_machine_name: client_machine_name.to_string(), + client_username: client_username.to_owned(), + client_machine_name: client_machine_name.to_owned(), }, LicenseEncryptionData { premaster_secret: Vec::from(premaster_secret), diff --git a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request/tests.rs b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request/tests.rs index 2766f4eed..ff3220d9e 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request/tests.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request/tests.rs @@ -117,7 +117,7 @@ lazy_static! { .concat() }; - pub static ref SERVER_LICENSE_REQUEST: ServerLicenseRequest = ServerLicenseRequest { + pub(crate) static ref SERVER_LICENSE_REQUEST: ServerLicenseRequest = ServerLicenseRequest { server_random: Vec::from(SERVER_RANDOM_BUFFER.as_ref()), product_info: ProductInfo { version: 0x60000, diff --git a/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response.rs b/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response.rs index 6482d34e3..1aca2853f 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response.rs @@ -128,7 +128,7 @@ impl PduParsing for ClientPlatformChallengeResponse { }) } - fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { self.license_header.to_buffer(&mut stream)?; BlobHeader::new(BlobType::EncryptedData, self.encrypted_challenge_response_data.len()) @@ -166,7 +166,7 @@ enum LicenseDetailLevel { } #[derive(Debug, PartialEq)] -pub struct PlatformChallengeResponseData { +pub(crate) struct PlatformChallengeResponseData { client_type: ClientType, license_detail_level: LicenseDetailLevel, challenge: Vec, @@ -198,7 +198,7 @@ impl PduParsing for PlatformChallengeResponseData { }) } - fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { stream.write_u16::(RESPONSE_DATA_VERSION)?; stream.write_u16::(self.client_type.to_u16().unwrap())?; stream.write_u16::(self.license_detail_level.to_u16().unwrap())?; @@ -214,9 +214,9 @@ impl PduParsing for PlatformChallengeResponseData { } #[derive(Debug, PartialEq, Eq)] -pub struct ClientHardwareIdentification { - pub platform_id: u32, - pub data: Vec, +pub(crate) struct ClientHardwareIdentification { + pub(crate) platform_id: u32, + pub(crate) data: Vec, } impl PduParsing for ClientHardwareIdentification { @@ -231,7 +231,7 @@ impl PduParsing for ClientHardwareIdentification { Ok(Self { platform_id, data }) } - fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { stream.write_u32::(self.platform_id)?; stream.write_all(&self.data)?; diff --git a/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response/test.rs b/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response/test.rs index 610677c5f..4524d4cc2 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response/test.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/client_platform_challenge_response/test.rs @@ -47,16 +47,16 @@ const DATA_BUFFER: [u8; 16] = [ ]; lazy_static! { - pub static ref RESPONSE: PlatformChallengeResponseData = PlatformChallengeResponseData { + pub(crate) static ref RESPONSE: PlatformChallengeResponseData = PlatformChallengeResponseData { client_type: ClientType::Win32, license_detail_level: LicenseDetailLevel::Detail, challenge: Vec::from(CHALLENGE_BUFFER.as_ref()), }; - pub static ref CLIENT_HARDWARE_IDENTIFICATION: ClientHardwareIdentification = ClientHardwareIdentification { + pub(crate) static ref CLIENT_HARDWARE_IDENTIFICATION: ClientHardwareIdentification = ClientHardwareIdentification { platform_id: HARDWARE_ID, data: Vec::from(DATA_BUFFER.as_ref()), }; - pub static ref CLIENT_PLATFORM_CHALLENGE_RESPONSE: ClientPlatformChallengeResponse = + pub(crate) static ref CLIENT_PLATFORM_CHALLENGE_RESPONSE: ClientPlatformChallengeResponse = ClientPlatformChallengeResponse { license_header: LicenseHeader { security_header: BasicSecurityHeader { diff --git a/crates/ironrdp-pdu/src/rdp/server_license/server_license_request.rs b/crates/ironrdp-pdu/src/rdp/server_license/server_license_request.rs index 604a29e4f..d02e0ef33 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/server_license_request.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/server_license_request.rs @@ -1,4 +1,5 @@ pub mod cert; + #[cfg(test)] mod tests; diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc.rs index 6d0ecb93c..30556362c 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc.rs @@ -247,9 +247,9 @@ impl FieldType { #[derive(Debug, Clone, PartialEq)] struct Header { - pub channel_id_type: u8, // 2 bit - pub pdu_dependent: u8, // 2 bit - pub pdu_type: PduType, // 4 bit + channel_id_type: u8, // 2 bit + pdu_dependent: u8, // 2 bit + pdu_type: PduType, // 4 bit } impl PduParsing for Header { diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/display.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/display.rs index 366aa6759..c95cc39f2 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/display.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/display.rs @@ -1,4 +1,4 @@ -use std::io::{self, Read, Write}; +use std::io; use bitflags::bitflags; use byteorder::{LittleEndian, ReadBytesExt as _, WriteBytesExt as _}; @@ -20,7 +20,7 @@ pub struct DisplayControlCapsPdu { impl PduParsing for DisplayControlCapsPdu { type Error = io::Error; - fn from_buffer(mut stream: impl Read) -> Result { + fn from_buffer(mut stream: impl io::Read) -> Result { let max_num_monitors = stream.read_u32::()?; let max_monitor_area_factora = stream.read_u32::()?; let max_monitor_area_factorb = stream.read_u32::()?; @@ -32,7 +32,7 @@ impl PduParsing for DisplayControlCapsPdu { }) } - fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { stream.write_u32::(self.max_num_monitors)?; stream.write_u32::(self.max_monitor_area_factora)?; stream.write_u32::(self.max_monitor_area_factorb)?; @@ -80,7 +80,7 @@ const MONITOR_PDU_HEADER_SIZE: usize = 8; impl PduParsing for Monitor { type Error = io::Error; - fn from_buffer(mut stream: impl Read) -> Result { + fn from_buffer(mut stream: impl io::Read) -> Result { let flags = MonitorFlags::from_bits(stream.read_u32::()?) .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Invalid monitor flags"))?; let left = stream.read_u32::()?; @@ -108,7 +108,7 @@ impl PduParsing for Monitor { }) } - fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { stream.write_u32::(self.flags.bits())?; stream.write_u32::(self.left)?; stream.write_u32::(self.top)?; @@ -136,7 +136,7 @@ pub struct MonitorLayoutPdu { impl PduParsing for MonitorLayoutPdu { type Error = io::Error; - fn from_buffer(mut stream: impl Read) -> Result { + fn from_buffer(mut stream: impl io::Read) -> Result { let _size = stream.read_u32::()?; let num_monitors = stream.read_u32::()?; let mut monitors = Vec::new(); @@ -146,7 +146,7 @@ impl PduParsing for MonitorLayoutPdu { Ok(Self { monitors }) } - fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { stream.write_u32::(MONITOR_SIZE as u32)?; stream.write_u32::(self.monitors.len() as u32)?; diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages.rs index b14ded396..e30b969be 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages.rs @@ -3,19 +3,22 @@ mod server; use std::io; mod avc_messages; -pub use avc_messages::{Avc420BitmapStream, Avc444BitmapStream, Encoding, QuantQuality}; use bitflags::bitflags; use byteorder::{LittleEndian, ReadBytesExt as _, WriteBytesExt as _}; -pub use client::{CacheImportReplyPdu, CapabilitiesAdvertisePdu, FrameAcknowledgePdu, QueueDepth}; use num_derive::{FromPrimitive, ToPrimitive}; use num_traits::{FromPrimitive as _, ToPrimitive as _}; +use thiserror::Error; + +#[rustfmt::skip] // do not re-order this +pub use avc_messages::{Avc420BitmapStream, Avc444BitmapStream, Encoding, QuantQuality}; +pub use client::{CacheImportReplyPdu, CapabilitiesAdvertisePdu, FrameAcknowledgePdu, QueueDepth}; +pub(crate) use server::RESET_GRAPHICS_PDU_SIZE; pub use server::{ CacheToSurfacePdu, CapabilitiesConfirmPdu, Codec1Type, Codec2Type, CreateSurfacePdu, DeleteEncodingContextPdu, DeleteSurfacePdu, EndFramePdu, EvictCacheEntryPdu, MapSurfaceToOutputPdu, MapSurfaceToScaledOutputPdu, MapSurfaceToScaledWindowPdu, PixelFormat, ResetGraphicsPdu, SolidFillPdu, StartFramePdu, SurfaceToCachePdu, - SurfaceToSurfacePdu, Timestamp, WireToSurface1Pdu, WireToSurface2Pdu, RESET_GRAPHICS_PDU_SIZE, + SurfaceToSurfacePdu, Timestamp, WireToSurface1Pdu, WireToSurface2Pdu, }; -use thiserror::Error; use super::RDP_GFX_HEADER_SIZE; use crate::gcc::MonitorDataError; @@ -215,7 +218,7 @@ impl PduParsing for Point { #[repr(u32)] #[derive(Debug, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)] -pub enum CapabilityVersion { +pub(crate) enum CapabilityVersion { V8 = 0x8_0004, V8_1 = 0x8_0105, V10 = 0xa_0002, diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/avc_messages.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/avc_messages.rs index fc7832eba..d371a2dc8 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/avc_messages.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/avc_messages.rs @@ -1,5 +1,5 @@ use std::fmt::Debug; -use std::io::Write; +use std::io::Write as _; use bit_field::BitField; use bitflags::bitflags; @@ -55,7 +55,7 @@ pub struct Avc420BitmapStream<'a> { pub data: &'a [u8], } -impl<'a> Debug for Avc420BitmapStream<'a> { +impl Debug for Avc420BitmapStream<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Avc420BitmapStream") .field("rectangles", &self.rectangles) diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/client.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/client.rs index cb1230fcb..b0330a103 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/client.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/client.rs @@ -1,4 +1,4 @@ -use std::io::{self, Read, Write}; +use std::io; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; @@ -46,7 +46,7 @@ pub struct FrameAcknowledgePdu { impl PduParsing for FrameAcknowledgePdu { type Error = GraphicsMessagesError; - fn from_buffer(mut stream: impl Read) -> Result { + fn from_buffer(mut stream: impl io::Read) -> Result { let queue_depth = QueueDepth::from_u32(stream.read_u32::()?); let frame_id = stream.read_u32::()?; let total_frames_decoded = stream.read_u32::()?; @@ -58,7 +58,7 @@ impl PduParsing for FrameAcknowledgePdu { }) } - fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { stream.write_u32::(self.queue_depth.to_u32())?; stream.write_u32::(self.frame_id)?; stream.write_u32::(self.total_frames_decoded)?; @@ -79,7 +79,7 @@ pub struct CacheImportReplyPdu { impl PduParsing for CacheImportReplyPdu { type Error = GraphicsMessagesError; - fn from_buffer(mut stream: impl Read) -> Result { + fn from_buffer(mut stream: impl io::Read) -> Result { let entries_count = stream.read_u16::()? as usize; let cache_slots = (0..entries_count) @@ -89,7 +89,7 @@ impl PduParsing for CacheImportReplyPdu { Ok(Self { cache_slots }) } - fn to_buffer(&self, mut stream: impl Write) -> Result<(), Self::Error> { + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { stream.write_u16::(self.cache_slots.len() as u16)?; for cache_slot in self.cache_slots.iter() { diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/server.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/server.rs index 680e73843..d95aa5cb4 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/server.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/gfx/graphics_messages/server.rs @@ -10,7 +10,7 @@ use crate::gcc::{Monitor, MonitorDataError}; use crate::geometry::InclusiveRectangle; use crate::PduParsing; -pub const RESET_GRAPHICS_PDU_SIZE: usize = 340; +pub(crate) const RESET_GRAPHICS_PDU_SIZE: usize = 340; const MAX_RESET_GRAPHICS_WIDTH_HEIGHT: u32 = 32_766; const MONITOR_COUNT_MAX: u32 = 16; diff --git a/crates/ironrdp-pdu/src/utils.rs b/crates/ironrdp-pdu/src/utils.rs index 55190aa57..a7c22a550 100644 --- a/crates/ironrdp-pdu/src/utils.rs +++ b/crates/ironrdp-pdu/src/utils.rs @@ -189,6 +189,7 @@ pub(crate) fn write_string_with_null_terminator( } pub trait SplitTo { + #[must_use] fn split_to(&mut self, n: usize) -> Self; } diff --git a/crates/ironrdp-rdcleanpath/src/lib.rs b/crates/ironrdp-rdcleanpath/src/lib.rs index 257b83591..a2e28c60c 100644 --- a/crates/ironrdp-rdcleanpath/src/lib.rs +++ b/crates/ironrdp-rdcleanpath/src/lib.rs @@ -127,11 +127,11 @@ impl RDCleanPathPdu { pub fn detect(src: &[u8]) -> DetectionResult { use der::{Decode as _, Encode as _}; - let Ok(mut reader) = der::SliceReader::new(src) else { + let Ok(mut slice_reader) = der::SliceReader::new(src) else { return DetectionResult::Failed; }; - let header = match der::Header::decode(&mut reader) { + let header = match der::Header::decode(&mut slice_reader) { Ok(header) => header, Err(e) => match e.kind() { der::ErrorKind::Incomplete { .. } => return DetectionResult::NotEnoughBytes, @@ -146,9 +146,11 @@ impl RDCleanPathPdu { return DetectionResult::Failed; }; - let total_length = header_encoded_len + body_length; + let Some(total_length) = header_encoded_len.checked_add(body_length) else { + return DetectionResult::Failed; + }; - match der::asn1::ContextSpecific::::decode_explicit(&mut reader, der::TagNumber::N0) { + match der::asn1::ContextSpecific::::decode_explicit(&mut slice_reader, der::TagNumber::N0) { Ok(Some(version)) if version.value == VERSION_1 => DetectionResult::Detected { version: VERSION_1, total_length, diff --git a/crates/ironrdp-rdpdr/README.md b/crates/ironrdp-rdpdr/README.md index 97c3f4399..b186d6a4e 100644 --- a/crates/ironrdp-rdpdr/README.md +++ b/crates/ironrdp-rdpdr/README.md @@ -1,3 +1,6 @@ # IronRDP RDPDR -RDPDR channel implementation. \ No newline at end of file +Implements the RDPDR static virtual channel as described in +[\[MS-RDPEFS\]: Remote Desktop Protocol: File System Virtual Channel Extension][spec] + +[spec]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/34d9de58-b2b5-40b6-b970-f82d4603bdb5 diff --git a/crates/ironrdp-rdpdr/src/lib.rs b/crates/ironrdp-rdpdr/src/lib.rs index 636425f9f..eaff126f9 100644 --- a/crates/ironrdp-rdpdr/src/lib.rs +++ b/crates/ironrdp-rdpdr/src/lib.rs @@ -1,9 +1,12 @@ -//! Implements the RDPDR static virtual channel as described in -//! [\[MS-RDPEFS\]: Remote Desktop Protocol: File System Virtual Channel Extension] -//! -//! [\[MS-RDPEFS\]: Remote Desktop Protocol: File System Virtual Channel Extension]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/34d9de58-b2b5-40b6-b970-f82d4603bdb5 +#![doc = include_str!("../README.md")] +#![allow(clippy::arithmetic_side_effects)] // FIXME: remove +#![allow(clippy::cast_lossless)] // FIXME: remove +#![allow(clippy::cast_possible_truncation)] // FIXME: remove +#![allow(clippy::cast_possible_wrap)] // FIXME: remove +#![allow(clippy::cast_sign_loss)] // FIXME: remove pub mod pdu; + use ironrdp_pdu::gcc::ChannelName; use ironrdp_pdu::{decode, other_err, PduResult}; use ironrdp_svc::{impl_as_any, CompressionCondition, StaticVirtualChannelProcessor, SvcMessage}; @@ -46,6 +49,7 @@ impl Rdpdr { } } + #[must_use] pub fn with_smartcard(mut self, device_id: u32) -> Self { self.device_list.add_smartcard(device_id); self.capabilities.add_smartcard(); diff --git a/crates/ironrdp-rdpdr/src/pdu/efs.rs b/crates/ironrdp-rdpdr/src/pdu/efs.rs index df5406a4a..189130f51 100644 --- a/crates/ironrdp-rdpdr/src/pdu/efs.rs +++ b/crates/ironrdp-rdpdr/src/pdu/efs.rs @@ -383,7 +383,7 @@ impl CapabilityMessage { } fn size(&self) -> usize { - self.header.size() + self.capability_data.size() + CapabilityHeader::SIZE + self.capability_data.size() } } @@ -437,16 +437,12 @@ impl CapabilityHeader { } fn encode(&self, dst: &mut WriteCursor) -> PduResult<()> { - ensure_size!(in: dst, size: self.size()); + ensure_size!(in: dst, size: Self::SIZE); dst.write_u16(self.cap_type as u16); dst.write_u16(self.length); dst.write_u32(self.version); Ok(()) } - - fn size(&self) -> usize { - Self::SIZE - } } #[derive(Debug, Clone, Copy)] @@ -465,13 +461,13 @@ enum CapabilityType { } /// GENERAL_CAPABILITY_VERSION_02 -pub const GENERAL_CAPABILITY_VERSION_02: u32 = 0x00000002; +pub const GENERAL_CAPABILITY_VERSION_02: u32 = 0x0000_0002; /// SMARTCARD_CAPABILITY_VERSION_01 -pub const SMARTCARD_CAPABILITY_VERSION_01: u32 = 0x00000001; +pub const SMARTCARD_CAPABILITY_VERSION_01: u32 = 0x0000_0001; /// DRIVE_CAPABILITY_VERSION_02 -pub const DRIVE_CAPABILITY_VERSION_02: u32 = 0x00000002; +pub const DRIVE_CAPABILITY_VERSION_02: u32 = 0x0000_0002; -impl std::convert::TryFrom for CapabilityType { +impl TryFrom for CapabilityType { type Error = PduError; fn try_from(value: u16) -> Result { @@ -518,7 +514,7 @@ impl CapabilityData { fn size(&self) -> usize { match self { - CapabilityData::General(general) => general.size(), + CapabilityData::General(_) => GeneralCapabilitySet::SIZE, CapabilityData::Printer => 0, CapabilityData::Port => 0, CapabilityData::Drive => 0, @@ -565,7 +561,7 @@ impl GeneralCapabilitySet { const SIZE: usize = size_of::() * 8 + size_of::() * 2; fn encode(&self, dst: &mut WriteCursor) -> PduResult<()> { - ensure_size!(in: dst, size: self.size()); + ensure_size!(in: dst, size: Self::SIZE); dst.write_u32(self.os_type); dst.write_u32(self.os_version); dst.write_u16(self.protocol_major_version); @@ -612,10 +608,6 @@ impl GeneralCapabilitySet { special_type_device_cap, }) } - - fn size(&self) -> usize { - Self::SIZE - } } bitflags! { @@ -626,37 +618,37 @@ bitflags! { #[derive(Debug, Clone, Copy)] struct IoCode1: u32 { /// Unused, always set. - const RDPDR_IRP_MJ_CREATE = 0x00000001; + const RDPDR_IRP_MJ_CREATE = 0x0000_0001; /// Unused, always set. - const RDPDR_IRP_MJ_CLEANUP = 0x00000002; + const RDPDR_IRP_MJ_CLEANUP = 0x0000_0002; /// Unused, always set. - const RDPDR_IRP_MJ_CLOSE = 0x00000004; + const RDPDR_IRP_MJ_CLOSE = 0x0000_0004; /// Unused, always set. - const RDPDR_IRP_MJ_READ = 0x00000008; + const RDPDR_IRP_MJ_READ = 0x0000_0008; /// Unused, always set. - const RDPDR_IRP_MJ_WRITE = 0x00000010; + const RDPDR_IRP_MJ_WRITE = 0x0000_0010; /// Unused, always set. - const RDPDR_IRP_MJ_FLUSH_BUFFERS = 0x00000020; + const RDPDR_IRP_MJ_FLUSH_BUFFERS = 0x0000_0020; /// Unused, always set. - const RDPDR_IRP_MJ_SHUTDOWN = 0x00000040; + const RDPDR_IRP_MJ_SHUTDOWN = 0x0000_0040; /// Unused, always set. - const RDPDR_IRP_MJ_DEVICE_CONTROL = 0x00000080; + const RDPDR_IRP_MJ_DEVICE_CONTROL = 0x0000_0080; /// Unused, always set. - const RDPDR_IRP_MJ_QUERY_VOLUME_INFORMATION = 0x00000100; + const RDPDR_IRP_MJ_QUERY_VOLUME_INFORMATION = 0x0000_0100; /// Unused, always set. - const RDPDR_IRP_MJ_SET_VOLUME_INFORMATION = 0x00000200; + const RDPDR_IRP_MJ_SET_VOLUME_INFORMATION = 0x0000_0200; /// Unused, always set. - const RDPDR_IRP_MJ_QUERY_INFORMATION = 0x00000400; + const RDPDR_IRP_MJ_QUERY_INFORMATION = 0x0000_0400; /// Unused, always set. - const RDPDR_IRP_MJ_SET_INFORMATION = 0x00000800; + const RDPDR_IRP_MJ_SET_INFORMATION = 0x0000_0800; /// Unused, always set. - const RDPDR_IRP_MJ_DIRECTORY_CONTROL = 0x00001000; + const RDPDR_IRP_MJ_DIRECTORY_CONTROL = 0x0000_1000; /// Unused, always set. - const RDPDR_IRP_MJ_LOCK_CONTROL = 0x00002000; + const RDPDR_IRP_MJ_LOCK_CONTROL = 0x0000_2000; /// Enable Query Security requests (IRP_MJ_QUERY_SECURITY). - const RDPDR_IRP_MJ_QUERY_SECURITY = 0x00004000; + const RDPDR_IRP_MJ_QUERY_SECURITY = 0x0000_4000; /// Enable Set Security requests (IRP_MJ_SET_SECURITY). - const RDPDR_IRP_MJ_SET_SECURITY = 0x00008000; + const RDPDR_IRP_MJ_SET_SECURITY = 0x0000_8000; /// Combination of all the required bits. const REQUIRED = Self::RDPDR_IRP_MJ_CREATE.bits() @@ -683,11 +675,11 @@ bitflags! { #[derive(Debug, Clone, Copy)] struct ExtendedPdu: u32 { /// Allow the client to send Client Drive Device List Remove packets. - const RDPDR_DEVICE_REMOVE_PDUS = 0x00000001; + const RDPDR_DEVICE_REMOVE_PDUS = 0x0000_0001; /// Unused, always set. - const RDPDR_CLIENT_DISPLAY_NAME_PDU = 0x00000002; + const RDPDR_CLIENT_DISPLAY_NAME_PDU = 0x0000_0002; /// Allow the server to send a Server User Logged On packet. - const RDPDR_USER_LOGGEDON_PDU = 0x00000004; + const RDPDR_USER_LOGGEDON_PDU = 0x0000_0004; } } @@ -699,7 +691,7 @@ bitflags! { /// Optionally present only in the Client Core Capability Response. /// Allows the server to send multiple simultaneous read or write requests /// on the same file from a redirected file system. - const ENABLE_ASYNCIO = 0x00000001; + const ENABLE_ASYNCIO = 0x0000_0001; } } @@ -795,7 +787,7 @@ impl DeviceAnnounceHeader { device_type: DeviceType::Smartcard, device_id, // This name is a constant defined by the spec. - preferred_dos_name: PreferredDosName("SCARD".to_string()), + preferred_dos_name: PreferredDosName("SCARD".to_owned()), device_data: Vec::new(), } } @@ -851,15 +843,15 @@ impl PreferredDosName { #[repr(u32)] pub enum DeviceType { /// RDPDR_DTYP_SERIAL - Serial = 0x00000001, + Serial = 0x0000_0001, /// RDPDR_DTYP_PARALLEL - Parallel = 0x00000002, + Parallel = 0x0000_0002, /// RDPDR_DTYP_PRINT - Print = 0x00000004, + Print = 0x0000_0004, /// RDPDR_DTYP_FILESYSTEM - Filesystem = 0x00000008, + Filesystem = 0x0000_0008, /// RDPDR_DTYP_SMARTCARD - Smartcard = 0x00000020, + Smartcard = 0x0000_0020, } impl From for u32 { @@ -868,16 +860,16 @@ impl From for u32 { } } -impl std::convert::TryFrom for DeviceType { +impl TryFrom for DeviceType { type Error = PduError; fn try_from(value: u32) -> Result { match value { - 0x00000001 => Ok(DeviceType::Serial), - 0x00000002 => Ok(DeviceType::Parallel), - 0x00000004 => Ok(DeviceType::Print), - 0x00000008 => Ok(DeviceType::Filesystem), - 0x00000020 => Ok(DeviceType::Smartcard), + 0x0000_0001 => Ok(DeviceType::Serial), + 0x0000_0002 => Ok(DeviceType::Parallel), + 0x0000_0004 => Ok(DeviceType::Print), + 0x0000_0008 => Ok(DeviceType::Filesystem), + 0x0000_0020 => Ok(DeviceType::Smartcard), _ => Err(invalid_message_err!("try_from", "DeviceType", "invalid value")), } } diff --git a/crates/ironrdp-rdpdr/src/pdu/mod.rs b/crates/ironrdp-rdpdr/src/pdu/mod.rs index dc99705b6..93bf86416 100644 --- a/crates/ironrdp-rdpdr/src/pdu/mod.rs +++ b/crates/ironrdp-rdpdr/src/pdu/mod.rs @@ -78,7 +78,7 @@ impl PduDecode<'_> for RdpdrPdu { } impl PduEncode for RdpdrPdu { - fn encode(&self, dst: &mut ironrdp_pdu::cursor::WriteCursor<'_>) -> PduResult<()> { + fn encode(&self, dst: &mut WriteCursor<'_>) -> PduResult<()> { self.header().encode(dst)?; match self { @@ -218,7 +218,7 @@ pub enum PacketId { Unimplemented, } -impl std::convert::TryFrom for PacketId { +impl TryFrom for PacketId { type Error = PduError; fn try_from(value: u16) -> Result { diff --git a/crates/ironrdp-rdpsnd/Cargo.toml b/crates/ironrdp-rdpsnd/Cargo.toml index 3b219ca79..ce08496e1 100644 --- a/crates/ironrdp-rdpsnd/Cargo.toml +++ b/crates/ironrdp-rdpsnd/Cargo.toml @@ -22,4 +22,3 @@ std = [] [dependencies] ironrdp-svc.workspace = true ironrdp-pdu = { workspace = true, features = ["alloc"] } -tracing.workspace = true diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index e861cd95a..272419daf 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -43,6 +43,7 @@ impl Default for RdpServerBuilder { } impl RdpServerBuilder { + #[allow(clippy::unused_self)] // ensuring state transition from WantsAddr pub fn with_addr(self, addr: impl Into) -> RdpServerBuilder { RdpServerBuilder { state: WantsSecurity { addr: addr.into() }, @@ -135,7 +136,7 @@ impl RdpServerBuilder { } } -pub struct NoopInputHandler; +struct NoopInputHandler; #[async_trait::async_trait] impl RdpServerInputHandler for NoopInputHandler { @@ -143,7 +144,7 @@ impl RdpServerInputHandler for NoopInputHandler { async fn mouse(&mut self, _: MouseEvent) {} } -pub struct NoopDisplay; +struct NoopDisplay; #[async_trait::async_trait] impl RdpServerDisplay for NoopDisplay { diff --git a/crates/ironrdp-server/src/capabilities.rs b/crates/ironrdp-server/src/capabilities.rs index 20f7bfaa1..84d6d52cb 100644 --- a/crates/ironrdp-server/src/capabilities.rs +++ b/crates/ironrdp-server/src/capabilities.rs @@ -2,7 +2,7 @@ use ironrdp_pdu::rdp::capability_sets; use crate::{DesktopSize, RdpServerOptions}; -pub fn capabilities(_opts: &RdpServerOptions, size: DesktopSize) -> Vec { +pub(crate) fn capabilities(_opts: &RdpServerOptions, size: DesktopSize) -> Vec { vec![ capability_sets::CapabilitySet::General(general_capabilities()), capability_sets::CapabilitySet::Bitmap(bitmap_capabilities(&size)), diff --git a/crates/ironrdp-server/src/encoder/bitmap.rs b/crates/ironrdp-server/src/encoder/bitmap.rs index 8cc5139f3..69baac752 100644 --- a/crates/ironrdp-server/src/encoder/bitmap.rs +++ b/crates/ironrdp-server/src/encoder/bitmap.rs @@ -8,30 +8,30 @@ use ironrdp_pdu::{PduEncode, PduError}; use crate::{BitmapUpdate, PixelOrder}; // PERF: we could also remove the need for this buffer -pub struct BitmapEncoder { +pub(crate) struct BitmapEncoder { buffer: Vec, } impl BitmapEncoder { - pub fn new() -> Self { + pub(crate) fn new() -> Self { Self { buffer: vec![0; u16::MAX as usize], } } - pub fn encode(&mut self, bitmap: &BitmapUpdate, output: &mut [u8]) -> Result { - let row_len = bitmap.width * bitmap.format.bytes_per_pixel() as u32; - let chunk_height = u16::MAX as u32 / row_len; + pub(crate) fn encode(&mut self, bitmap: &BitmapUpdate, output: &mut [u8]) -> Result { + let row_len = bitmap.width * u32::from(bitmap.format.bytes_per_pixel()); + let chunk_height = u32::from(u16::MAX) / row_len; let mut cursor = WriteCursor::new(output); let chunks = bitmap.data.chunks((row_len * chunk_height) as usize); - let total = chunks.size_hint().0; - BitmapUpdateData::encode_header(total as u16, &mut cursor)?; + let total = u16::try_from(chunks.clone().count()).unwrap(); + BitmapUpdateData::encode_header(total, &mut cursor)?; for (i, chunk) in chunks.enumerate() { - let height = chunk.len() as u32 / row_len; - let top = bitmap.top + i as u32 * chunk_height; + let height = u32::try_from(chunk.len()).unwrap() / row_len; + let top = bitmap.top + u32::try_from(i).unwrap() * chunk_height; let encoder = BitmapStreamEncoder::new(bitmap.width as usize, height as usize); @@ -53,19 +53,19 @@ impl BitmapEncoder { let data = BitmapData { rectangle: InclusiveRectangle { - left: bitmap.left as u16, - top: top as u16, - right: (bitmap.left + bitmap.width - 1) as u16, - bottom: (top + height - 1) as u16, + left: u16::try_from(bitmap.left).unwrap(), + top: u16::try_from(top).unwrap(), + right: u16::try_from(bitmap.left + bitmap.width - 1).unwrap(), + bottom: u16::try_from(top + height - 1).unwrap(), }, - width: bitmap.width as u16, - height: height as u16, - bits_per_pixel: bitmap.format.bytes_per_pixel() as u16 * 8, + width: u16::try_from(bitmap.width).unwrap(), + height: u16::try_from(height).unwrap(), + 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: len as u16, - scan_width: bitmap.width as u16, - uncompressed_size: chunk.len() as u16, + main_body_size: u16::try_from(len).unwrap(), + scan_width: u16::try_from(bitmap.width).unwrap(), + uncompressed_size: u16::try_from(chunk.len()).unwrap(), }), bitmap_data: &self.buffer[..len], }; diff --git a/crates/ironrdp-server/src/encoder/mod.rs b/crates/ironrdp-server/src/encoder/mod.rs index d9dd66dfc..dbeff3c70 100644 --- a/crates/ironrdp-server/src/encoder/mod.rs +++ b/crates/ironrdp-server/src/encoder/mod.rs @@ -1,4 +1,4 @@ -pub mod bitmap; +pub(crate) mod bitmap; use std::cmp; @@ -14,20 +14,20 @@ const MAX_FASTPATH_UPDATE_SIZE: usize = 16_374; const FASTPATH_HEADER_SIZE: usize = 6; -pub struct UpdateEncoder { +pub(crate) struct UpdateEncoder { buffer: Vec, bitmap: BitmapEncoder, } impl UpdateEncoder { - pub fn new() -> Self { + pub(crate) fn new() -> Self { Self { buffer: vec![0; 16384], bitmap: BitmapEncoder::new(), } } - pub fn bitmap(&mut self, bitmap: BitmapUpdate) -> Option { + pub(crate) fn bitmap(&mut self, bitmap: BitmapUpdate) -> Option { let len = loop { match self.bitmap.encode(&bitmap, self.buffer.as_mut_slice()) { Err(e) => match e.kind() { @@ -49,25 +49,25 @@ impl UpdateEncoder { } } -pub struct UpdateFragmenter<'a> { +pub(crate) struct UpdateFragmenter<'a> { code: UpdateCode, index: usize, data: &'a [u8], } impl<'a> UpdateFragmenter<'a> { - pub fn new(code: UpdateCode, data: &'a [u8]) -> Self { + pub(crate) fn new(code: UpdateCode, data: &'a [u8]) -> Self { Self { code, index: 0, data } } - pub fn size_hint(&self) -> usize { + pub(crate) fn size_hint(&self) -> usize { FASTPATH_HEADER_SIZE + cmp::min(self.data.len(), MAX_FASTPATH_UPDATE_SIZE) } - pub fn next(&mut self, dst: &mut [u8]) -> Option { + pub(crate) fn next(&mut self, dst: &mut [u8]) -> Option { let (consumed, written) = self.encode_next(dst)?; self.data = &self.data[consumed..]; - self.index += 1; + self.index.checked_add(1)?; Some(written) } diff --git a/crates/ironrdp-server/src/handler.rs b/crates/ironrdp-server/src/handler.rs index 23c1ba355..f775ab741 100644 --- a/crates/ironrdp-server/src/handler.rs +++ b/crates/ironrdp-server/src/handler.rs @@ -89,6 +89,7 @@ impl From<(u16, fast_path::KeyboardFlags)> for KeyboardEvent { } impl From<(u16, scan_code::KeyboardFlags)> for KeyboardEvent { + #[allow(clippy::cast_possible_truncation)] // we are actually truncating the value fn from((key, flags): (u16, scan_code::KeyboardFlags)) -> Self { let extended = flags.contains(scan_code::KeyboardFlags::EXTENDED); if flags.contains(scan_code::KeyboardFlags::RELEASE) { @@ -122,6 +123,7 @@ impl From for KeyboardEvent { } impl From for KeyboardEvent { + #[allow(clippy::cast_possible_truncation)] // we are actually truncating the value fn from(value: SyncToggleFlags) -> Self { KeyboardEvent::Synchronize(SynchronizeFlags::from_bits_truncate(value.bits() as u8)) } diff --git a/crates/ironrdp-server/src/lib.rs b/crates/ironrdp-server/src/lib.rs index cac12c11b..d7e43ff5d 100644 --- a/crates/ironrdp-server/src/lib.rs +++ b/crates/ironrdp-server/src/lib.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] // TODO: should we enable this lint back? + #[macro_use] extern crate tracing; diff --git a/crates/ironrdp-session-generators/Cargo.toml b/crates/ironrdp-session-generators/Cargo.toml index c478fa9d0..1f3f2fd3c 100644 --- a/crates/ironrdp-session-generators/Cargo.toml +++ b/crates/ironrdp-session-generators/Cargo.toml @@ -10,6 +10,6 @@ doctest = false test = false [dependencies] -ironrdp-session.workspace = true -ironrdp-pdu-generators.workspace = true -proptest.workspace = true +# ironrdp-session.workspace = true +# ironrdp-pdu-generators.workspace = true +# proptest.workspace = true diff --git a/crates/ironrdp-session-generators/src/lib.rs b/crates/ironrdp-session-generators/src/lib.rs index 8b1378917..d4bd0bf76 100644 --- a/crates/ironrdp-session-generators/src/lib.rs +++ b/crates/ironrdp-session-generators/src/lib.rs @@ -1 +1,6 @@ - +// No need to be as strict as in production libraries +#![allow(clippy::arithmetic_side_effects)] +#![allow(clippy::cast_lossless)] +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] diff --git a/crates/ironrdp-session/Cargo.toml b/crates/ironrdp-session/Cargo.toml index 9b0fa4a11..93e076f8f 100644 --- a/crates/ironrdp-session/Cargo.toml +++ b/crates/ironrdp-session/Cargo.toml @@ -19,9 +19,8 @@ test = false bitflags = "2" # TODO: investigate usage in this crate ironrdp-connector.workspace = true # TODO: at some point, this dependency could be removed (good for compilation speed) ironrdp-svc.workspace = true -ironrdp-dvc.workspace = true +# ironrdp-dvc.workspace = true ironrdp-error.workspace = true ironrdp-graphics.workspace = true ironrdp-pdu = { workspace = true, features = ["std"] } -sspi.workspace = true tracing.workspace = true diff --git a/crates/ironrdp-session/src/active_stage.rs b/crates/ironrdp-session/src/active_stage.rs index afab88dfd..752b4df32 100644 --- a/crates/ironrdp-session/src/active_stage.rs +++ b/crates/ironrdp-session/src/active_stage.rs @@ -40,7 +40,7 @@ impl ActiveStage { } } - pub fn update_mouse_pos(&mut self, x: usize, y: usize) { + pub fn update_mouse_pos(&mut self, x: u16, y: u16) { self.fast_path_processor.update_mouse_pos(x, y); } @@ -76,8 +76,8 @@ impl ActiveStage { // If mouse was moved by client - we should update framebuffer to reflect new // pointer position let mouse_pos = events.iter().find_map(|event| match event { - FastPathInputEvent::MouseEvent(event) => Some((event.x_position as usize, event.y_position as usize)), - FastPathInputEvent::MouseEventEx(event) => Some((event.x_position as usize, event.y_position as usize)), + FastPathInputEvent::MouseEvent(event) => Some((event.x_position, event.y_position)), + FastPathInputEvent::MouseEventEx(event) => Some((event.x_position, event.y_position)), _ => None, }); @@ -87,7 +87,7 @@ impl ActiveStage { }; // Graphics update is only sent when update is visually changed the framebuffer - if let Some(rect) = image.move_pointer(mouse_x as u16, mouse_y as u16)? { + if let Some(rect) = image.move_pointer(mouse_x, mouse_y)? { output.push(ActiveStageOutput::GraphicsUpdate(rect)); } diff --git a/crates/ironrdp-session/src/fast_path.rs b/crates/ironrdp-session/src/fast_path.rs index 3569bfb8c..c15507f25 100644 --- a/crates/ironrdp-session/src/fast_path.rs +++ b/crates/ironrdp-session/src/fast_path.rs @@ -34,12 +34,12 @@ pub struct Processor { bitmap_stream_decoder: BitmapStreamDecoder, pointer_cache: PointerCache, use_system_pointer: bool, - mouse_pos_update: Option<(usize, usize)>, + mouse_pos_update: Option<(u16, u16)>, no_server_pointer: bool, } impl Processor { - pub fn update_mouse_pos(&mut self, x: usize, y: usize) { + pub fn update_mouse_pos(&mut self, x: u16, y: u16) { self.mouse_pos_update = Some((x, y)); } @@ -53,7 +53,7 @@ impl Processor { let mut processor_updates = Vec::new(); if let Some((x, y)) = self.mouse_pos_update.take() { - if let Some(rect) = image.move_pointer(x as u16, y as u16)? { + if let Some(rect) = image.move_pointer(x, y)? { processor_updates.push(UpdateKind::Region(rect)); } } @@ -110,8 +110,8 @@ impl Processor { match self.bitmap_stream_decoder.decode_bitmap_stream_to_rgb24( update.bitmap_data, &mut buf, - update.width as usize, - update.height as usize, + usize::from(update.width), + usize::from(update.height), ) { Ok(()) => image.apply_rgb24_bitmap(&buf, &update.rectangle)?, Err(err) => { @@ -214,7 +214,9 @@ impl Processor { .expect("Failed to decode color pointer attribute"), ); - let _ = self.pointer_cache.insert(cache_index as usize, decoded_pointer.clone()); + let _ = self + .pointer_cache + .insert(cache_index as usize, Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); @@ -237,7 +239,7 @@ impl Processor { } } } else { - eprintln!("Cached pointer not found {}", cache_index); + warn!("Cached pointer not found {}", cache_index); } } PointerUpdateData::New(pointer) => { @@ -248,7 +250,9 @@ impl Processor { .expect("Failed to decode pointer attribute"), ); - let _ = self.pointer_cache.insert(cache_index as usize, decoded_pointer.clone()); + let _ = self + .pointer_cache + .insert(cache_index as usize, Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); @@ -262,7 +266,9 @@ impl Processor { .expect("Failed to decode large pointer attribute"), ); - let _ = self.pointer_cache.insert(cache_index as usize, decoded_pointer.clone()); + let _ = self + .pointer_cache + .insert(cache_index as usize, Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); diff --git a/crates/ironrdp-session/src/image.rs b/crates/ironrdp-session/src/image.rs index 316eb66bc..4e3cf6d14 100644 --- a/crates/ironrdp-session/src/image.rs +++ b/crates/ironrdp-session/src/image.rs @@ -47,6 +47,7 @@ struct PointerRenderingState { } #[allow(clippy::too_many_arguments)] +#[allow(clippy::cast_lossless)] // FIXME fn copy_cursor_data( from: &[u8], from_pos: (usize, usize), @@ -264,6 +265,9 @@ impl DecodedImage { } } + #[allow(clippy::cast_lossless)] // FIXME + #[allow(clippy::cast_possible_wrap)] // FIXME + #[allow(clippy::cast_possible_truncation)] // FIXME fn recalculate_pointer_geometry(&mut self) { let x = self.pointer_x; let y = self.pointer_y; diff --git a/crates/ironrdp-session/src/lib.rs b/crates/ironrdp-session/src/lib.rs index 443492d67..8eebd3daf 100644 --- a/crates/ironrdp-session/src/lib.rs +++ b/crates/ironrdp-session/src/lib.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] // FIXME: remove + #[macro_use] extern crate tracing; @@ -18,7 +20,7 @@ use core::fmt; pub use active_stage::{ActiveStage, ActiveStageOutput}; -pub type SessionResult = std::result::Result; +pub type SessionResult = Result; #[non_exhaustive] #[derive(Debug)] @@ -84,7 +86,9 @@ impl SessionErrorExt for SessionError { } pub trait SessionResultExt { + #[must_use] fn with_context(self, context: &'static str) -> Self; + #[must_use] fn with_source(self, source: E) -> Self where E: std::error::Error + Sync + Send + 'static; diff --git a/crates/ironrdp-session/src/rfx.rs b/crates/ironrdp-session/src/rfx.rs index 8fd274d9e..48838981e 100644 --- a/crates/ironrdp-session/src/rfx.rs +++ b/crates/ironrdp-session/src/rfx.rs @@ -72,8 +72,8 @@ impl DecodingContext { Headers::CodecVersions(_) => (), } } - let context = context.ok_or(general_err!("context header is missing"))?; - let channels = channels.ok_or(general_err!("channels header is missing"))?; + let context = context.ok_or_else(|| general_err!("context header is missing"))?; + let channels = channels.ok_or_else(|| general_err!("channels header is missing"))?; if channels.0.is_empty() { return Err(general_err!("no RFX channel announced")); @@ -94,8 +94,8 @@ impl DecodingContext { input: &mut &[u8], ) -> SessionResult<(FrameId, InclusiveRectangle)> { let channel = self.channels.0.first().unwrap(); - let width = channel.width as u16; - let height = channel.height as u16; + let width = channel.width.as_u16(); + let height = channel.height.as_u16(); let entropy_algorithm = self.context.entropy_algorithm; let frame_begin = rfx::FrameBeginPdu::from_buffer_consume(input)?; @@ -155,9 +155,9 @@ impl DecodingContext { #[derive(Debug, Clone)] struct DecodingTileContext { - pub tile_output: Vec, - pub ycbcr_buffer: Vec>, - pub ycbcr_temp_buffer: Vec, + tile_output: Vec, + ycbcr_buffer: Vec>, + ycbcr_temp_buffer: Vec, } impl DecodingTileContext { @@ -255,8 +255,8 @@ fn map_tiles_data<'a>(tiles: &'_ [Tile<'a>], quants: &'_ [Quant]) -> Vec { - pub quants: [Quant; 3], - pub data: [&'a [u8]; 3], + quants: [Quant; 3], + data: [&'a [u8]; 3], } enum SequenceState { diff --git a/crates/ironrdp-session/src/x224/display.rs b/crates/ironrdp-session/src/x224/display.rs index 60299fb27..d60e165c9 100644 --- a/crates/ironrdp-session/src/x224/display.rs +++ b/crates/ironrdp-session/src/x224/display.rs @@ -4,7 +4,7 @@ use ironrdp_pdu::PduParsing; use super::DynamicChannelDataHandler; use crate::SessionResult; -pub struct Handler; +pub(crate) struct Handler; impl DynamicChannelDataHandler for Handler { fn process_complete_data(&mut self, complete_data: Vec) -> SessionResult>> { diff --git a/crates/ironrdp-session/src/x224/gfx.rs b/crates/ironrdp-session/src/x224/gfx.rs index 39dc500b4..75609e924 100644 --- a/crates/ironrdp-session/src/x224/gfx.rs +++ b/crates/ironrdp-session/src/x224/gfx.rs @@ -15,7 +15,7 @@ pub trait GfxHandler { fn on_message(&self, message: ServerPdu) -> SessionResult>; } -pub struct Handler { +pub(crate) struct Handler { decompressor: zgfx::Decompressor, decompressed_buffer: Vec, frames_decoded: u32, @@ -23,7 +23,7 @@ pub struct Handler { } impl Handler { - pub fn new(gfx_handler: Option>) -> Self { + pub(crate) fn new(gfx_handler: Option>) -> Self { Self { decompressor: zgfx::Decompressor::new(), decompressed_buffer: Vec::with_capacity(1024 * 16), @@ -96,7 +96,7 @@ bitflags! { } } -pub fn create_capabilities_advertise(graphics_config: &Option) -> SessionResult> { +pub(crate) fn create_capabilities_advertise(graphics_config: &Option) -> SessionResult> { let mut capabilities = Vec::new(); if let Some(config) = graphics_config { diff --git a/crates/ironrdp-session/src/x224/mod.rs b/crates/ironrdp-session/src/x224/mod.rs index 307dfff35..ce8add3f0 100644 --- a/crates/ironrdp-session/src/x224/mod.rs +++ b/crates/ironrdp-session/src/x224/mod.rs @@ -17,9 +17,11 @@ use ironrdp_svc::{ StaticChannelSet, StaticVirtualChannel, StaticVirtualChannelProcessor, SvcMessage, SvcProcessorMessages, }; -pub use self::gfx::GfxHandler; use crate::{SessionErrorExt as _, SessionResult}; +#[rustfmt::skip] +pub use self::gfx::GfxHandler; + pub const RDP8_GRAPHICS_PIPELINE_NAME: &str = "Microsoft::Windows::RDS::Graphics"; pub const RDP8_DISPLAY_PIPELINE_NAME: &str = "Microsoft::Windows::RDS::DisplayControl"; @@ -85,7 +87,7 @@ impl Processor { .get_channel_id_by_type::() .ok_or_else(|| reason_err!("SVC", "channel not found"))?; - self.process_svc_messages(messages.into(), channel_id, self.user_channel_id) + process_svc_messages(messages.into(), channel_id, self.user_channel_id) } /// Processes a received PDU. Returns a buffer with encoded data to send to the server, if any. @@ -103,7 +105,7 @@ impl Processor { _ => { if let Some(svc) = self.static_channels.get_by_channel_id_mut(channel_id) { let response_pdus = svc.process(data_ctx.user_data).map_err(crate::SessionError::pdu)?; - self.process_svc_messages(response_pdus, channel_id, data_ctx.initiator_id) + process_svc_messages(response_pdus, channel_id, data_ctx.initiator_id) } else { Err(reason_err!("X224", "unexpected channel received: ID {channel_id}")) } @@ -323,47 +325,42 @@ impl Processor { .map_err(crate::legacy::map_error)?; Ok(written) } +} - /// Processes a vector of [`SvcMessage`] in preparation for sending them to the server on the `channel_id` channel. - /// - /// This includes chunkifying the messages, adding MCS, x224, and tpkt headers, and encoding them into a buffer. - /// The messages returned here are ready to be sent to the server. - /// - /// The caller is responsible for ensuring that the `channel_id` corresponds to the correct channel. - fn process_svc_messages( - &self, - messages: Vec, - channel_id: u16, - initiator_id: u16, - ) -> SessionResult> { - // For each response PDU, chunkify it and add appropriate static channel headers. - let chunks = StaticVirtualChannel::chunkify(messages).map_err(crate::SessionError::pdu)?; - - // Place each chunk into a SendDataRequest - let mcs_pdus = chunks - .iter() - .map(|buf| mcs::SendDataRequest { - initiator_id, - channel_id, - user_data: Cow::Borrowed(buf.filled()), - }) - .collect::>(); - - // SendDataRequest is [`McsPdu`], which is [`x224Pdu`], which is [`PduEncode`]. [`PduEncode`] for [`x224Pdu`] - // also takes care of adding the Tpkt header, so therefore we can just call `encode_buf` on each of these and - // we will create a buffer of fully encoded PDUs ready to send to the server. - // - // For example, if we had 2 chunks, our fully_encoded_responses buffer would look like: - // - // [ | tpkt | x224 | mcs::SendDataRequest | chunk 1 | tpkt | x224 | mcs::SendDataRequest | chunk 2 | ] - // |<------------------- PDU 1 ------------------>|<------------------- PDU 2 ------------------>| - let mut fully_encoded_responses = WriteBuf::new(); // TODO(perf): reuse this buffer using `clear` and `filled` as appropriate - for pdu in mcs_pdus { - encode_buf(&pdu, &mut fully_encoded_responses).map_err(crate::SessionError::pdu)?; - } - - Ok(fully_encoded_responses.into_inner()) +/// Processes a vector of [`SvcMessage`] in preparation for sending them to the server on the `channel_id` channel. +/// +/// This includes chunkifying the messages, adding MCS, x224, and tpkt headers, and encoding them into a buffer. +/// The messages returned here are ready to be sent to the server. +/// +/// The caller is responsible for ensuring that the `channel_id` corresponds to the correct channel. +fn process_svc_messages(messages: Vec, channel_id: u16, initiator_id: u16) -> SessionResult> { + // For each response PDU, chunkify it and add appropriate static channel headers. + let chunks = StaticVirtualChannel::chunkify(messages).map_err(crate::SessionError::pdu)?; + + // Place each chunk into a SendDataRequest + let mcs_pdus = chunks + .iter() + .map(|buf| mcs::SendDataRequest { + initiator_id, + channel_id, + user_data: Cow::Borrowed(buf.filled()), + }) + .collect::>(); + + // SendDataRequest is [`McsPdu`], which is [`x224Pdu`], which is [`PduEncode`]. [`PduEncode`] for [`x224Pdu`] + // also takes care of adding the Tpkt header, so therefore we can just call `encode_buf` on each of these and + // we will create a buffer of fully encoded PDUs ready to send to the server. + // + // For example, if we had 2 chunks, our fully_encoded_responses buffer would look like: + // + // [ | tpkt | x224 | mcs::SendDataRequest | chunk 1 | tpkt | x224 | mcs::SendDataRequest | chunk 2 | ] + // |<------------------- PDU 1 ------------------>|<------------------- PDU 2 ------------------>| + let mut fully_encoded_responses = WriteBuf::new(); // TODO(perf): reuse this buffer using `clear` and `filled` as appropriate + for pdu in mcs_pdus { + encode_buf(&pdu, &mut fully_encoded_responses).map_err(crate::SessionError::pdu)?; } + + Ok(fully_encoded_responses.into_inner()) } fn create_dvc( diff --git a/crates/ironrdp-svc/README.md b/crates/ironrdp-svc/README.md new file mode 100644 index 000000000..5fa66f08e --- /dev/null +++ b/crates/ironrdp-svc/README.md @@ -0,0 +1,3 @@ +# IronRDP SVC + +IronRDP traits to implement RDP static virtual channels. \ No newline at end of file diff --git a/crates/ironrdp-svc/src/lib.rs b/crates/ironrdp-svc/src/lib.rs index 884a71a45..d4b45c632 100644 --- a/crates/ironrdp-svc/src/lib.rs +++ b/crates/ironrdp-svc/src/lib.rs @@ -1,3 +1,6 @@ +#![doc = include_str!("../README.md")] +// TODO: #![warn(missing_docs)] + extern crate alloc; // Re-export ironrdp_pdu crate for convenience @@ -60,6 +63,7 @@ pub struct SvcMessage { impl SvcMessage { /// Adds additional SVC header flags to the message. + #[must_use] pub fn with_flags(mut self, flags: ChannelFlags) -> Self { self.flags |= flags; self @@ -179,7 +183,7 @@ struct ChunkProcessor { } impl ChunkProcessor { - pub fn new() -> Self { + fn new() -> Self { Self { chunked_pdu: Vec::new(), } @@ -188,6 +192,7 @@ impl ChunkProcessor { /// Takes a vector of PDUs and breaks them into chunks prefixed with a Channel PDU Header (`CHANNEL_PDU_HEADER`). /// /// Each chunk is at most `max_chunk_len` bytes long (not including the Channel PDU Header). + #[allow(clippy::unused_self)] // For symmetry with `dechunkify` fn chunkify(messages: Vec, max_chunk_len: usize) -> PduResult> { let mut results = Vec::new(); for message in messages { @@ -202,7 +207,7 @@ impl ChunkProcessor { /// For chunked payloads, returns `Ok(None)` until the last chunk is received, at which point /// it returns `Ok(Some(payload))`. fn dechunkify(&mut self, mut payload: &[u8]) -> PduResult>> { - let last = self.process_header(&mut payload)?; + let last = Self::process_header(&mut payload)?; // Extend the chunked_pdu buffer with the payload self.chunked_pdu.extend_from_slice(payload); @@ -218,7 +223,7 @@ impl ChunkProcessor { } /// Returns whether this was the last chunk based on the flags in the channel header. - fn process_header(&self, payload: &mut &[u8]) -> PduResult { + fn process_header(payload: &mut &[u8]) -> PduResult { let channel_header = ironrdp_pdu::rdp::vc::ChannelPduHeader::from_buffer(payload) .map_err(|e| custom_err!("failed to decode svc channel header", e))?; Ok(channel_header.flags.contains(ChannelControlFlags::FLAG_LAST)) @@ -284,7 +289,7 @@ impl ChunkProcessor { // Otherwise, update the chunk start and end indices for the next iteration. chunk_start_index = chunk_end_index; - chunk_end_index = std::cmp::min(total_len, chunk_end_index + max_chunk_len); + chunk_end_index = std::cmp::min(total_len, chunk_end_index.saturating_add(max_chunk_len)); } Ok(chunks) @@ -510,23 +515,23 @@ bitflags! { #[derive(Debug, PartialEq, Copy, Clone)] pub struct ChannelFlags: u32 { /// CHANNEL_FLAG_FIRST - const FIRST = 0x00000001; + const FIRST = 0x0000_0001; /// CHANNEL_FLAG_LAST - const LAST = 0x00000002; + const LAST = 0x0000_0002; /// CHANNEL_FLAG_SHOW_PROTOCOL - const SHOW_PROTOCOL = 0x00000010; + const SHOW_PROTOCOL = 0x0000_0010; /// CHANNEL_FLAG_SUSPEND - const SUSPEND = 0x00000020; + const SUSPEND = 0x0000_0020; /// CHANNEL_FLAG_RESUME - const RESUME = 0x00000040; + const RESUME = 0x0000_0040; /// CHANNEL_FLAG_SHADOW_PERSISTENT - const SHADOW_PERSISTENT = 0x00000080; + const SHADOW_PERSISTENT = 0x0000_0080; /// CHANNEL_PACKET_COMPRESSED - const COMPRESSED = 0x00200000; + const COMPRESSED = 0x0020_0000; /// CHANNEL_PACKET_AT_FRONT - const AT_FRONT = 0x00400000; + const AT_FRONT = 0x0040_0000; /// CHANNEL_PACKET_FLUSHED - const FLUSHED = 0x00800000; + const FLUSHED = 0x0080_0000; } } @@ -564,6 +569,7 @@ impl PduEncode for ChannelPduHeader { Self::NAME } + #[allow(clippy::arithmetic_side_effects)] fn size(&self) -> usize { std::mem::size_of::() * 2 } diff --git a/crates/ironrdp-testsuite-core/Cargo.toml b/crates/ironrdp-testsuite-core/Cargo.toml index 77a98e8a9..2e41256cf 100644 --- a/crates/ironrdp-testsuite-core/Cargo.toml +++ b/crates/ironrdp-testsuite-core/Cargo.toml @@ -16,9 +16,7 @@ path = "tests/main.rs" harness = true [dependencies] -anyhow = "1" array-concat = "0.5.2" -expect-test.workspace = true ironrdp-pdu.workspace = true lazy_static = "1.4.0" paste = "1" @@ -37,3 +35,5 @@ pretty_assertions = "1.3.0" proptest.workspace = true rdp-rs = { git = "https://github.com/citronneur/rdp-rs", rev = "7ac880d7efb7f05efef3c84476f7c24f4053e0ea" } rstest.workspace = true +expect-test.workspace = true +anyhow = "1" diff --git a/crates/ironrdp-testsuite-core/src/lib.rs b/crates/ironrdp-testsuite-core/src/lib.rs index 5f60e34d4..f8695ab58 100644 --- a/crates/ironrdp-testsuite-core/src/lib.rs +++ b/crates/ironrdp-testsuite-core/src/lib.rs @@ -1,3 +1,10 @@ +// No need to be as strict as in production libraries +#![allow(clippy::arithmetic_side_effects)] +#![allow(clippy::cast_lossless)] +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] + #[macro_use] extern crate array_concat; #[macro_use] diff --git a/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs b/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs index d22cfad2b..6f801880c 100644 --- a/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs +++ b/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs @@ -253,8 +253,8 @@ const FRAME_END_PDU: FrameEndPdu = FrameEndPdu; lazy_static::lazy_static! { static ref CHANNELS_PDU: ChannelsPdu = ChannelsPdu(vec![ - Channel { width: 64, height: 64 }, - Channel { width: 32, height: 32 } + RfxChannel { width: 64, height: 64 }, + RfxChannel { width: 32, height: 32 } ]); static ref REGION_PDU: RegionPdu = RegionPdu { rectangles: vec![ diff --git a/crates/ironrdp-tokio/Cargo.toml b/crates/ironrdp-tokio/Cargo.toml index 6e15cabe2..a2b4f8c86 100644 --- a/crates/ironrdp-tokio/Cargo.toml +++ b/crates/ironrdp-tokio/Cargo.toml @@ -18,5 +18,4 @@ test = false [dependencies] bytes = "1" ironrdp-async.workspace = true -tokio-util = { version = "0.7.7", features = ["codec"] } tokio = { version = "1", features = ["io-util"] } diff --git a/crates/ironrdp-web/src/canvas.rs b/crates/ironrdp-web/src/canvas.rs index 6fccdd731..b44995f92 100644 --- a/crates/ironrdp-web/src/canvas.rs +++ b/crates/ironrdp-web/src/canvas.rs @@ -3,13 +3,13 @@ use std::num::NonZeroU32; use ironrdp::pdu::geometry::{InclusiveRectangle, Rectangle as _}; use web_sys::HtmlCanvasElement; -pub struct Canvas { +pub(crate) struct Canvas { width: u32, surface: softbuffer::Surface, } impl Canvas { - pub fn new(render_canvas: HtmlCanvasElement, width: u32, height: u32) -> anyhow::Result { + pub(crate) fn new(render_canvas: HtmlCanvasElement, width: u32, height: u32) -> anyhow::Result { render_canvas.set_width(width); render_canvas.set_height(height); @@ -35,7 +35,7 @@ impl Canvas { Ok(Self { width, surface }) } - pub fn draw(&mut self, buffer: &[u8], region: InclusiveRectangle) -> anyhow::Result<()> { + pub(crate) fn draw(&mut self, buffer: &[u8], region: InclusiveRectangle) -> anyhow::Result<()> { let region_width = region.width(); let region_height = region.height(); diff --git a/crates/ironrdp-web/src/image.rs b/crates/ironrdp-web/src/image.rs index 876ee8cca..e4896bd9d 100644 --- a/crates/ironrdp-web/src/image.rs +++ b/crates/ironrdp-web/src/image.rs @@ -1,7 +1,9 @@ +#![allow(clippy::arithmetic_side_effects)] + use ironrdp::pdu::geometry::{InclusiveRectangle, Rectangle as _}; use ironrdp::session::image::DecodedImage; -pub fn extract_partial_image(image: &DecodedImage, region: InclusiveRectangle) -> (InclusiveRectangle, Vec) { +pub(crate) fn extract_partial_image(image: &DecodedImage, region: InclusiveRectangle) -> (InclusiveRectangle, Vec) { // PERF: needs actual benchmark to find a better heuristic if region.height() > 64 || region.width() > 512 { extract_whole_rows(image, region) diff --git a/crates/ironrdp-web/src/lib.rs b/crates/ironrdp-web/src/lib.rs index cea3cc85a..7ba735a04 100644 --- a/crates/ironrdp-web/src/lib.rs +++ b/crates/ironrdp-web/src/lib.rs @@ -1,4 +1,6 @@ #![allow(clippy::new_without_default)] // Default trait can’t be used by wasm consumer anyway +#![allow(unsafe_op_in_unsafe_fn)] // We can’t control code generated by `wasm-bindgen` +#![allow(unused_crate_dependencies)] // Some crates are added solely so we can enable additionnal features #[macro_use] extern crate tracing; diff --git a/crates/ironrdp-web/src/network_client.rs b/crates/ironrdp-web/src/network_client.rs index cd12f7ed3..61fb055cf 100644 --- a/crates/ironrdp-web/src/network_client.rs +++ b/crates/ironrdp-web/src/network_client.rs @@ -60,7 +60,7 @@ impl NetworkClient for WasmNetworkClient { Self::NAME } - fn supported_protocols(&self) -> &[sspi::network_client::NetworkProtocol] { + fn supported_protocols(&self) -> &[NetworkProtocol] { Self::SUPPORTED_PROTOCOLS } diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 98ee3bb27..1226bd5f1 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -361,9 +361,8 @@ impl Session { for out in outputs { match out { ActiveStageOutput::ResponseFrame(frame) => { - // PERF: unnecessary copy self.writer_tx - .unbounded_send(frame.to_vec()) + .unbounded_send(frame) .context("Send frame to writer task")?; } ActiveStageOutput::GraphicsUpdate(region) => { @@ -413,10 +412,7 @@ impl Session { self.h_send_inputs(inputs) } - fn h_send_inputs( - &self, - inputs: smallvec::SmallVec<[ironrdp::pdu::input::fast_path::FastPathInputEvent; 2]>, - ) -> Result<(), IronRdpError> { + fn h_send_inputs(&self, inputs: smallvec::SmallVec<[FastPathInputEvent; 2]>) -> Result<(), IronRdpError> { if !inputs.is_empty() { trace!("Inputs: {inputs:?}"); @@ -451,6 +447,7 @@ impl Session { Ok(()) } + #[allow(clippy::unused_self)] // FIXME: not yet implemented pub fn shutdown(&self) -> Result<(), IronRdpError> { // TODO: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/27915739-8f77-487e-9927-55008af7fd68 Ok(()) @@ -483,6 +480,7 @@ fn build_config( color_depth: 32, lossy_compression: true, }), + #[allow(clippy::arithmetic_side_effects)] // fine unless we ending 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) @@ -555,9 +553,9 @@ where use x509_cert::der::Decode as _; #[derive(Clone, Copy, Debug)] - pub struct RDCleanPathHint; + struct RDCleanPathHint; - pub const RDCLEANPATH_HINT: RDCleanPathHint = RDCleanPathHint; + const RDCLEANPATH_HINT: RDCleanPathHint = RDCleanPathHint; impl ironrdp::pdu::PduHint for RDCleanPathHint { fn find_size(&self, bytes: &[u8]) -> ironrdp::pdu::PduResult> { diff --git a/xtask/src/check.rs b/xtask/src/check.rs index 0bea091e6..df76245f2 100644 --- a/xtask/src/check.rs +++ b/xtask/src/check.rs @@ -1,5 +1,80 @@ use crate::prelude::*; +const EXTRA_LINTS: &[&str] = &[ + // == Safer unsafe == // + "unsafe_op_in_unsafe_fn", + "invalid_reference_casting", + "pointer_structural_match", + "clippy::undocumented_unsafe_blocks", + "clippy::multiple_unsafe_ops_per_block", + "clippy::transmute_ptr_to_ptr", + "clippy::as_ptr_cast_mut", + "clippy::cast_ptr_alignment", + "clippy::fn_to_numeric_cast_any", + "clippy::ptr_cast_constness", + // == Correctness == // + "unused_tuple_struct_fields", + "clippy::arithmetic_side_effects", + "clippy::cast_lossless", + "clippy::cast_possible_truncation", + "clippy::cast_possible_wrap", + "clippy::cast_sign_loss", + "clippy::float_cmp", + "clippy::as_underscore", + // TODO: "clippy::unwrap_used", // let’s either handle `None`, `Err` or use `expect` to give a reason + "clippy::large_stack_frames", + // == Style, readability == // + "absolute_paths_not_starting_with_crate", + "single_use_lifetimes", + "unreachable_pub", + "unused_lifetimes", + "unused_qualifications", + "keyword_idents", + "noop_method_call", + "clippy::semicolon_outside_block", // with semicolon-outside-block-ignore-multiline = true + "clippy::clone_on_ref_ptr", + "clippy::cloned_instead_of_copied", + "clippy::trait_duplication_in_bounds", + "clippy::type_repetition_in_bounds", + "clippy::checked_conversions", + "clippy::get_unwrap", + // TODO: "clippy::similar_names", // reduce risk of confusing similar names together, and check for typos when shadowing variables + "clippy::str_to_string", + "clippy::string_to_string", + // TODO: "clippy::std_instead_of_alloc", + // TODO: "clippy::std_instead_of_core", + "clippy::unreadable_literal", + "clippy::separated_literal_suffix", + "clippy::unused_self", + // TODO: "clippy::use_self", // NOTE(@CBenoit): not sure about that one + "clippy::useless_let_if_seq", + // TODO: "clippy::partial_pub_fields", + "clippy::string_add", + "clippy::range_plus_one", + // TODO: "missing_docs" // NOTE(@CBenoit): we probably want to ensure this in core tier crates only + // == Compile-time / optimization == // + "unused_crate_dependencies", + "unused_macro_rules", + "clippy::inline_always", + "clippy::or_fun_call", + "clippy::unnecessary_box_returns", + // == Extra-pedantic clippy == // + "clippy::collection_is_never_read", + "clippy::copy_iterator", + "clippy::expl_impl_clone_on_copy", + "clippy::implicit_clone", + "clippy::large_types_passed_by_value", + "clippy::redundant_clone", + "clippy::string_to_string", + "clippy::alloc_instead_of_core", + "clippy::empty_drop", + "clippy::return_self_not_must_use", + "clippy::wildcard_dependencies", + // == Let’s not merge unintended eprint!/print! statements in libraries == // + "clippy::print_stderr", + "clippy::print_stdout", +]; + pub fn fmt(sh: &Shell) -> anyhow::Result<()> { let _s = Section::new("FORMATTING"); @@ -16,8 +91,13 @@ pub fn fmt(sh: &Shell) -> anyhow::Result<()> { pub fn lints(sh: &Shell) -> anyhow::Result<()> { let _s = Section::new("LINTS"); - cmd!(sh, "{CARGO} clippy --workspace --locked -- -D warnings").run()?; + + let cmd = cmd!(sh, "{CARGO} clippy --workspace --locked -- -D warnings"); + + EXTRA_LINTS.iter().fold(cmd, |cmd, lint| cmd.args(["-W", lint])).run()?; + println!("All good!"); + Ok(()) } diff --git a/xtask/src/cov.rs b/xtask/src/cov.rs index 99c7aee9c..25aa62501 100644 --- a/xtask/src/cov.rs +++ b/xtask/src/cov.rs @@ -117,7 +117,7 @@ pub fn report_github(sh: &Shell, repo: &str, pr_id: u32) -> anyhow::Result<()> { let body = comment["body"].get::().context("comment body")?; if body.starts_with(COMMENT_HEADER) { - let comment_id = *comment["id"].get::().context("id")? as u64; + let comment_id = get_json_int(comment, "id")?; prev_comment_id = Some(comment_id); break; } @@ -280,9 +280,9 @@ struct CoverageReport { impl CoverageReport { fn from_json_value(lines: &tinyjson::JsonValue) -> anyhow::Result { - let total_lines = *lines["count"].get::().context("invalid value for `count`")? as u64; - let covered_lines = *lines["covered"].get::().context("invalid value for `covered`")? as u64; - let covered_lines_percent = *lines["percent"].get::().context("invalid value for `covered`")?; + let total_lines = get_json_int(lines, "count")?; + let covered_lines = get_json_int(lines, "covered")?; + let covered_lines_percent = get_json_float(lines, "percent")?; let original_json_data = lines.stringify().context("original json data")?; @@ -334,3 +334,17 @@ impl fmt::Display for CoverageReport { Ok(()) } } + +fn get_json_float(value: &tinyjson::JsonValue, key: &str) -> anyhow::Result { + value[key] + .get::() + .copied() + .with_context(|| format!("invalid value for `{key}`")) +} + +fn get_json_int(value: &tinyjson::JsonValue, key: &str) -> anyhow::Result { + // tinyjson does not expose any integers at all, so we need the f64 to u64 as casting + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + get_json_float(value, key).map(|value| value as u64) +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 475d43653..1c6bbaaa4 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -1,3 +1,8 @@ +#![allow(clippy::print_stdout)] +#![allow(clippy::print_stderr)] +#![allow(clippy::unwrap_used)] +#![allow(unreachable_pub)] + #[macro_use] mod macros; diff --git a/xtask/src/section.rs b/xtask/src/section.rs index 347f9ca5d..cf5ce59b7 100644 --- a/xtask/src/section.rs +++ b/xtask/src/section.rs @@ -10,7 +10,7 @@ impl Section { flush_all(); eprintln!("::group::{name}"); let start = Instant::now(); - Section { name, start } + Self { name, start } } } From 702441a64e21b7728421962a6cd670e9f1043583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 22:39:01 -0400 Subject: [PATCH 02/14] Elaborate comment on clippy::similar_names --- xtask/src/check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xtask/src/check.rs b/xtask/src/check.rs index df76245f2..5b28fb7c5 100644 --- a/xtask/src/check.rs +++ b/xtask/src/check.rs @@ -38,7 +38,7 @@ const EXTRA_LINTS: &[&str] = &[ "clippy::type_repetition_in_bounds", "clippy::checked_conversions", "clippy::get_unwrap", - // TODO: "clippy::similar_names", // reduce risk of confusing similar names together, and check for typos when shadowing variables + // TODO: "clippy::similar_names", // reduce risk of confusing similar names together, and protects against typos when variable shadowing was intended "clippy::str_to_string", "clippy::string_to_string", // TODO: "clippy::std_instead_of_alloc", From de46fc5b93ce98696a421bc9cca53401081297fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 22:43:24 -0400 Subject: [PATCH 03/14] Enable lint against dbg! --- xtask/src/check.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/xtask/src/check.rs b/xtask/src/check.rs index 5b28fb7c5..2ad235649 100644 --- a/xtask/src/check.rs +++ b/xtask/src/check.rs @@ -73,6 +73,7 @@ const EXTRA_LINTS: &[&str] = &[ // == Let’s not merge unintended eprint!/print! statements in libraries == // "clippy::print_stderr", "clippy::print_stdout", + "clippy::dbg_macro", ]; pub fn fmt(sh: &Shell) -> anyhow::Result<()> { From f6a45f6c9730ad7065b5918e26b5ae3b32a6653d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 22:47:54 -0400 Subject: [PATCH 04/14] Remove redundant lint --- xtask/src/check.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/xtask/src/check.rs b/xtask/src/check.rs index 2ad235649..1af362d19 100644 --- a/xtask/src/check.rs +++ b/xtask/src/check.rs @@ -65,7 +65,6 @@ const EXTRA_LINTS: &[&str] = &[ "clippy::implicit_clone", "clippy::large_types_passed_by_value", "clippy::redundant_clone", - "clippy::string_to_string", "clippy::alloc_instead_of_core", "clippy::empty_drop", "clippy::return_self_not_must_use", From e6a20844d2f663cb14c823110c4e9a51273515af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 23:04:36 -0400 Subject: [PATCH 05/14] fix typos --- .../src/windows/clipboard_impl.rs | 6 +++--- crates/ironrdp-cliprdr/src/lib.rs | 10 ++++++---- crates/ironrdp-web/src/lib.rs | 7 ++++++- crates/ironrdp-web/src/session.rs | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs index 6882a8b82..cc38c1310 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs @@ -295,13 +295,13 @@ pub(crate) unsafe extern "system" fn clipboard_subproc( if msg == WM_DESTROY { // Transfer ownership and drop previously allocated context - // SAFETY: `data` is a valid pointer, returned by `Box::into_raw`, transfered to OS earlier + // SAFETY: `data` is a valid pointer, returned by `Box::into_raw`, transferred to OS earlier // via `SetWindowSubclass` call. let _ = unsafe { Box::from_raw(data as *mut WinClipboardImpl) }; return LRESULT(0); } - // SAFETY: `data` is a valid pointer, returned by `Box::into_raw`, transfered to OS earlier + // SAFETY: `data` is a valid pointer, returned by `Box::into_raw`, transferred to OS earlier // via `SetWindowSubclass` call. let ctx = unsafe { &mut *(data as *mut WinClipboardImpl) }; @@ -360,7 +360,7 @@ pub(crate) unsafe extern "system" fn clipboard_subproc( _ => { // Call next event handler in the subclass chain - // SAFETY: `DefSubclassProc` is always safe to call in conext of subclass event loop + // SAFETY: `DefSubclassProc` is always safe to call in context of subclass event loop return unsafe { DefSubclassProc(hwnd, msg, wparam, lparam) }; } }; diff --git a/crates/ironrdp-cliprdr/src/lib.rs b/crates/ironrdp-cliprdr/src/lib.rs index e06bf0821..f3cb52949 100644 --- a/crates/ironrdp-cliprdr/src/lib.rs +++ b/crates/ironrdp-cliprdr/src/lib.rs @@ -55,12 +55,14 @@ pub struct Cliprdr { impl_as_any!(Cliprdr); macro_rules! ready_guard { - ($self:ident, $function:ident) => { + ($self:ident, $function:ident) => {{ + let _ = Self::$function; // ensure the function actually exists + if $self.state != CliprdrState::Ready { error!(?$self.state, concat!("Attempted to initiate ", stringify!($function), " in incorrect state")); return Ok(Vec::new().into()); } - }; + }}; } impl Cliprdr { @@ -147,7 +149,7 @@ impl Cliprdr { /// /// If data is not available anymore, an error response should be sent instead. pub fn submit_format_data(&self, response: FormatDataResponse<'static>) -> PduResult { - ready_guard!(self, sumbit_format_data); + ready_guard!(self, submit_format_data); let pdu = ClipboardPdu::FormatDataResponse(response); @@ -162,7 +164,7 @@ impl Cliprdr { /// /// If data is not available anymore, an error response should be sent instead. pub fn submit_file_contents(&self, response: FileContentsResponse<'static>) -> PduResult { - ready_guard!(self, sumbit_file_contents); + ready_guard!(self, submit_file_contents); let pdu = ClipboardPdu::FileContentsResponse(response); diff --git a/crates/ironrdp-web/src/lib.rs b/crates/ironrdp-web/src/lib.rs index 7ba735a04..ec4b39bf6 100644 --- a/crates/ironrdp-web/src/lib.rs +++ b/crates/ironrdp-web/src/lib.rs @@ -1,6 +1,11 @@ #![allow(clippy::new_without_default)] // Default trait can’t be used by wasm consumer anyway #![allow(unsafe_op_in_unsafe_fn)] // We can’t control code generated by `wasm-bindgen` -#![allow(unused_crate_dependencies)] // Some crates are added solely so we can enable additionnal features + +// Silence the unused_crate_dependencies lint +// These crates are added just to enable additional WASM features +extern crate chrono as _; +extern crate getrandom as _; +extern crate time as _; #[macro_use] extern crate tracing; diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 1226bd5f1..807996884 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -480,7 +480,7 @@ fn build_config( color_depth: 32, lossy_compression: true, }), - #[allow(clippy::arithmetic_side_effects)] // fine unless we ending with an insanely big version + #[allow(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) From 96906ca37c649d36cd88a1828e13a2b388aa2054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 23:16:19 -0400 Subject: [PATCH 06/14] usize::from instead of as usize --- crates/ironrdp-session/src/fast_path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ironrdp-session/src/fast_path.rs b/crates/ironrdp-session/src/fast_path.rs index c15507f25..b430aa38b 100644 --- a/crates/ironrdp-session/src/fast_path.rs +++ b/crates/ironrdp-session/src/fast_path.rs @@ -216,7 +216,7 @@ impl Processor { let _ = self .pointer_cache - .insert(cache_index as usize, Rc::clone(&decoded_pointer)); + .insert(usize::from(cache_index), Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); @@ -225,7 +225,7 @@ impl Processor { PointerUpdateData::Cached(cached) => { let cache_index = cached.cache_index; - if let Some(cached_pointer) = self.pointer_cache.get(cache_index as usize) { + if let Some(cached_pointer) = self.pointer_cache.get(usize::from(cache_index)) { // Disable system pointer processor_updates.push(UpdateKind::PointerHidden); self.use_system_pointer = false; @@ -252,7 +252,7 @@ impl Processor { let _ = self .pointer_cache - .insert(cache_index as usize, Rc::clone(&decoded_pointer)); + .insert(usize::from(cache_index), Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); @@ -268,7 +268,7 @@ impl Processor { let _ = self .pointer_cache - .insert(cache_index as usize, Rc::clone(&decoded_pointer)); + .insert(usize::from(cache_index), Rc::clone(&decoded_pointer)); if let Some(rect) = image.update_pointer(decoded_pointer)? { processor_updates.push(UpdateKind::Region(rect)); From ef1b7205e8841b2c05f01a8f022357d6a5e2f653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 28 Sep 2023 23:26:19 -0400 Subject: [PATCH 07/14] Fix tests --- crates/ironrdp-pdu/src/codecs/rfx.rs | 8 +++++-- .../src/codecs/rfx/header_messages.rs | 24 ++++++++++++------- .../ironrdp-testsuite-core/tests/pdu/rfx.rs | 4 ++-- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/ironrdp-pdu/src/codecs/rfx.rs b/crates/ironrdp-pdu/src/codecs/rfx.rs index f9455d087..c32c697d4 100644 --- a/crates/ironrdp-pdu/src/codecs/rfx.rs +++ b/crates/ironrdp-pdu/src/codecs/rfx.rs @@ -8,12 +8,16 @@ use num_derive::{FromPrimitive, ToPrimitive}; use num_traits::{FromPrimitive as _, ToPrimitive as _}; use thiserror::Error; +use crate::{PduBufferParsing, PduParsing}; + +#[rustfmt::skip] pub use self::data_messages::{ ContextPdu, EntropyAlgorithm, FrameBeginPdu, FrameEndPdu, OperatingMode, Quant, RegionPdu, RfxRectangle, Tile, TileSetPdu, }; -pub use self::header_messages::{ChannelsPdu, CodecVersionsPdu, RfxChannel, SyncPdu}; -use crate::{PduBufferParsing, PduParsing}; +pub use self::header_messages::{ + ChannelsPdu, CodecVersionsPdu, RfxChannel, RfxChannelHeight, RfxChannelWidth, SyncPdu, +}; const BLOCK_HEADER_SIZE: usize = 6; const CODEC_CHANNEL_HEADER_SIZE: usize = 2; diff --git a/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs b/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs index b91bfd589..07754c7da 100644 --- a/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs +++ b/crates/ironrdp-pdu/src/codecs/rfx/header_messages.rs @@ -164,6 +164,13 @@ pub struct RfxChannel { pub struct RfxChannelWidth(i16); impl RfxChannelWidth { + pub fn new(value: i16) -> Result { + (1..=4096) + .contains(&value) + .then_some(Self(value)) + .ok_or(RfxError::InvalidChannelWidth(value)) + } + pub fn as_u16(self) -> u16 { u16::try_from(self.0).expect("integer within the range of 1 to 4096") } @@ -179,6 +186,13 @@ impl RfxChannelWidth { pub struct RfxChannelHeight(i16); impl RfxChannelHeight { + pub fn new(value: i16) -> Result { + (1..=2048) + .contains(&value) + .then_some(Self(value)) + .ok_or(RfxError::InvalidChannelWidth(value)) + } + pub fn as_u16(self) -> u16 { u16::try_from(self.0).expect("integer within the range of 1 to 2048") } @@ -198,16 +212,10 @@ impl PduBufferParsing<'_> for RfxChannel { } let width = buffer.read_i16::()?; - if width < 1 && width > 4096 { - return Err(RfxError::InvalidChannelWidth(width)); - } - let width = RfxChannelWidth(width); + let width = RfxChannelWidth::new(width)?; let height = buffer.read_i16::()?; - if height < 1 && height > 2048 { - return Err(RfxError::InvalidChannelHeight(height)); - } - let height = RfxChannelHeight(height); + let height = RfxChannelHeight::new(height)?; Ok(Self { width, height }) } diff --git a/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs b/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs index 6f801880c..a621af286 100644 --- a/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs +++ b/crates/ironrdp-testsuite-core/tests/pdu/rfx.rs @@ -253,8 +253,8 @@ const FRAME_END_PDU: FrameEndPdu = FrameEndPdu; lazy_static::lazy_static! { static ref CHANNELS_PDU: ChannelsPdu = ChannelsPdu(vec![ - RfxChannel { width: 64, height: 64 }, - RfxChannel { width: 32, height: 32 } + RfxChannel { width: RfxChannelWidth::new(64).unwrap(), height: RfxChannelHeight::new(64).unwrap() }, + RfxChannel { width: RfxChannelWidth::new(32).unwrap(), height: RfxChannelHeight::new(32).unwrap() } ]); static ref REGION_PDU: RegionPdu = RegionPdu { rectangles: vec![ From 866f1b1de7b64c3b60ef9fdbf3580f98e4e43033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 00:02:06 -0400 Subject: [PATCH 08/14] Fix lints for windows build --- Cargo.lock | 1 - crates/ironrdp-cliprdr-native/Cargo.toml | 1 - crates/ironrdp-cliprdr-native/src/windows.rs | 5 +-- .../src/windows/clipboard_data_ref.rs | 8 ++--- .../src/windows/clipboard_impl.rs | 35 +++++++++++-------- .../src/windows/cliprdr_backend.rs | 2 +- .../src/windows/os_clipboard.rs | 17 +++++---- .../src/windows/remote_format_registry.rs | 8 ++--- .../src/windows/utils.rs | 6 ++-- 9 files changed, 46 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc8f034eb..d1b325d79 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,7 +1890,6 @@ name = "ironrdp-cliprdr-native" version = "0.1.0" dependencies = [ "ironrdp-cliprdr", - "ironrdp-svc", "thiserror", "tracing", "windows 0.48.0", diff --git a/crates/ironrdp-cliprdr-native/Cargo.toml b/crates/ironrdp-cliprdr-native/Cargo.toml index 2114df012..df33d80c2 100644 --- a/crates/ironrdp-cliprdr-native/Cargo.toml +++ b/crates/ironrdp-cliprdr-native/Cargo.toml @@ -18,7 +18,6 @@ test = false [target.'cfg(windows)'.dependencies] ironrdp-cliprdr.workspace = true -ironrdp-svc.workspace = true thiserror.workspace = true tracing.workspace = true windows = { version = "0.48.0", features = [ diff --git a/crates/ironrdp-cliprdr-native/src/windows.rs b/crates/ironrdp-cliprdr-native/src/windows.rs index df7bd7591..55ffc63f0 100644 --- a/crates/ironrdp-cliprdr-native/src/windows.rs +++ b/crates/ironrdp-cliprdr-native/src/windows.rs @@ -142,7 +142,8 @@ impl WinClipboard { // // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. - let winapi_result = unsafe { SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as _) }; + let winapi_result = + unsafe { SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as usize) }; if winapi_result == FALSE { return Err(WinCliprdrError::WindowSubclass); @@ -177,7 +178,7 @@ impl Drop for WinClipboard { } /// Windows-specific clipboard backend factory -pub struct WinCliprdrBackendFactory { +struct WinCliprdrBackendFactory { tx: mpsc_sync::SyncSender, window: HWND, } diff --git a/crates/ironrdp-cliprdr-native/src/windows/clipboard_data_ref.rs b/crates/ironrdp-cliprdr-native/src/windows/clipboard_data_ref.rs index 80b8b9357..5b31890f1 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/clipboard_data_ref.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/clipboard_data_ref.rs @@ -6,7 +6,7 @@ use windows::Win32::System::Memory::{GlobalLock, GlobalSize, GlobalUnlock}; use crate::windows::os_clipboard::OwnedOsClipboard; /// Wrapper for global clipboard data handle ready for reading. -pub struct ClipboardDataRef<'a> { +pub(crate) struct ClipboardDataRef<'a> { _os_clipboard: &'a OwnedOsClipboard, handle: HGLOBAL, data: *const u8, @@ -15,7 +15,7 @@ pub struct ClipboardDataRef<'a> { impl<'a> ClipboardDataRef<'a> { /// Get new clipboard data from the clipboard. If no data is available for the specified /// format, or handle can't be locked, returns `None`. - pub fn get(os_clipboard: &'a OwnedOsClipboard, format: ClipboardFormatId) -> Option { + pub(crate) fn get(os_clipboard: &'a OwnedOsClipboard, format: ClipboardFormatId) -> Option { // SAFETY: it is safe to call `GetClipboardData`, because we own the clipboard // before calling this function. let handle = match unsafe { GetClipboardData(format.value()) } { @@ -47,13 +47,13 @@ impl<'a> ClipboardDataRef<'a> { /// /// E.g. for `CF_TEXT` /// format it's required to search for null-terminator to get the actual size of the string. - pub fn size(&self) -> usize { + pub(crate) fn size(&self) -> usize { // SAFETY: We always own non-null handle, so it is safe to call `GlobalSize` on it unsafe { GlobalSize(self.handle) } } /// Pointer to the data buffer available for reading. - pub fn data(&self) -> &[u8] { + pub(crate) fn data(&self) -> &[u8] { let size = self.size(); // SAFETY: `data` pointer is valid during the lifetime of the wrapper unsafe { std::slice::from_raw_parts(self.data, size) } diff --git a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs index cc38c1310..ebdbf2e2d 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs @@ -24,17 +24,17 @@ const IDT_CLIPBOARD_RETRY: usize = 1; /// Internal implementation of the clipboard processing logic. pub(crate) struct WinClipboardImpl { - pub window: HWND, - pub message_proxy: Box, - pub window_is_active: bool, - pub backend_rx: mpsc_sync::Receiver, + window: HWND, + message_proxy: Box, + window_is_active: bool, + backend_rx: mpsc_sync::Receiver, // Number of attempts spent to process current clipboard message - pub attempt: usize, + attempt: u32, // Message to retry - pub retry_message: Option, + retry_message: Option, // Formats available on the remote (represented as LOCAL format ids) - pub available_formats_on_remote: Vec, - pub remote_format_registry: RemoteClipboardFormatRegistry, + available_formats_on_remote: Vec, + remote_format_registry: RemoteClipboardFormatRegistry, } impl WinClipboardImpl { @@ -56,7 +56,6 @@ impl WinClipboardImpl { } fn on_format_data_response( - &mut self, requested_local_format: ClipboardFormatId, response: &FormatDataResponse, ) -> WinCliprdrResult<()> { @@ -71,6 +70,7 @@ impl WinClipboardImpl { unsafe { render_format(requested_local_format, response.data())?; } + Ok(()) } @@ -121,7 +121,7 @@ impl WinClipboardImpl { fn on_render_format(&mut self, format: ClipboardFormatId) -> WinCliprdrResult> { // Owning clipboard is not required when processing `WM_RENDERFORMAT` message if let Some(response) = self.get_remote_format_data(format)? { - self.on_format_data_response(format, &response)?; + Self::on_format_data_response(format, &response)?; } Ok(None) @@ -209,8 +209,8 @@ impl WinClipboardImpl { BackendEvent::RenderFormat(format) => self.on_render_format(*format), BackendEvent::RenderAllFormats => self.on_render_all_formats(), - unhandled_event => { - warn!("Unhandled clipboard event: {:?}", unhandled_event); + BackendEvent::DowngradedCapabilities(flags) => { + warn!(%flags, "Unhandled downgraded capabilities event"); Ok(None) } }; @@ -244,15 +244,19 @@ impl WinClipboardImpl { const MAX_PROCESSING_ATTEMPTS: usize = 10; const PROCESSING_TIMEOUT_MS: u32 = 100; + #[allow(clippy::arithmetic_side_effects)] + // self.attempt can’t be greater than MAX_PROCESSING_ATTEMPTS if self.attempt < MAX_PROCESSING_ATTEMPTS { self.attempt += 1; + self.retry_message = Some(event); + // SAFETY: `SetTimer` is always safe to call when `hwnd` is a valid window handle unsafe { SetTimer( self.window, IDT_CLIPBOARD_RETRY, - self.attempt as u32 * PROCESSING_TIMEOUT_MS, + self.attempt * PROCESSING_TIMEOUT_MS, None, ) }; @@ -307,7 +311,7 @@ pub(crate) unsafe extern "system" fn clipboard_subproc( match msg { // We need to keep track of window state to distinguish between local and remote copy - WM_ACTIVATE => ctx.window_is_active = wparam.0 != WA_INACTIVE as _, + WM_ACTIVATE => ctx.window_is_active = wparam.0 != WA_INACTIVE as usize, // as conversion is fine for constants // Sent by the OS when OS clipboard content is changed WM_CLIPBOARDUPDATE => { // SAFETY: `GetClipboardOwner` is always safe to call. @@ -324,7 +328,8 @@ pub(crate) unsafe extern "system" fn clipboard_subproc( } // Sent by the OS when delay-rendered data is requested for rendering. WM_RENDERFORMAT => { - ctx.handle_event(BackendEvent::RenderFormat(ClipboardFormatId::new(wparam.0 as _))); + #[allow(clippy::cast_possible_truncation)] // should never truncate in practice + ctx.handle_event(BackendEvent::RenderFormat(ClipboardFormatId::new(wparam.0 as u32))); } // Sent by the OS when all delay-rendered data is requested for rendering. WM_RENDERALLFORMATS => { diff --git a/crates/ironrdp-cliprdr-native/src/windows/cliprdr_backend.rs b/crates/ironrdp-cliprdr-native/src/windows/cliprdr_backend.rs index 189938855..835e771c0 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/cliprdr_backend.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/cliprdr_backend.rs @@ -11,7 +11,7 @@ use windows::Win32::UI::WindowsAndMessaging::PostMessageW; use crate::windows::{BackendEvent, WM_CLIPRDR_BACKEND_EVENT}; #[derive(Debug)] -pub struct WinCliprdrBackend { +pub(crate) struct WinCliprdrBackend { backend_event_tx: mpsc_sync::SyncSender, window: HWND, } diff --git a/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs b/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs index 600e7f31c..8fc6e75fe 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs @@ -9,10 +9,10 @@ use crate::windows::utils::get_last_winapi_error; use crate::windows::WinCliprdrError; /// Safe wrapper around windows. Clipboard is automatically closed on drop. -pub struct OwnedOsClipboard; +pub(crate) struct OwnedOsClipboard; impl OwnedOsClipboard { - pub fn new(window: HWND) -> Result { + pub(crate) fn new(window: HWND) -> Result { // SAFETY: `window` is valid handle, therefore it is safe to call `OpenClipboard`. if unsafe { OpenClipboard(window) } == FALSE { // Retryable error @@ -28,7 +28,8 @@ impl OwnedOsClipboard { } /// Enumerates all available formats in the current clipboard. - pub fn enum_available_formats(&self) -> Result, WinCliprdrError> { + #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII + pub(crate) fn enum_available_formats(&self) -> Result, WinCliprdrError> { const DEFAULT_FORMATS_CAPACITY: usize = 16; // Sane default for format name. If format name is longer than this, // `GetClipboardFormatNameW` will truncate it. @@ -48,7 +49,9 @@ impl OwnedOsClipboard { let format = if !format_id.is_standard() { // SAFETY: It is safe to call `GetClipboardFormatNameW` with correct buffer pointer // and size (wrapped as slice via `windows` crate) - let read_chars = unsafe { GetClipboardFormatNameW(raw_format, &mut format_name_w) } as usize; + let read_chars: usize = unsafe { GetClipboardFormatNameW(raw_format, &mut format_name_w) } + .try_into() + .expect("never negative"); if read_chars != 0 { let format_name = String::from_utf16(format_name_w[..read_chars].as_ref()) @@ -77,7 +80,8 @@ impl OwnedOsClipboard { Ok(formats) } - pub fn clear(&mut self) -> Result<(), WinCliprdrError> { + #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII, and exclusive &mut self reference + pub(crate) fn clear(&mut self) -> Result<(), WinCliprdrError> { // We need to empty clipboard before setting any delay-rendered data // // SAFETY: We own the clipboard at moment of method invocation, therefore it is safe to @@ -89,7 +93,8 @@ impl OwnedOsClipboard { Ok(()) } - pub fn delay_render(&mut self, format: ClipboardFormatId) -> Result<(), WinCliprdrError> { + #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII, and exclusive &mut self reference + pub(crate) fn delay_render(&mut self, format: ClipboardFormatId) -> Result<(), WinCliprdrError> { // SAFETY: We own the clipboard at moment of method invocation, therefore it is safe to // call `SetClipboardData`. let result = unsafe { SetClipboardData(format.value(), HANDLE(0)) }; diff --git a/crates/ironrdp-cliprdr-native/src/windows/remote_format_registry.rs b/crates/ironrdp-cliprdr-native/src/windows/remote_format_registry.rs index ccb509dbd..9025ea104 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/remote_format_registry.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/remote_format_registry.rs @@ -8,7 +8,7 @@ use windows::Win32::System::DataExchange::RegisterClipboardFormatW; use crate::windows::utils::get_last_winapi_error; #[derive(Debug, Default)] -pub struct RemoteClipboardFormatRegistry { +pub(crate) struct RemoteClipboardFormatRegistry { remote_to_local: HashMap, local_to_remote: HashMap, } @@ -16,7 +16,7 @@ pub struct RemoteClipboardFormatRegistry { impl RemoteClipboardFormatRegistry { /// Clear current format mapping, as per RDP spec, format mapping is reset on every /// received `CLIPRDR_FORMAT_LIST` message. - pub fn clear(&mut self) { + pub(crate) fn clear(&mut self) { self.remote_to_local.clear(); self.local_to_remote.clear(); } @@ -34,7 +34,7 @@ impl RemoteClipboardFormatRegistry { /// /// Returns local format id for the remote format id. /// If the format is unknown or not supported on the local machine, returns `None`. - pub fn register(&mut self, remote_format: &ClipboardFormat) -> Option { + pub(crate) fn register(&mut self, remote_format: &ClipboardFormat) -> Option { if remote_format.id().is_standard() { // Standard formats such as `CF_TEXT` have fixed ids, which are same on all machines. return Some(remote_format.id()); @@ -87,7 +87,7 @@ impl RemoteClipboardFormatRegistry { Some(mapped_format_id) } - pub fn local_to_remote(&self, local_format: ClipboardFormatId) -> Option { + pub(crate) fn local_to_remote(&self, local_format: ClipboardFormatId) -> Option { if local_format.is_standard() { return Some(local_format); } diff --git a/crates/ironrdp-cliprdr-native/src/windows/utils.rs b/crates/ironrdp-cliprdr-native/src/windows/utils.rs index 5cf88707d..264595834 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/utils.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/utils.rs @@ -26,7 +26,7 @@ impl GlobalMemoryBuffer { // - `dst` is valid for writes of `data.len()` bytes, we allocated enough above. // - Both `data` and `dst` are properly aligned: u8 alignment is 1 // - Memory regions are not overlapping, `dst` was allocated by us just above. - unsafe { std::ptr::copy_nonoverlapping(data.as_ptr(), dst as _, data.len()) }; + unsafe { std::ptr::copy_nonoverlapping(data.as_ptr(), dst as *mut u8, data.len()) }; // SAFETY: We called `GlobalLock` on this handle just above. unsafe { GlobalUnlock(handle) }; @@ -52,7 +52,7 @@ impl Drop for GlobalMemoryBuffer { /// /// SAFETY: This function should only be called in the context of processing /// `WM_RENDERFORMAT` or `WM_RENDERALLFORMATS` messages inside WinAPI message loop. -pub unsafe fn render_format(format: ClipboardFormatId, data: &[u8]) -> WinCliprdrResult<()> { +pub(crate) unsafe fn render_format(format: ClipboardFormatId, data: &[u8]) -> WinCliprdrResult<()> { // Allocate buffer and copy data into it let global_data = GlobalMemoryBuffer::from_slice(data)?; @@ -71,7 +71,7 @@ pub unsafe fn render_format(format: ClipboardFormatId, data: &[u8]) -> WinCliprd } /// Return last WinAPI error code. -pub fn get_last_winapi_error() -> WIN32_ERROR { +pub(crate) fn get_last_winapi_error() -> WIN32_ERROR { // SAFETY: `GetLastError` is always safe to call. unsafe { GetLastError() } } From 1fb3ee087eb725087c87d8a16417ab5e31e29284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 00:09:21 -0400 Subject: [PATCH 09/14] Second pass --- .../src/windows/clipboard_impl.rs | 10 +++++----- .../ironrdp-cliprdr-native/src/windows/os_clipboard.rs | 9 +++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs index ebdbf2e2d..4697a9f9c 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs @@ -154,7 +154,7 @@ impl WinClipboardImpl { for format in formats { if let Some(response) = self.get_remote_format_data(format)? { - self.on_format_data_response(format, &response)?; + Self::on_format_data_response(format, &response)?; } } @@ -210,7 +210,7 @@ impl WinClipboardImpl { BackendEvent::RenderAllFormats => self.on_render_all_formats(), BackendEvent::DowngradedCapabilities(flags) => { - warn!(%flags, "Unhandled downgraded capabilities event"); + warn!(?flags, "Unhandled downgraded capabilities event"); Ok(None) } }; @@ -241,11 +241,11 @@ impl WinClipboardImpl { self.attempt = 0; } Some(err) => { - const MAX_PROCESSING_ATTEMPTS: usize = 10; + const MAX_PROCESSING_ATTEMPTS: u32 = 10; const PROCESSING_TIMEOUT_MS: u32 = 100; #[allow(clippy::arithmetic_side_effects)] - // self.attempt can’t be greater than MAX_PROCESSING_ATTEMPTS + // self.attempt can’t be greater than MAX_PROCESSING_ATTEMPTS, so the arithmetic is safe here if self.attempt < MAX_PROCESSING_ATTEMPTS { self.attempt += 1; @@ -311,7 +311,7 @@ pub(crate) unsafe extern "system" fn clipboard_subproc( match msg { // We need to keep track of window state to distinguish between local and remote copy - WM_ACTIVATE => ctx.window_is_active = wparam.0 != WA_INACTIVE as usize, // as conversion is fine for constants + WM_ACTIVATE => ctx.window_is_active = wparam.0 != WA_INACTIVE as usize, // `as` conversion is fine for constants // Sent by the OS when OS clipboard content is changed WM_CLIPBOARDUPDATE => { // SAFETY: `GetClipboardOwner` is always safe to call. diff --git a/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs b/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs index 8fc6e75fe..aa891291b 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/os_clipboard.rs @@ -28,8 +28,8 @@ impl OwnedOsClipboard { } /// Enumerates all available formats in the current clipboard. - #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII - pub(crate) fn enum_available_formats(&self) -> Result, WinCliprdrError> { + #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII, and exclusive &mut self reference + pub(crate) fn enum_available_formats(&mut self) -> Result, WinCliprdrError> { const DEFAULT_FORMATS_CAPACITY: usize = 16; // Sane default for format name. If format name is longer than this, // `GetClipboardFormatNameW` will truncate it. @@ -80,10 +80,11 @@ impl OwnedOsClipboard { Ok(formats) } + /// Empties the clipboard + /// + /// It is required to empty clipboard before setting any delay-rendered data. #[allow(clippy::unused_self)] // ensure we own the clipboard using RAII, and exclusive &mut self reference pub(crate) fn clear(&mut self) -> Result<(), WinCliprdrError> { - // We need to empty clipboard before setting any delay-rendered data - // // SAFETY: We own the clipboard at moment of method invocation, therefore it is safe to // call `EmptyClipboard`. if unsafe { EmptyClipboard() } == FALSE { From b110da3a85c6b82e602b2fdb49f6c69650b45c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 00:21:27 -0400 Subject: [PATCH 10/14] Third pass --- crates/ironrdp-cliprdr-native/src/windows.rs | 14 ++----------- .../src/windows/clipboard_impl.rs | 21 +++++++++++++++++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/crates/ironrdp-cliprdr-native/src/windows.rs b/crates/ironrdp-cliprdr-native/src/windows.rs index 55ffc63f0..4eb65414e 100644 --- a/crates/ironrdp-cliprdr-native/src/windows.rs +++ b/crates/ironrdp-cliprdr-native/src/windows.rs @@ -125,21 +125,11 @@ impl WinClipboard { let (backend_tx, backend_rx) = mpsc_sync::sync_channel(BACKEND_CHANNEL_SIZE); - let ctx = Box::new(WinClipboardImpl { - message_proxy: Box::new(message_proxy), - backend_rx, - window, - attempt: 0, - retry_message: None, - // We assume that we start with current window active - window_is_active: true, - remote_format_registry: Default::default(), - available_formats_on_remote: Vec::new(), - }); + let ctx = Box::new(WinClipboardImpl::new(window, message_proxy, backend_rx)); // We need to receive winapi messages in the main thread, so we need to add a subclass to // the window. - // + // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. let winapi_result = diff --git a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs index 4697a9f9c..5a203b773 100644 --- a/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs +++ b/crates/ironrdp-cliprdr-native/src/windows/clipboard_impl.rs @@ -1,5 +1,5 @@ use std::collections::HashSet; -use std::sync::mpsc as mpsc_sync; +use std::sync::mpsc; use std::time::Duration; use ironrdp_cliprdr::backend::{ClipboardMessage, ClipboardMessageProxy}; @@ -27,7 +27,7 @@ pub(crate) struct WinClipboardImpl { window: HWND, message_proxy: Box, window_is_active: bool, - backend_rx: mpsc_sync::Receiver, + backend_rx: mpsc::Receiver, // Number of attempts spent to process current clipboard message attempt: u32, // Message to retry @@ -38,6 +38,23 @@ pub(crate) struct WinClipboardImpl { } impl WinClipboardImpl { + pub(crate) fn new( + window: HWND, + message_proxy: impl ClipboardMessageProxy + 'static, + backend_rx: mpsc::Receiver, + ) -> Self { + Self { + window, + message_proxy: Box::new(message_proxy), + window_is_active: true, // We assume that we start with current window active, + backend_rx, + attempt: 0, + retry_message: None, + available_formats_on_remote: Vec::new(), + remote_format_registry: Default::default(), + } + } + fn on_format_data_request(&mut self, request: &FormatDataRequest) -> WinCliprdrResult> { // Get data from the clipboard and send event to the main event loop let clipboard = OwnedOsClipboard::new(self.window)?; From c9d2f25b98d8357149851e017dcdbff18af7589e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 00:30:14 -0400 Subject: [PATCH 11/14] Probably last pass --- crates/ironrdp-cliprdr-native/src/windows.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ironrdp-cliprdr-native/src/windows.rs b/crates/ironrdp-cliprdr-native/src/windows.rs index 4eb65414e..18aba2ee6 100644 --- a/crates/ironrdp-cliprdr-native/src/windows.rs +++ b/crates/ironrdp-cliprdr-native/src/windows.rs @@ -129,11 +129,11 @@ impl WinClipboard { // We need to receive winapi messages in the main thread, so we need to add a subclass to // the window. - - // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, - // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. - let winapi_result = - unsafe { SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as usize) }; + let winapi_result = unsafe { + // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, + // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. + SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as usize) + }; if winapi_result == FALSE { return Err(WinCliprdrError::WindowSubclass); From 0e1b2854a1410c788c47fb2ef0fc08aef45dfda9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 02:33:32 -0400 Subject: [PATCH 12/14] Fix configuration for undocumented_unsafe_blocks lint --- clippy.toml | 2 ++ crates/ironrdp-cliprdr-native/src/windows.rs | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clippy.toml b/clippy.toml index 0ac765717..2ded6b073 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,3 @@ semicolon-outside-block-ignore-multiline = true +accept-comment-above-statement = true +accept-comment-above-attributes = true diff --git a/crates/ironrdp-cliprdr-native/src/windows.rs b/crates/ironrdp-cliprdr-native/src/windows.rs index 18aba2ee6..fd26bc978 100644 --- a/crates/ironrdp-cliprdr-native/src/windows.rs +++ b/crates/ironrdp-cliprdr-native/src/windows.rs @@ -129,11 +129,11 @@ impl WinClipboard { // We need to receive winapi messages in the main thread, so we need to add a subclass to // the window. - let winapi_result = unsafe { - // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, - // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. - SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as usize) - }; + // + // SAFETY: `window` is a valid window handle, `clipboard_subproc` is in the static memory, + // `ctx` is valid and its ownership is transferred to the subclass via `into_raw`. + let winapi_result = + unsafe { SetWindowSubclass(window, Some(clipboard_subproc), 0, Box::into_raw(ctx) as usize) }; if winapi_result == FALSE { return Err(WinCliprdrError::WindowSubclass); From 27ec5ada775fcc8a540ecd9e338e5ad5b3bc05e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 02:51:47 -0400 Subject: [PATCH 13/14] Fix --- crates/ironrdp-client/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ironrdp-client/src/main.rs b/crates/ironrdp-client/src/main.rs index ada59e57a..cf3cf2cfb 100644 --- a/crates/ironrdp-client/src/main.rs +++ b/crates/ironrdp-client/src/main.rs @@ -45,7 +45,7 @@ fn main() -> anyhow::Result<()> { // while the gui window is still open. let win_clipboard = unsafe { WinClipboard::new( - HWND(gui.window.hwnd() as _), + HWND(gui.window().hwnd() as isize), ClientClipboardMessageProxy::new(input_event_sender.clone()), )? }; From d01a6e1e3e4ecc8af5c705a6bb8b69c54f0b0424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 29 Sep 2023 03:07:40 -0400 Subject: [PATCH 14/14] =?UTF-8?q?That=E2=80=99s=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/ironrdp-client/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ironrdp-client/src/main.rs b/crates/ironrdp-client/src/main.rs index cf3cf2cfb..cc55d2831 100644 --- a/crates/ironrdp-client/src/main.rs +++ b/crates/ironrdp-client/src/main.rs @@ -45,7 +45,7 @@ fn main() -> anyhow::Result<()> { // while the gui window is still open. let win_clipboard = unsafe { WinClipboard::new( - HWND(gui.window().hwnd() as isize), + HWND(gui.window().hwnd()), ClientClipboardMessageProxy::new(input_event_sender.clone()), )? };