diff --git a/crates/ironrdp-displaycontrol/src/server.rs b/crates/ironrdp-displaycontrol/src/server.rs index 8b845aed8e..8b9f82fb73 100644 --- a/crates/ironrdp-displaycontrol/src/server.rs +++ b/crates/ironrdp-displaycontrol/src/server.rs @@ -10,6 +10,22 @@ pub trait DisplayControlHandler: Send { fn monitor_layout(&self, layout: DisplayControlMonitorLayout) { debug!(?layout); } + + /// 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. + /// 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 + /// (MS-RDPEDISP does not bound them, but the wire encoding does); an + /// overrider deriving values from runtime display state should propagate + /// 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)) + } } /// A server for the Display Control Virtual Channel. @@ -32,9 +48,7 @@ impl DvcProcessor for DisplayControlServer { } fn start(&mut self, _channel_id: u32) -> PduResult> { - let pdu: DisplayControlPdu = DisplayControlCapabilities::new(1, 3840, 2400) - .map_err(|e| decode_err!(e))? - .into(); + let pdu: DisplayControlPdu = self.handler.capabilities()?.into(); Ok(vec![Box::new(pdu)]) } diff --git a/crates/ironrdp-testsuite-core/tests/displaycontrol/mod.rs b/crates/ironrdp-testsuite-core/tests/displaycontrol/mod.rs index 3130e460a4..fe57452122 100644 --- a/crates/ironrdp-testsuite-core/tests/displaycontrol/mod.rs +++ b/crates/ironrdp-testsuite-core/tests/displaycontrol/mod.rs @@ -3,7 +3,9 @@ use std::sync::{Arc, Mutex}; use ironrdp_core::decode; use ironrdp_displaycontrol::client::DisplayControlClient; use ironrdp_displaycontrol::pdu; +use ironrdp_displaycontrol::server::{DisplayControlHandler, DisplayControlServer}; use ironrdp_dvc::DvcProcessor as _; +use ironrdp_pdu::{PduResult, decode_err}; use ironrdp_testsuite_core::encode_decode_test; encode_decode_test! { @@ -217,3 +219,74 @@ fn client_process_rejects_trailing_bytes_after_caps() { ); assert!(!client.ready()); } + +struct OutOfRangeCapsHandler; + +impl DisplayControlHandler for OutOfRangeCapsHandler { + fn capabilities(&self) -> PduResult { + // More than 1024 monitors is rejected by DisplayControlCapabilities::new, per + // invalid_caps above. + pdu::DisplayControlCapabilities::new(2000, 100, 100).map_err(|e| decode_err!(e)) + } +} + +#[test] +fn server_start_propagates_an_invalid_capabilities_override_as_an_error() { + // A handler deriving capabilities from real runtime state can hit the wire-encoding bound + // DisplayControlCapabilities::new enforces; start() must surface that as a PduResult::Err + // rather than let a handler's own expect()/panic tear down the connection. + let mut server = DisplayControlServer::new(Box::new(OutOfRangeCapsHandler)); + assert!(server.start(0).is_err()); +} + +/// Decode a `DvcMessage` produced by `DisplayControlServer::start()` back into the +/// `DisplayControlPdu` it encodes, the same round trip a real client would perform. +fn decode_dvc(msg: &ironrdp_dvc::DvcMessage) -> pdu::DisplayControlPdu { + let bytes = ironrdp_core::encode_vec(msg.as_ref()).expect("encode dvc message"); + decode(&bytes).expect("decode dvc message") +} + +struct MultiMonitorCapsHandler; + +impl DisplayControlHandler for MultiMonitorCapsHandler { + fn capabilities(&self) -> PduResult { + pdu::DisplayControlCapabilities::new(2, 3840, 2400).map_err(|e| decode_err!(e)) + } +} + +#[test] +fn server_start_encodes_a_multi_monitor_override() { + // The motivating case for a fallible, overridable capabilities(): a handler + // advertising more than the hardcoded single-monitor default. Verify the + // override's values actually reach the encoded DISPLAYCONTROL_CAPS_PDU, not + // just that start() succeeds. + let mut server = DisplayControlServer::new(Box::new(MultiMonitorCapsHandler)); + let messages = server.start(0).expect("valid override should not error"); + assert_eq!(messages.len(), 1); + match decode_dvc(&messages[0]) { + pdu::DisplayControlPdu::Caps(caps) => { + assert_eq!(caps, pdu::DisplayControlCapabilities::new(2, 3840, 2400).unwrap()); + } + other => panic!("expected DisplayControlPdu::Caps, got {other:?}"), + } +} + +struct DefaultCapsHandler; + +impl DisplayControlHandler for DefaultCapsHandler {} + +#[test] +fn server_start_encodes_the_default_capabilities_when_not_overridden() { + // The default capabilities() impl (single monitor, 3840x2400) was the + // hardcoded start() literal before this PR and remains untested; pin it so a + // regression in the default constants or the .into() encoding step is caught. + let mut server = DisplayControlServer::new(Box::new(DefaultCapsHandler)); + let messages = server.start(0).expect("default capabilities should not error"); + assert_eq!(messages.len(), 1); + match decode_dvc(&messages[0]) { + pdu::DisplayControlPdu::Caps(caps) => { + assert_eq!(caps, pdu::DisplayControlCapabilities::new(1, 3840, 2400).unwrap()); + } + other => panic!("expected DisplayControlPdu::Caps, got {other:?}"), + } +}