feat(acceptor): honor the client-requested desktop size

[clintcan]

Problem

An ironrdp-server always serves the desktop size reported by its
RdpServerDisplay, regardless of what the client asked for. When that size
differs from the client's actual display resolution, the client rescales every
decoded frame — which on mstsc costs noticeable typing latency and, with H.264
over EGFX, contributes to audio drift. The only workaround today is to know the
client's resolution out-of-band and hard-code the display handler to match it.

The natural-looking hook — RdpServerDisplay::request_initial_size, added in
#1083 — can't actually be used to detect the client's request. It's fed the
desktop size from the client's Confirm Active, but per [MS-RDPBCGR]
2.2.1.13.2 a conformant client copies the server's Demand Active size into
its Confirm Active and echoes that back. Verified live: FreeRDP launched with
/size:1024x768 echoes the server's size, not 1024x768 (this matches
FreeRDP's rdp_apply_bitmap_capability_set, and mstsc follows the same
desktopResizeFlag contract). The client's own requested resolution is carried
only in the GCC Client Core Data of the MCS Connect Initial — which the
acceptor parses and currently discards — and the acceptor commits a size in
Demand Active before any server-side code runs.

(This is also, I think, the substance of the question Benoît Cortier (@CBenoit) raised on #1083
about whether request_initial_size during reactivation is the right shape.)

Approach

Adopt the client's Core Data desktop size in the acceptor, before Demand
Active is sent, gated behind an opt-in flag. Because it happens pre-Demand-
Active, the session is negotiated at the client's resolution from the start —
no Deactivation-Reactivation resize round trip. The display handler still
learns the negotiated size through the existing request_initial_size call
(the Confirm Active echo now equals the adopted size).

Opt-in, default off → no behavior change for existing servers.

API

• ironrdp-acceptor: new Acceptor::set_honor_client_desktop_size(bool)
  (additive). When set, the BasicSettingsWaitInitial step adopts the Core
  Data size if it's within the protocol-legal range (200..=8192) and writes it
  into the server Bitmap capability set before Demand Active. Also factors the
  existing bitmap-capset desktop-size mutation (previously inline in
  new_deactivation_reactivation) into a shared set_bitmap_desktop_size
  helper, and adds a small pure validate_desktop_size helper.
• ironrdp-server: new builder method
  RdpServerBuilder::with_honor_client_desktop_size(bool), backed by a new
  RdpServerOptions::honor_client_desktop_size field, forwarded to each
  connection's acceptor in run_connection.

Notes / questions for review

1. Where the server option lives. I put it on RdpServerOptions (the
   config bag, next to max_request_size) rather than threading a new
   parameter through RdpServer::new as display_suppressed does. That adds a
   public field to RdpServerOptions, which is a minor-version breaking change
   for anyone constructing it by struct literal (the builder is unaffected).
   Happy to move it to a new parameter instead if you'd prefer to keep
   RdpServerOptions stable — let me know which you'd like.
2. Tests. ironrdp-acceptor's lib sets test = false, so I didn't add
   inline unit tests (they wouldn't run). The two new helpers are pure and
   trivial; I verified the end-to-end behavior against real clients (below). If
   there's a preferred place for acceptor coverage, I'll add it there.

Testing

Built and clippy-clean (-D warnings) for ironrdp-acceptor and
ironrdp-server --features egfx against current master. Exercised through a
downstream server with real clients:

• sdl-freerdp /size:1024x768 — legacy bitmap path, session negotiated at
  1024×768 from the first frame.
• sdl-freerdp /size:1280x800 then reconnect /size:800x600 — H.264/EGFX,
  surface + encoder created at the requested size each connection.
• mstsc full-screen on a 1920×1080 monitor — session served at 1920×1080,
  client presents 1:1, typing latency gone.

[clintcan]

Rebased onto current master — it had picked up a small conflict in ironrdp-server/src/builder.rs (the new autodetect_rtt field landed next to where this PR adds honor_client_desktop_size; both kept). It's mergeable again and CI is green so far. Ready for review whenever you have a chance 🙂

The two design questions from the description are still open for your call: (1) the option lives on RdpServerBuilder/RdpServerOptions here — happy to move it to a RdpServer setter/param instead if you prefer; (2) the acceptor adopts the size in BasicSettingsWaitInitial, with no inline test since the acceptor lib is test = false — glad to add coverage wherever you'd like it.

[clintcan]

Friendly review ping 🙂 — CI is green and it's rebased clean on master (mergeable). Now that #1359 has landed, this (honor the client-requested desktop size) and the small companion #1397 (expose the client's keyboard layout on AcceptorResult) are the two remaining acceptor pieces on my side.

Happy to adjust per the two design questions in the description — the option placement (RdpServerOptions field vs a RdpServer::new param) and where you'd like test coverage to live given the acceptor lib is test = false. No rush; just flagging it's ready whenever you have a slot.

[Copilot]

Pull request overview

This PR adds an opt-in server/acceptor knob to negotiate the RDP session desktop size using the client’s originally requested resolution (from GCC Client Core Data) so the server can start at the client’s native size without a Deactivation–Reactivation resize round trip.

Changes:

• Add Acceptor::set_honor_client_desktop_size(bool) and implement pre–Demand Active adoption of the GCC Core Data desktop size (with range validation).
• Add RdpServerBuilder::with_honor_client_desktop_size(bool) and plumb the option through RdpServerOptions into each connection’s acceptor.
• Factor bitmap-capability desktop sizing into a shared set_bitmap_desktop_size helper.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/ironrdp-server/src/server.rs	Adds honor_client_desktop_size to server options and forwards it into the per-connection acceptor.
crates/ironrdp-server/src/builder.rs	Adds a builder method to configure the new option and wires it into RdpServerOptions.
crates/ironrdp-acceptor/src/connection.rs	Implements the opt-in handshake-time size adoption and capability-set mutation helpers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Benoît Cortier (CBenoit)]

Thanks for chasing this down the Core Data path. The writeup is exactly right, and adopting the size pre-Demand-Active (instead of reaching for another Deactivation-Reactivation) is the correct shape.

praise: The validate_desktop_size -> Option shape and factoring set_bitmap_desktop_size out of new_deactivation_reactivation are both clean. I like the "parse-don’t-validate" approach here too.

issue: set_honor_client_desktop_size reads as self-contained, but its correctness actually depends on a contract on a different trait. With the flag on, the encoder is still built from self.display.size() after request_initial_size(client_size) is called, so a RdpServerDisplay whose request_initial_size ignores its argument (fixed framebuffer) will trip the client_size < display_size branch and likely drop the client. Worth noting that branch was effectively dormant before this PR: without the flag, client_size equals the display's own echoed size, so an initial connection could never drive the mismatch with a client-controlled value. Please document the precondition on set_honor_client_desktop_size (and mirror it on with_honor_client_desktop_size): enabling this requires a display handler whose request_initial_size actually adopts, or at least intersects, the requested size. Right now the coupling is implicit and that's one misuse-resistance gap.

suggestion: The reject path is silent: when validate_desktop_size returns None we fall back to the server size with no log, while the honor path gets a debug!. A trace!/debug! on the None arm ("client requested out-of-range size WxH, keeping server size") would save an operator the "why wasn't my size honored?" hunt.

suggestion: On your option-placement question: keep it as the RdpServerOptions field (natural home next to max_request_size), and let's add #[non_exhaustive] to RdpServerOptions in this PR. I'm fine with the break, and non_exhaustive stops the next field addition from being another one. The builder stays the blessed construction path regardless.

thought: The [200, 8192] clamp is the protocol ceiling, not a resource guard. With the flag on, a client-controlled 8192x8192 sizes the encoder/surface allocation from an untrusted number. Fine for an opt-in, default-off knob and consistent with how max_request_size trusts its input, so I'm not asking to change it here. Just flagging that if we ever want stronger operator control, the better surface is a clamp/range policy rather than a bare bool.

[clintcan]

Thanks for the thorough review! Rebased onto current master (picking up #1397, which touches the same connection.rs) and pushed a follow-up addressing every point — commit 3d315f8:

• issue — implicit cross-trait coupling. Documented the precondition on both set_honor_client_desktop_size (acceptor) and with_honor_client_desktop_size (builder): enabling it requires a RdpServerDisplay whose request_initial_size actually adopts (or at least intersects) the requested size, otherwise the encoder is still built from the handler's own size and the client_size < display_size mismatch can drop the client. Spelled out that a fixed-size handler should leave it disabled.
• suggestion — silent reject path. Added a debug! on the validate_desktop_size → None arm ("client requested an out-of-range desktop size; keeping the server-provided size") so an operator isn't left hunting for why their size wasn't honored.
• suggestion — option placement + non_exhaustive. Kept it as the RdpServerOptions field and marked RdpServerOptions #[non_exhaustive] in this PR, so this is the last source-breaking field addition and the builder stays the blessed path. (Also resolves Copilot's source-breaking-field note.)
• Copilot — doc wording. Fixed the MIN_DESKTOP_DIM doc: an out-of-range dimension falls back to the server-provided size rather than being "treated as malformed".
• thought — [200, 8192] is a protocol ceiling, not a resource guard. Agreed, and left as-is here per your note. Tracked the "clamp/range policy rather than a bare bool" idea as a future follow-up (an operator-set max size) so a client can't drive the encoder/surface allocation off an untrusted number — happy to do that as a separate PR if you'd like it.

Build / fmt / clippy (-D warnings) / tests all clean on the rebased branch.

[Benoît Cortier (CBenoit)]

LGTM!

thought — [200, 8192] is a protocol ceiling, not a resource guard. Agreed, and left as-is here per your note. Tracked the "clamp/range policy rather than a bare bool" idea as a future follow-up (an operator-set max size) so a client can't drive the encoder/surface allocation off an untrusted number — happy to do that as a separate PR if you'd like it.

I would appreciate that, thank you! 🙂