feat(connector): expose SUPPORT_DYN_VC_GFX_PROTOCOL early-cap flag for EGFX clients - #1237
feat(connector): expose SUPPORT_DYN_VC_GFX_PROTOCOL early-cap flag for EGFX clients#1237GlassOnTin wants to merge 1 commit into
Conversation
|
Reading through, this is the smallest possible fix and the doc-comment captures the foot-gun precisely. Without wiring a DvcClientProcessor for the Graphics channel, modern Windows servers stop sending legacy bitmap updates entirely and the desktop goes blank, which is exactly the GRD case in #1232 and the failure mode every downstream EGFX client hits. No conflict with #1098 (multitransport bootstrapping) on my read of the GCC block area, just two independent flags being OR'd in. Looks good from the server-side perspective too: any IronRDP server that already speaks EGFX (lamco-rdp-server, GRD) sees this flag and routes graphics through DVC as expected. |
27f3273 to
84ddffa
Compare
|
GlassOnTin Hi, could you rebase this? I'm trying to get this prioritized, and this would be a first step on the client side work I'm looking t. Thanks! |
…r EGFX clients Currently, `early_capability_flags` in the GCC core data is built from a fixed set in `connection.rs`. Clients that want to use the Graphics Pipeline Extension (MS-RDPEGFX) — by attaching a DvcClientProcessor for `Microsoft::Windows::RDS::Graphics` — have no way to set `SUPPORT_DYN_VC_GFX_PROTOCOL` without forking the connector, and modern Windows servers won't open the EGFX channel unless the client advertises support. This commit adds an opt-in `Config.support_dyn_vc_gfx_protocol: bool` (default `false`). When set, the flag is OR'd into `early_capability_flags` alongside the existing `WANT_32_BPP_SESSION` conditional. Existing consumers are unaffected; the doc comment includes a safety note that setting this without an EGFX implementation will cause Windows to stop sending legacy bitmap updates, leaving the desktop blank. Used downstream by Haven (an Android RDP/VNC client) which implements EGFX with ClearCodec + RemoteFxProgressive decoders; this lets us drop the vendored ironrdp-connector fork. Default `false` to preserve current behaviour.
84ddffa to
dbe5d82
Compare
|
Done — rebased onto current The only conflicts were in the test builders, where
|
|
Thank you! Benoît Cortier (@CBenoit) can you expedite this please? Thank you. My client issue will be posted later today. |
|
For reviewer context: I've opened #1472 as a sibling to this — a small |
…tionResult (#1488) Per [MS-RDPBCGR] 2.2.8.1.2, a client must not send fast-path input events unless the server advertised `INPUT_FLAG_FASTPATH_INPUT` or `INPUT_FLAG_FASTPATH_INPUT2` in its Input Capability Set. Today the connector discards the server's capability sets after Demand Active, so client code has no way to honour that requirement. This PR captures the server's Input capability flags during capabilities exchange, carries them through the `ConnectionFinalization`/`Finalized` activation states (so they are refreshed correctly across a Deactivation-Reactivation Sequence too), and surfaces them as `ConnectionResult::input_flags`. The session layer can then choose between fast-path and slow-path input per server. ### Motivation / real-world interop This is not theoretical: VirtualBox's VRDP server closes the connection outright on receiving a fast-path input PDU — its `VBox.log` reports ``` VRDP: Network packet length is incorrect 0x0004. Closing connection. ``` (a single fast-path scancode event is a 4-byte packet). VirtualBox never advertises fast-path input; its Demand Active offers `InputFlags(SCANCODES)` alone. mstsc and FreeRDP honour the negotiation and fall back to slow-path `TS_INPUT_PDU`s, which is why they work against VRDE. Haven (Android RDP client built on IronRDP) has been shipping this exact change as a vendored-connector patch since v5.86.1, with the slow-path fallback keyed off `ConnectionResult::input_flags`. Verified against a real VirtualBox 7.2.6 VRDE server: before the gate, the first arrow-key press killed the session with the log line above; with the gate, extended input sessions run clean, and a fast-path-capable server on the same host still takes the fast-path branch. (Discussed in #1158; this is the third and last piece Haven carries in its connector fork, alongside #1237 and #1472.) ### Changes - `connection_activation.rs`: capture `input_flags` from the `CapabilitySet::Input` in Server Demand Active (empty if absent); add the field to `ConnectionActivationState::{ConnectionFinalization, Finalized}`. - `connection.rs`: add `ConnectionResult::input_flags`, populated from the `Finalized` state. - Call sites in `ironrdp-client`, `ironrdp-web`, ffi, and the e2e test updated for the new variant field (all currently ignore it). ### Testing - Two new integration tests in `ironrdp-testsuite-core/tests/session/connection_activation.rs`: the fixture's Demand Active yields `SCANCODES | MOUSEX | UNICODE | FASTPATH_INPUT_2` in the `ConnectionFinalization` state, and a Demand Active with the Input capability stripped yields `InputFlags::empty()`. - `cargo check --workspace --all-targets`, `cargo clippy --workspace --all-targets`, and `cargo fmt --check` are clean; the 7 activation tests pass. Co-authored-by: GlassOnTin <GlassOnTin@users.noreply.github.com>
github-actions
Bot
added
A-web-client
2 participants
Currently,
early_capability_flagsin the GCC core data is built from a fixed set inconnection.rs. Clients that want to use the Graphics Pipeline Extension (MS-RDPEGFX) — by attaching aDvcClientProcessorforMicrosoft::Windows::RDS::Graphics— have no way to setSUPPORT_DYN_VC_GFX_PROTOCOLwithout forking the connector, and modern Windows servers won't open the EGFX channel unless the client advertises support.This PR adds an opt-in
Config.support_dyn_vc_gfx_protocol: bool(defaultfalse). When set, the flag is OR'd intoearly_capability_flagsalongside the existingWANT_32_BPP_SESSIONconditional. Existing consumers are unaffected; the doc comment includes a safety note that setting this without an EGFX implementation will cause Windows to stop sending legacy bitmap updates, leaving the desktop blank.Used downstream by Haven (an Android RDP/VNC client) which implements EGFX with ClearCodec + RemoteFxProgressive decoders; this lets us drop a vendored fork of
ironrdp-connectorwe currently carry just for this one flag.Default
falseto preserve current behaviour.cargo check --workspaceclean — six other in-treeConfig { … }builders updated with the default-false field.