From 7c5a99bda912dcb8a877365234a04b10f3a53f86 Mon Sep 17 00:00:00 2001 From: uchouT Date: Tue, 3 Mar 2026 10:53:15 +0800 Subject: [PATCH 1/4] feat(dvc): abstract listener trait in client-side Signed-off-by: uchouT --- crates/ironrdp-dvc/src/client.rs | 105 ++++++++++++++++++++++++++++++- crates/ironrdp-dvc/src/lib.rs | 77 ----------------------- 2 files changed, 104 insertions(+), 78 deletions(-) diff --git a/crates/ironrdp-dvc/src/client.rs b/crates/ironrdp-dvc/src/client.rs index 9cce57ff2..cb0112201 100644 --- a/crates/ironrdp-dvc/src/client.rs +++ b/crates/ironrdp-dvc/src/client.rs @@ -1,7 +1,10 @@ +use alloc::boxed::Box; +use alloc::collections::btree_map::BTreeMap; use alloc::vec::Vec; use core::any::TypeId; use core::fmt; +use crate::alloc::borrow::ToOwned as _; use ironrdp_core::{Decode as _, DecodeResult, ReadCursor, impl_as_any}; use ironrdp_pdu::{self as pdu, decode_err, encode_err, pdu_other_err}; use ironrdp_svc::{ChannelFlags, CompressionCondition, SvcClientProcessor, SvcMessage, SvcProcessor}; @@ -13,10 +16,37 @@ use crate::pdu::{ CapabilitiesResponsePdu, CapsVersion, ClosePdu, CreateResponsePdu, CreationStatus, DrdynvcClientPdu, DrdynvcServerPdu, }; -use crate::{DvcProcessor, DynamicChannelSet, DynamicVirtualChannel, encode_dvc_messages}; +use crate::{DvcProcessor, DynamicChannelId, DynamicChannelName, DynamicVirtualChannel, encode_dvc_messages}; pub trait DvcClientProcessor: DvcProcessor {} +pub trait DvcChannelListener: Send { + /// Called for each incoming DYNVC_CREATE_REQ matching this name. + /// Return `None` to reject (NO_LISTENER). + fn create(&mut self) -> Option>; +} + +pub type DynamicChannelListener = Box; + +/// For pre-registered Dvc +pub struct OnceListener { + inner: Option>, +} + +impl OnceListener { + pub fn new(dvc_processor: impl DvcClientProcessor) -> Self { + Self { + inner: Some(Box::new(dvc_processor)), + } + } +} + +impl DvcChannelListener for OnceListener { + fn create(&mut self) -> Option> { + self.inner.take() + } +} + /// DRDYNVC Static Virtual Channel (the Remote Desktop Protocol: Dynamic Virtual Channel Extension) /// /// It adds support for dynamic virtual channels (DVC). @@ -182,6 +212,79 @@ impl SvcProcessor for DrdynvcClient { } } +struct DynamicChannelSet { + channels: BTreeMap, + name_to_channel_id: BTreeMap, + channel_id_to_name: BTreeMap, + type_id_to_name: BTreeMap, +} + +impl DynamicChannelSet { + #[inline] + fn new() -> Self { + Self { + channels: BTreeMap::new(), + name_to_channel_id: BTreeMap::new(), + channel_id_to_name: BTreeMap::new(), + type_id_to_name: BTreeMap::new(), + } + } + + fn insert(&mut self, channel: T) -> Option { + let name = channel.channel_name().to_owned(); + self.type_id_to_name.insert(TypeId::of::(), name.clone()); + self.channels.insert(name, DynamicVirtualChannel::new(channel)) + } + + fn attach_channel_id(&mut self, name: DynamicChannelName, id: DynamicChannelId) -> Option { + self.channel_id_to_name.insert(id, name.clone()); + self.name_to_channel_id.insert(name.clone(), id); + let dvc = self.get_by_channel_name_mut(&name)?; + let old_id = dvc.channel_id; + dvc.channel_id = Some(id); + old_id + } + + fn get_by_type_id(&self, type_id: TypeId) -> Option<&DynamicVirtualChannel> { + self.type_id_to_name + .get(&type_id) + .and_then(|name| self.channels.get(name)) + } + + fn get_by_channel_name(&self, name: &DynamicChannelName) -> Option<&DynamicVirtualChannel> { + self.channels.get(name) + } + + fn get_by_channel_name_mut(&mut self, name: &DynamicChannelName) -> Option<&mut DynamicVirtualChannel> { + self.channels.get_mut(name) + } + + fn get_by_channel_id(&self, id: DynamicChannelId) -> Option<&DynamicVirtualChannel> { + self.channel_id_to_name + .get(&id) + .and_then(|name| self.channels.get(name)) + } + + fn get_by_channel_id_mut(&mut self, id: DynamicChannelId) -> Option<&mut DynamicVirtualChannel> { + self.channel_id_to_name + .get(&id) + .and_then(|name| self.channels.get_mut(name)) + } + + fn remove_by_channel_id(&mut self, id: DynamicChannelId) -> Option { + if let Some(name) = self.channel_id_to_name.remove(&id) { + return self.name_to_channel_id.remove(&name); + // Channels are retained in the `self.channels` and `self.type_id_to_name` map to allow potential + // dynamic re-addition by the server. + } + None + } + + #[inline] + fn values(&self) -> impl Iterator { + self.channels.values() + } +} impl SvcClientProcessor for DrdynvcClient {} fn decode_dvc_message(user_data: &[u8]) -> DecodeResult { diff --git a/crates/ironrdp-dvc/src/lib.rs b/crates/ironrdp-dvc/src/lib.rs index 952a792c6..457b71aa1 100644 --- a/crates/ironrdp-dvc/src/lib.rs +++ b/crates/ironrdp-dvc/src/lib.rs @@ -5,14 +5,11 @@ extern crate alloc; use alloc::boxed::Box; -use alloc::collections::BTreeMap; use alloc::string::String; use alloc::vec::Vec; -use core::any::TypeId; use pdu::DrdynvcDataPdu; -use crate::alloc::borrow::ToOwned as _; // Re-export ironrdp_pdu crate for convenience #[rustfmt::skip] // do not re-order this pub use pub use ironrdp_pdu; @@ -154,79 +151,5 @@ impl DynamicVirtualChannel { } } -struct DynamicChannelSet { - channels: BTreeMap, - name_to_channel_id: BTreeMap, - channel_id_to_name: BTreeMap, - type_id_to_name: BTreeMap, -} - -impl DynamicChannelSet { - #[inline] - fn new() -> Self { - Self { - channels: BTreeMap::new(), - name_to_channel_id: BTreeMap::new(), - channel_id_to_name: BTreeMap::new(), - type_id_to_name: BTreeMap::new(), - } - } - - fn insert(&mut self, channel: T) -> Option { - let name = channel.channel_name().to_owned(); - self.type_id_to_name.insert(TypeId::of::(), name.clone()); - self.channels.insert(name, DynamicVirtualChannel::new(channel)) - } - - fn attach_channel_id(&mut self, name: DynamicChannelName, id: DynamicChannelId) -> Option { - self.channel_id_to_name.insert(id, name.clone()); - self.name_to_channel_id.insert(name.clone(), id); - let dvc = self.get_by_channel_name_mut(&name)?; - let old_id = dvc.channel_id; - dvc.channel_id = Some(id); - old_id - } - - fn get_by_type_id(&self, type_id: TypeId) -> Option<&DynamicVirtualChannel> { - self.type_id_to_name - .get(&type_id) - .and_then(|name| self.channels.get(name)) - } - - fn get_by_channel_name(&self, name: &DynamicChannelName) -> Option<&DynamicVirtualChannel> { - self.channels.get(name) - } - - fn get_by_channel_name_mut(&mut self, name: &DynamicChannelName) -> Option<&mut DynamicVirtualChannel> { - self.channels.get_mut(name) - } - - fn get_by_channel_id(&self, id: DynamicChannelId) -> Option<&DynamicVirtualChannel> { - self.channel_id_to_name - .get(&id) - .and_then(|name| self.channels.get(name)) - } - - fn get_by_channel_id_mut(&mut self, id: DynamicChannelId) -> Option<&mut DynamicVirtualChannel> { - self.channel_id_to_name - .get(&id) - .and_then(|name| self.channels.get_mut(name)) - } - - fn remove_by_channel_id(&mut self, id: DynamicChannelId) -> Option { - if let Some(name) = self.channel_id_to_name.remove(&id) { - return self.name_to_channel_id.remove(&name); - // Channels are retained in the `self.channels` and `self.type_id_to_name` map to allow potential - // dynamic re-addition by the server. - } - None - } - - #[inline] - fn values(&self) -> impl Iterator { - self.channels.values() - } -} - pub type DynamicChannelName = String; pub type DynamicChannelId = u32; From b1f80baa0bebb80465d6469cce78ddbf2b0dbccd Mon Sep 17 00:00:00 2001 From: uchouT Date: Tue, 3 Mar 2026 17:50:43 +0800 Subject: [PATCH 2/4] feat(dvc): add listener/factory pattern for client-side DVC creation Introduce DvcChannelListener trait and OnceListener to support same-name multi-instance dynamic virtual channels. DVCs are now created on-demand via try_create_channel() when the server sends a CreateRequest, rather than pre-allocated at registration. The existing with_dynamic_channel() API is preserved via OnceListener. BREAKING: get_dvc_by_type_id() now returns None until the server sends a CreateRequest for the channel. Previously it returned Some immediately after registration. Signed-off-by: uchouT perf(dvc): reduce another btreemap search Signed-off-by: uchouT perf(dvc): reduce needless allocation in OnceListener Signed-off-by: uchouT --- crates/ironrdp-dvc/src/client.rs | 155 ++++++++++++++++++------------- crates/ironrdp-dvc/src/lib.rs | 4 +- 2 files changed, 92 insertions(+), 67 deletions(-) diff --git a/crates/ironrdp-dvc/src/client.rs b/crates/ironrdp-dvc/src/client.rs index cb0112201..15ed123a3 100644 --- a/crates/ironrdp-dvc/src/client.rs +++ b/crates/ironrdp-dvc/src/client.rs @@ -21,20 +21,21 @@ use crate::{DvcProcessor, DynamicChannelId, DynamicChannelName, DynamicVirtualCh pub trait DvcClientProcessor: DvcProcessor {} pub trait DvcChannelListener: Send { + fn channel_name(&self) -> &str; /// Called for each incoming DYNVC_CREATE_REQ matching this name. /// Return `None` to reject (NO_LISTENER). - fn create(&mut self) -> Option>; + fn create(&mut self) -> Option>; } pub type DynamicChannelListener = Box; /// For pre-registered Dvc -pub struct OnceListener { - inner: Option>, +struct OnceListener { + inner: Option>, } impl OnceListener { - pub fn new(dvc_processor: impl DvcClientProcessor) -> Self { + fn new(dvc_processor: impl DvcProcessor) -> Self { Self { inner: Some(Box::new(dvc_processor)), } @@ -42,7 +43,13 @@ impl OnceListener { } impl DvcChannelListener for OnceListener { - fn create(&mut self) -> Option> { + fn channel_name(&self) -> &str { + self.inner + .as_ref() + .expect("channel name called after created") + .channel_name() + } + fn create(&mut self) -> Option> { self.inner.take() } } @@ -81,14 +88,22 @@ impl DrdynvcClient { } } - // FIXME(#61): it’s likely we want to enable adding dynamic channels at any point during the session (message passing? other approach?) - #[must_use] + /// with a pre-registered dynamic virtual channel pub fn with_dynamic_channel(mut self, channel: T) -> Self where T: DvcProcessor + 'static, { - self.dynamic_channels.insert(channel); + self.dynamic_channels.insert_dvc(channel); + self + } + + #[must_use] + pub fn with_listener(mut self, listener: T) -> Self + where + T: DvcChannelListener + 'static, + { + self.dynamic_channels.insert_listener(listener); self } @@ -96,7 +111,7 @@ impl DrdynvcClient { where T: DvcProcessor + 'static, { - self.dynamic_channels.insert(channel); + self.dynamic_channels.insert_dvc(channel); } pub fn get_dvc_by_type_id(&self) -> Option<&DynamicVirtualChannel> @@ -157,20 +172,12 @@ impl SvcProcessor for DrdynvcClient { responses.push(self.create_capabilities_response(CapsVersion::V2)); } - let channel_exists = self.dynamic_channels.get_by_channel_name(&channel_name).is_some(); - let (creation_status, start_messages) = if channel_exists { - // If we have a handler for this channel, attach the channel ID - // and get any start messages. - self.dynamic_channels - .attach_channel_id(channel_name.clone(), channel_id); - let dynamic_channel = self - .dynamic_channels - .get_by_channel_name_mut(&channel_name) - .expect("channel exists"); - (CreationStatus::OK, dynamic_channel.start()?) - } else { - (CreationStatus::NO_LISTENER, Vec::new()) - }; + let (creation_status, start_messages) = + if let Some(dvc) = self.dynamic_channels.try_create_channel(&channel_name, channel_id) { + (CreationStatus::OK, dvc.start()?) + } else { + (CreationStatus::NO_LISTENER, Vec::new()) + }; let create_response = DrdynvcClientPdu::Create(CreateResponsePdu::new(channel_id, creation_status)); debug!("Send DVC Create Response PDU: {create_response:?}"); @@ -212,77 +219,95 @@ impl SvcProcessor for DrdynvcClient { } } +struct ListenerEntry { + listener: DynamicChannelListener, + /// `Some` only for channels registered via `with_dynamic_channel()`. + type_id: Option, +} + struct DynamicChannelSet { - channels: BTreeMap, - name_to_channel_id: BTreeMap, - channel_id_to_name: BTreeMap, - type_id_to_name: BTreeMap, + listeners: BTreeMap, + active_channels: BTreeMap, + type_id_to_channel_id: BTreeMap, } impl DynamicChannelSet { #[inline] fn new() -> Self { Self { - channels: BTreeMap::new(), - name_to_channel_id: BTreeMap::new(), - channel_id_to_name: BTreeMap::new(), - type_id_to_name: BTreeMap::new(), + listeners: BTreeMap::new(), + active_channels: BTreeMap::new(), + type_id_to_channel_id: BTreeMap::new(), } } - fn insert(&mut self, channel: T) -> Option { - let name = channel.channel_name().to_owned(); - self.type_id_to_name.insert(TypeId::of::(), name.clone()); - self.channels.insert(name, DynamicVirtualChannel::new(channel)) + fn insert_listener(&mut self, listener: T) { + let name = listener.channel_name().to_owned(); + self.listeners.insert( + name, + ListenerEntry { + listener: Box::new(listener), + type_id: None, + }, + ); } - fn attach_channel_id(&mut self, name: DynamicChannelName, id: DynamicChannelId) -> Option { - self.channel_id_to_name.insert(id, name.clone()); - self.name_to_channel_id.insert(name.clone(), id); - let dvc = self.get_by_channel_name_mut(&name)?; - let old_id = dvc.channel_id; - dvc.channel_id = Some(id); - old_id + fn insert_dvc(&mut self, channel: T) { + let name = channel.channel_name().to_owned(); + self.listeners.insert( + name, + ListenerEntry { + listener: Box::new(OnceListener::new(channel)), + type_id: Some(TypeId::of::()), + }, + ); } - fn get_by_type_id(&self, type_id: TypeId) -> Option<&DynamicVirtualChannel> { - self.type_id_to_name - .get(&type_id) - .and_then(|name| self.channels.get(name)) - } + fn try_create_channel( + &mut self, + name: &DynamicChannelName, + channel_id: DynamicChannelId, + ) -> Option<&mut DynamicVirtualChannel> { + let entry = self.listeners.get_mut(name)?; + let processor = entry.listener.create()?; + + if let Some(type_id) = entry.type_id { + self.type_id_to_channel_id.insert(type_id, channel_id); + } - fn get_by_channel_name(&self, name: &DynamicChannelName) -> Option<&DynamicVirtualChannel> { - self.channels.get(name) + let mut dvc = DynamicVirtualChannel::from_boxed(processor); + dvc.channel_id = Some(channel_id); + let dvc = match self.active_channels.entry(channel_id) { + alloc::collections::btree_map::Entry::Occupied(mut e) => { + e.insert(dvc); + e.into_mut() + } + alloc::collections::btree_map::Entry::Vacant(e) => e.insert(dvc), + }; + Some(dvc) } - fn get_by_channel_name_mut(&mut self, name: &DynamicChannelName) -> Option<&mut DynamicVirtualChannel> { - self.channels.get_mut(name) + fn get_by_type_id(&self, type_id: TypeId) -> Option<&DynamicVirtualChannel> { + self.type_id_to_channel_id + .get(&type_id) + .and_then(|id| self.active_channels.get(id)) } fn get_by_channel_id(&self, id: DynamicChannelId) -> Option<&DynamicVirtualChannel> { - self.channel_id_to_name - .get(&id) - .and_then(|name| self.channels.get(name)) + self.active_channels.get(&id) } fn get_by_channel_id_mut(&mut self, id: DynamicChannelId) -> Option<&mut DynamicVirtualChannel> { - self.channel_id_to_name - .get(&id) - .and_then(|name| self.channels.get_mut(name)) + self.active_channels.get_mut(&id) } - fn remove_by_channel_id(&mut self, id: DynamicChannelId) -> Option { - if let Some(name) = self.channel_id_to_name.remove(&id) { - return self.name_to_channel_id.remove(&name); - // Channels are retained in the `self.channels` and `self.type_id_to_name` map to allow potential - // dynamic re-addition by the server. - } - None + fn remove_by_channel_id(&mut self, id: DynamicChannelId) { + self.active_channels.remove(&id); } #[inline] fn values(&self) -> impl Iterator { - self.channels.values() + self.active_channels.values() } } impl SvcClientProcessor for DrdynvcClient {} diff --git a/crates/ironrdp-dvc/src/lib.rs b/crates/ironrdp-dvc/src/lib.rs index 457b71aa1..319424852 100644 --- a/crates/ironrdp-dvc/src/lib.rs +++ b/crates/ironrdp-dvc/src/lib.rs @@ -108,9 +108,9 @@ pub struct DynamicVirtualChannel { } impl DynamicVirtualChannel { - fn new(handler: T) -> Self { + fn from_boxed(processor: Box) -> Self { Self { - channel_processor: Box::new(handler), + channel_processor: processor, complete_data: CompleteData::new(), channel_id: None, } From b37241e3f360d23e8077b2c29bcdcd78dc7a1801 Mon Sep 17 00:00:00 2001 From: uchouT Date: Thu, 5 Mar 2026 04:12:02 +0800 Subject: [PATCH 3/4] feat(dvc): add create_channel() for runtime server-side DVC creation Unlike `with_dynamic_channel`, this method is designed for runtime use and doesn't record a TypeId mapping. Signed-off-by: uchouT chore(dvc): docs modified and new fn attach_listener Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/ironrdp-dvc/src/client.rs | 8 +++++++- crates/ironrdp-dvc/src/server.rs | 23 +++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/ironrdp-dvc/src/client.rs b/crates/ironrdp-dvc/src/client.rs index 15ed123a3..18432a42a 100644 --- a/crates/ironrdp-dvc/src/client.rs +++ b/crates/ironrdp-dvc/src/client.rs @@ -88,8 +88,8 @@ impl DrdynvcClient { } } - #[must_use] /// with a pre-registered dynamic virtual channel + #[must_use] pub fn with_dynamic_channel(mut self, channel: T) -> Self where T: DvcProcessor + 'static, @@ -107,6 +107,12 @@ impl DrdynvcClient { self } + pub fn attach_listener(&mut self, listener: T) + where + T: DvcChannelListener + 'static, + { + self.dynamic_channels.insert_listener(listener); + } pub fn attach_dynamic_channel(&mut self, channel: T) where T: DvcProcessor + 'static, diff --git a/crates/ironrdp-dvc/src/server.rs b/crates/ironrdp-dvc/src/server.rs index 001db8cf2..80bc73016 100644 --- a/crates/ironrdp-dvc/src/server.rs +++ b/crates/ironrdp-dvc/src/server.rs @@ -96,8 +96,6 @@ impl DrdynvcServer { .is_some_and(|c| c.state == ChannelState::Opened) } - // FIXME(#61): it's likely we want to enable adding dynamic channels at any point during the session (message passing? other approach?) - /// Registers a dynamic channel with the server. /// /// # Panics @@ -121,6 +119,27 @@ impl DrdynvcServer { .get_mut(id) .ok_or_else(|| invalid_field_err!("DRDYNVC", "", "invalid channel id")) } + + /// Creates a new DVC, returns CreateRequest PDU to send to client. + /// + /// # Panics + /// + /// Panics if the number of registered dynamic channels exceeds `u32::MAX`. + pub fn create_channel(&mut self, channel: T) -> PduResult + where + T: DvcServerProcessor + 'static, + { + let channel_name = channel.channel_name().into(); + let mut dvc = DynamicChannel::new(channel); + dvc.state = ChannelState::Creation; + + let id = self.dynamic_channels.insert(dvc); + // The slab index is used as the DVC channel ID (a u32). + let channel_id = u32::try_from(id).expect("DVC channel count should not exceed u32::MAX"); + + let req = DrdynvcServerPdu::Create(CreateRequestPdu::new(channel_id, channel_name)); + as_svc_msg_with_flag(req) + } } impl_as_any!(DrdynvcServer); From 5f199eaa267acb538e22e9c999541986f82be915 Mon Sep 17 00:00:00 2001 From: uchouT Date: Thu, 5 Mar 2026 15:48:08 +0800 Subject: [PATCH 4/4] fix(dvc): forget to remove type_id mapping when remove channel by id Signed-off-by: uchouT chore(dvc): docs update, clarify limitation Signed-off-by: uchouT chore(dvc): clean style for edition 2024 Signed-off-by: uchouT --- crates/ironrdp-dvc/src/client.rs | 13 ++++++++++++- crates/ironrdp-dvc/src/lib.rs | 6 ++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-dvc/src/client.rs b/crates/ironrdp-dvc/src/client.rs index 18432a42a..809060029 100644 --- a/crates/ironrdp-dvc/src/client.rs +++ b/crates/ironrdp-dvc/src/client.rs @@ -98,6 +98,7 @@ impl DrdynvcClient { self } + /// Bind a listener. Doesn't support type id look up #[must_use] pub fn with_listener(mut self, listener: T) -> Self where @@ -107,12 +108,14 @@ impl DrdynvcClient { self } + /// Doesn't support type id look up pub fn attach_listener(&mut self, listener: T) where T: DvcChannelListener + 'static, { self.dynamic_channels.insert_listener(listener); } + pub fn attach_dynamic_channel(&mut self, channel: T) where T: DvcProcessor + 'static, @@ -308,7 +311,15 @@ impl DynamicChannelSet { } fn remove_by_channel_id(&mut self, id: DynamicChannelId) { - self.active_channels.remove(&id); + if let Some(dvc) = self.active_channels.remove(&id) { + let type_id = dvc.processor_type_id(); + + // Only matters for pre-registered channels + if let alloc::collections::btree_map::Entry::Occupied(entry) = self.type_id_to_channel_id.entry(type_id) + && entry.get() == &id { + entry.remove(); + } + } } #[inline] diff --git a/crates/ironrdp-dvc/src/lib.rs b/crates/ironrdp-dvc/src/lib.rs index 319424852..2794523dd 100644 --- a/crates/ironrdp-dvc/src/lib.rs +++ b/crates/ironrdp-dvc/src/lib.rs @@ -4,6 +4,8 @@ extern crate alloc; +use core::any::TypeId; + use alloc::boxed::Box; use alloc::string::String; use alloc::vec::Vec; @@ -116,6 +118,10 @@ impl DynamicVirtualChannel { } } + fn processor_type_id(&self) -> TypeId { + self.channel_processor.as_any().type_id() + } + pub fn is_open(&self) -> bool { self.channel_id.is_some() }