From 1a77ca84d454694fb64b18b75a839ccef1a85848 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Mon, 7 Sep 2026 20:44:35 -0500 Subject: [PATCH 1/3] feat(server): let RdpServerDisplay report its monitor count DisplayControlBackend, ironrdp-server's internal DisplayControlHandler implementor, never overrode capabilities() (added by the previous commit), so it fell through to the single-monitor default regardless of how many monitors the consumer's RdpServerDisplay actually serves. Added a default RdpServerDisplay::monitor_count() method (1, matching today's behavior), fetched once alongside size() before the dynamic channels are attached, and threaded into DisplayControlBackend so its capabilities() now reports the real count. --- crates/ironrdp-server/src/display.rs | 9 +++++++++ crates/ironrdp-server/src/server.rs | 24 +++++++++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/crates/ironrdp-server/src/display.rs b/crates/ironrdp-server/src/display.rs index 0807013ae6..80bb5b881a 100644 --- a/crates/ironrdp-server/src/display.rs +++ b/crates/ironrdp-server/src/display.rs @@ -335,6 +335,15 @@ pub trait RdpServerDisplay: Send { fn request_layout(&mut self, layout: DisplayControlMonitorLayout) { debug!(?layout, "Requesting layout") } + + /// Report how many independent monitors this display can present to the client. + /// + /// Called once, before the Display Control Virtual Channel opens, to build the + /// capabilities the server advertises. Defaults to `1`, matching every existing + /// implementation's current behavior. + async fn monitor_count(&mut self) -> u32 { + 1 + } } #[cfg(test)] diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index f33f99bae3..8d14475d7f 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -14,7 +14,7 @@ use ironrdp_async::Framed; use ironrdp_cliprdr::CliprdrServer; use ironrdp_cliprdr::backend::ClipboardMessage; use ironrdp_core::{decode, encode_vec, impl_as_any}; -use ironrdp_displaycontrol::pdu::DisplayControlMonitorLayout; +use ironrdp_displaycontrol::pdu::{DisplayControlCapabilities, DisplayControlMonitorLayout}; use ironrdp_displaycontrol::server::{DisplayControlHandler, DisplayControlServer}; use ironrdp_dvc as dvc; #[cfg(feature = "usb")] @@ -555,11 +555,12 @@ impl dvc::DvcServerProcessor for AInputHandler {} struct DisplayControlBackend { display: Arc>>, + monitor_count: u32, } impl DisplayControlBackend { - fn new(display: Arc>>) -> Self { - Self { display } + fn new(display: Arc>>, monitor_count: u32) -> Self { + Self { display, monitor_count } } } @@ -568,6 +569,13 @@ impl DisplayControlHandler for DisplayControlBackend { let display = Arc::clone(&self.display); task::spawn_blocking(move || display.blocking_lock().request_layout(layout)); } + + fn capabilities(&self) -> DisplayControlCapabilities { + DisplayControlCapabilities::new(self.monitor_count, 3840, 2400).unwrap_or_else(|e| { + warn!(monitor_count = self.monitor_count, error = %e, "RdpServerDisplay::monitor_count() out of range, falling back to 1"); + DisplayControlCapabilities::new(1, 3840, 2400).expect("(1, 3840, 2400) are always within the valid range") + }) + } } #[cfg(feature = "usb")] @@ -1830,7 +1838,7 @@ impl RdpServer { self.gfx_handle.as_ref() } - fn attach_channels(&mut self, acceptor: &mut Acceptor) { + fn attach_channels(&mut self, acceptor: &mut Acceptor, monitor_count: u32) { if let Some(cliprdr_factory) = self.cliprdr_factory.as_deref() { let backend = cliprdr_factory.build_cliprdr_backend(); @@ -1851,7 +1859,7 @@ impl RdpServer { acceptor.attach_static_channel(RdpdrServer::new(backend)); } - let dcs_backend = DisplayControlBackend::new(Arc::clone(&self.display)); + let dcs_backend = DisplayControlBackend::new(Arc::clone(&self.display), monitor_count); let dvc = dvc::DrdynvcServer::new() .with_dynamic_channel(AInputHandler { handler: Arc::clone(&self.handler), @@ -1993,7 +2001,8 @@ impl RdpServer { // `accept_finalize`, which is where the acceptor first consumes the // static channel set (the MCS Connect Initial); `accept_begin`, already // done, stops at the security-upgrade gate before that. - self.attach_channels(&mut candidate.acceptor); + let monitor_count = self.display.lock().await.monitor_count().await; + self.attach_channels(&mut candidate.acceptor, monitor_count); self.finalize_negotiated(*candidate).await } @@ -2121,6 +2130,7 @@ impl RdpServer { self.display_suppressed.store(false, Ordering::Relaxed); let size = self.display.lock().await.size().await; + let monitor_count = self.display.lock().await.monitor_count().await; let capabilities = capabilities::capabilities(&self.opts, size); let mut pending = PendingConnection::new( self.opts.security.clone(), @@ -2130,7 +2140,7 @@ impl RdpServer { self.opts.honor_client_desktop_size, ); - self.attach_channels(pending.acceptor_mut()); + self.attach_channels(pending.acceptor_mut(), monitor_count); let Some(negotiated) = pending.negotiate_and_authenticate(stream, tls).await? else { return Ok(()); From 0ca57bd70e75e3c9f63685f09985b68d5f065e78 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Fri, 11 Sep 2026 17:43:16 -0500 Subject: [PATCH 2/3] review: fold a zero monitor_count into the out-of-range fallback DisplayControlCapabilities::new only rejects max_num_monitors > 1024, so an override returning 0 passed validation and encoded MaxNumMonitors=0, advertising a server that supports no monitors. Checked explicitly and folded into the existing warn-and-fall-back-to-1 path. --- crates/ironrdp-server/src/server.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 8d14475d7f..13755212e6 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -571,8 +571,17 @@ impl DisplayControlHandler for DisplayControlBackend { } fn capabilities(&self) -> DisplayControlCapabilities { - DisplayControlCapabilities::new(self.monitor_count, 3840, 2400).unwrap_or_else(|e| { - warn!(monitor_count = self.monitor_count, error = %e, "RdpServerDisplay::monitor_count() out of range, falling back to 1"); + // `DisplayControlCapabilities::new` only rejects `monitor_count > 1024`; 0 passes its + // validation (0 * 3840 * 2400 does not overflow) but would advertise a server that + // supports no monitors, so it is folded into the same out-of-range fallback below. + let monitor_count = if self.monitor_count == 0 { + warn!("RdpServerDisplay::monitor_count() returned 0, falling back to 1"); + 1 + } else { + self.monitor_count + }; + DisplayControlCapabilities::new(monitor_count, 3840, 2400).unwrap_or_else(|e| { + warn!(monitor_count, error = %e, "RdpServerDisplay::monitor_count() out of range, falling back to 1"); DisplayControlCapabilities::new(1, 3840, 2400).expect("(1, 3840, 2400) are always within the valid range") }) } From 21b1372847e2f2387b9e5e71c9501847ed1840e6 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sat, 12 Sep 2026 08:26:57 -0500 Subject: [PATCH 3/3] review: clarify monitor_count semantics and dedupe single-monitor caps Reworded RdpServerDisplay::monitor_count()'s doc comment to state it's a per-session capacity ceiling (MS-RDPEDISP MaxNumMonitors), not a current topology report, since the PR body's "report the real total" phrasing read as the latter and could mislead a consumer into treating it as a live count that goes stale after the value is sampled once. Added DisplayControlCapabilities::single_monitor(), following the crate's existing named-constructor convention, and switched both places that had reimplemented the (1, 3840, 2400) literal plus its expect message to use it instead: the trait's default capabilities() and the out-of-range fallback in DisplayControlBackend. --- crates/ironrdp-displaycontrol/src/pdu/mod.rs | 11 +++++++++++ crates/ironrdp-displaycontrol/src/server.rs | 8 ++++---- crates/ironrdp-server/src/display.rs | 14 ++++++++++---- crates/ironrdp-server/src/server.rs | 9 +++++---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/crates/ironrdp-displaycontrol/src/pdu/mod.rs b/crates/ironrdp-displaycontrol/src/pdu/mod.rs index a564657eed..0a5ebc0f72 100644 --- a/crates/ironrdp-displaycontrol/src/pdu/mod.rs +++ b/crates/ironrdp-displaycontrol/src/pdu/mod.rs @@ -174,6 +174,17 @@ impl DisplayControlCapabilities { pub fn max_monitor_area(&self) -> u64 { self.max_monitor_area } + + /// One 4K-area (3840x2400) monitor: the single-monitor capabilities every + /// existing server advertised before per-display monitor counts existed, + /// and the safe fallback for a monitor count that turns out to be invalid. + /// + /// # Panics + /// + /// Never: `(1, 3840, 2400)` is always within [`new`](Self::new)'s valid range. + pub fn single_monitor() -> Self { + Self::new(1, 3840, 2400).expect("(1, 3840, 2400) are always within the valid range") + } } impl Encode for DisplayControlCapabilities { diff --git a/crates/ironrdp-displaycontrol/src/server.rs b/crates/ironrdp-displaycontrol/src/server.rs index 8b9f82fb73..fde336afbc 100644 --- a/crates/ironrdp-displaycontrol/src/server.rs +++ b/crates/ironrdp-displaycontrol/src/server.rs @@ -13,9 +13,9 @@ pub trait DisplayControlHandler: Send { /// Capabilities advertised to the client when the channel starts. /// - /// Defaults to a single-monitor value (`max_num_monitors = 1`, factors - /// `3840`/`2400`, i.e. one 4K-area monitor), so any handler that doesn't - /// override this keeps that behavior. + /// Defaults to [`DisplayControlCapabilities::single_monitor()`] (one monitor, + /// factors `3840`/`2400`, i.e. one 4K-area monitor), so any handler that + /// doesn't override this keeps that behavior. /// A handler serving more than one monitor should override this, e.g. /// `DisplayControlCapabilities::new(monitor_count, 3840, 2400)`. Fallible /// because [`DisplayControlCapabilities::new`] validates its arguments @@ -24,7 +24,7 @@ pub trait DisplayControlHandler: Send { /// that error rather than `expect` it, since this is called from /// [`DvcProcessor::start`] and a panic there aborts the connection. fn capabilities(&self) -> PduResult { - DisplayControlCapabilities::new(1, 3840, 2400).map_err(|e| decode_err!(e)) + Ok(DisplayControlCapabilities::single_monitor()) } } diff --git a/crates/ironrdp-server/src/display.rs b/crates/ironrdp-server/src/display.rs index 80bb5b881a..3d232cf37c 100644 --- a/crates/ironrdp-server/src/display.rs +++ b/crates/ironrdp-server/src/display.rs @@ -336,11 +336,17 @@ pub trait RdpServerDisplay: Send { debug!(?layout, "Requesting layout") } - /// Report how many independent monitors this display can present to the client. + /// The maximum number of monitors this display will honor in a client's + /// `request_layout()` call for the rest of the session. /// - /// Called once, before the Display Control Virtual Channel opens, to build the - /// capabilities the server advertises. Defaults to `1`, matching every existing - /// implementation's current behavior. + /// This is a capacity ceiling (MS-RDPEDISP `MaxNumMonitors`), not a report + /// of the current topology: the client may request any layout up to this + /// many monitors, and it is validated against this number, not the other + /// way around. Called once, before the Display Control Virtual Channel + /// opens, to build the capabilities the server advertises; the display's + /// actual monitor count may later grow or shrink within that ceiling + /// without a way to advertise a new one mid-session. Defaults to `1`, + /// matching every existing implementation's current behavior. async fn monitor_count(&mut self) -> u32 { 1 } diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 13755212e6..9363b9309c 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -570,7 +570,7 @@ impl DisplayControlHandler for DisplayControlBackend { task::spawn_blocking(move || display.blocking_lock().request_layout(layout)); } - fn capabilities(&self) -> DisplayControlCapabilities { + fn capabilities(&self) -> PduResult { // `DisplayControlCapabilities::new` only rejects `monitor_count > 1024`; 0 passes its // validation (0 * 3840 * 2400 does not overflow) but would advertise a server that // supports no monitors, so it is folded into the same out-of-range fallback below. @@ -580,10 +580,11 @@ impl DisplayControlHandler for DisplayControlBackend { } else { self.monitor_count }; - DisplayControlCapabilities::new(monitor_count, 3840, 2400).unwrap_or_else(|e| { + let capabilities = DisplayControlCapabilities::new(monitor_count, 3840, 2400).unwrap_or_else(|e| { warn!(monitor_count, error = %e, "RdpServerDisplay::monitor_count() out of range, falling back to 1"); - DisplayControlCapabilities::new(1, 3840, 2400).expect("(1, 3840, 2400) are always within the valid range") - }) + DisplayControlCapabilities::single_monitor() + }); + Ok(capabilities) } }