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 0807013ae6..3d232cf37c 100644 --- a/crates/ironrdp-server/src/display.rs +++ b/crates/ironrdp-server/src/display.rs @@ -335,6 +335,21 @@ pub trait RdpServerDisplay: Send { fn request_layout(&mut self, layout: DisplayControlMonitorLayout) { debug!(?layout, "Requesting layout") } + + /// The maximum number of monitors this display will honor in a client's + /// `request_layout()` call for the rest of the session. + /// + /// 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 + } } #[cfg(test)] diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index f33f99bae3..9363b9309c 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,23 @@ impl DisplayControlHandler for DisplayControlBackend { let display = Arc::clone(&self.display); task::spawn_blocking(move || display.blocking_lock().request_layout(layout)); } + + 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. + let monitor_count = if self.monitor_count == 0 { + warn!("RdpServerDisplay::monitor_count() returned 0, falling back to 1"); + 1 + } else { + self.monitor_count + }; + 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::single_monitor() + }); + Ok(capabilities) + } } #[cfg(feature = "usb")] @@ -1830,7 +1848,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 +1869,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 +2011,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 +2140,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 +2150,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(());