Conversation
The client registers the graphics pipeline channel but always told the server it does not support it (`support_dyn_vc_gfx_protocol: false`), so a server kept graphics on the legacy bitmap path. That path travels on the main connection and cannot move onto a reliable UDP tunnel: measured against Windows, input, pointer and video channels moved to the tunnel while the desktop kept painting over TCP. The UDP handshake also always offered version 3. `ConnectionConfig`'s `offer_version` already lets a caller offer MS-RDPEUDP (version 1 or 2) outright, which is what the Windows hosts measured settle on, but the client gave no way to set it. ConfigBuilder gains: - `with_graphics_pipeline(bool)`, off by default, which advertises the graphics pipeline to the server. - `with_udp_offer_version(UdpVersion)`, version 3 by default, which sets the highest RDP-UDP version the SYN offers. `UdpVersion` is re-exported from `config`. The viewer exposes both as `--egfx` and `--udp-offer`, next to a new `--udp` switch for the existing `with_udp_transport`. Each also reads an environment variable (`IRONRDP_EGFX`, `IRONRDP_UDP_OFFER`, `IRONRDP_UDP`).
github-actions
Bot
added
needs-review
…#2007) Windows lists every dynamic channel it intends to move in its Soft-Sync request, including the ones the client declined with NO_LISTENER. Against a Windows 11 host the request lists channels 2, 6, 7, 8, 9, 10, 11 and 12 (CoreInput, MouseCursor, Graphics, Video, Geometry, ...), and only channel 7, the graphics pipeline, is open. `process_soft_sync_request` dropped a whole channel list as soon as one ID in it was not open. The tunnel was then never switched, and the channels the client had opened stayed on TCP while the server was already sending them on the tunnel (MS-RDPEDYC 3.2.5.3.1). Unopened channels are now skipped one by one, and the tunnel is switched for the rest. ## Testing - New `dvc::client::soft_sync_skips_channels_the_client_did_not_open` in `ironrdp-testsuite-core`. - Live, against a Windows 11 host over RDP-UDP version 2, with the viewer built from a branch that also carries the tunnel and client PRs of this series: the Soft-Sync request above now switches the tunnel, and the graphics pipeline moves onto it. ## Checks - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets --features helper,__bench --locked -- -D warnings` - `cargo test --locked -p ironrdp-testsuite-core -p ironrdp-testsuite-extra`, plus the lib tests of the crates touched here - `cargo test --workspace --locked` on a branch that merges this PR with the other Windows interop PRs from this series - `typos` on the changed files ## Series These PRs port the Windows interop fixes and Linux backends from a downstream IronRDP fork, so the fork can be retired. Each one is based on `master` and can be reviewed and merged on its own. I also checked that all of them merge cleanly together in this order. - #2007 fix(dvc): Soft-Sync tunnel with declined channels - #2008 fix(session)!: channels and graphics on the tunnel - #2009 fix(rdpeudp): auto-detect on the tunnel - #2010 fix(graphics)!: SRL streams from Windows - #2011 fix(egfx): bitmap cache across ResetGraphics - #2012 feat(session): bandwidth measurements during the session - #2013 feat(client): graphics pipeline and RDP-UDP version options - #2014 fix(client): resize reconnects on the graphics pipeline - #2015 feat(client): transport event - #2016 feat(cliprdr): Linux clipboard backend - #2017 feat(rdpdr): printer on Linux and macOS Co-authored-by: AKolenda <testedemail2222@gmail.com>
github-actions
Bot
added
kind/protocol
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR adds two opt-in client knobs: with_graphics_pipeline, which now drives support_dyn_vc_gfx_protocol (default false, so negotiated behavior is unchanged), and with_udp_offer_version (default V3), plumbed into the RDPEUDP ConnectionConfig at bootstrap, plus --udp/--udp-offer/--egfx viewer flags with env vars. Wiring checks out: the graphics-pipeline channel (GraphicsPipelineClient) is already unconditionally registered in the client, V3 offers carry the cookie hash while V1/V2 omit it, and UDP bootstrap failures fall back to TCP. Independently verified that the connector unconditionally advertises SUPPORT_NET_CHAR_AUTODETECT and implements connect-time auto-detection, so the protocol reviewer's open conformance question is resolved and rejected. Published two low-severity gaps: missing tests for the new builder options and --udp-offer being silently inert without --udp.
Tests the builder defaults and the viewer flags in the testsuite, where CI runs them. The viewer now rejects --udp-offer without --udp, since the offered version is only read by the UDP handshake and would otherwise be ignored without a word.
github-actions
Bot
added
needs-review
github-actions
Bot
added
risk/medium
There was a problem hiding this comment.
PR #2013 adds two opt-in ConfigBuilder knobs: with_graphics_pipeline (flips support_dyn_vc_gfx_protocol, off by default) and with_udp_offer_version (UdpVersion, V3 by default), plus viewer flags --egfx, --udp-offer, and --udp with env fallbacks and tests. Independent inspection confirms correctness: defaults are unchanged; feature gating is consistent (the viewer depends on client-all, which enables ironrdp-client/udp, so the un-gated UdpVersion use in cli.rs always compiles); the offer version is threaded into the only UdpBootstrapConfig construction site and applied via ConnectionConfig::offer_version; the --udp-offer-requires---udp check correctly permits an explicit --udp=false; and tests exercise the real viewer parse path. The sole publishable finding is the code-compressor's low-severity note that the manual dependency bail duplicates clap's `requires` attribute, already used elsewhere in the same Args struct. No protocol, safety, or correctness defects were found.
github-actions
Bot
added
ai-reviewed/2
Benoît Cortier (CBenoit)
added
needs-author-action
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
LGTM, thank you!
I agree with the last suggestion: clap also supports some kind of relations between arguments, so it may be good to use it.
Note to my agent: you can merge on my behalf if there is no major change outside of the CLI improvement.
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
|
Pushed bbc3b6f with the requested clap dependency check. All 39 client configuration integration tests pass, including builder defaults, CLI overrides and UDP dependency validation. Formatting and targeted Clippy with warnings denied also pass. The earlier builder-coverage and silently ignored --udp-offer findings are covered by those tests. |
github-actions
Bot
added
automation-failed
ignore this, my automated code review agent, no idea why it activates on external PR's |
|
Rechecked the failed notification against the latest commit: the normal build/test CI suite passes. The separate public API check fails before comparing changes because its fresh dependency resolution selects incompatible picky-krb 0.12.5 with sspi 0.21.3. The focused workflow repair is #2071, which builds both revisions with their committed lockfiles. It has passed a real IronRDP API build and unchanged/breaking/stale-lockfile fixtures locally. The workflow runs from the base branch, so this check needs that repair merged before a rerun can use it. |
Adds client configuration for advertising the graphics pipeline and selecting the highest RDP-UDP version offered during the handshake.
ConfigBuilder::with_graphics_pipeline(bool)controls the graphics capability and remains off by default.ConfigBuilder::with_udp_offer_version(UdpVersion)sets the offered version and retains version 3 as its default.UdpVersionis re-exported fromconfig, and the value is readable fromConfig.--egfx,--udp, and--udp-offer <1|2|3>, withIRONRDP_EGFX,IRONRDP_UDP, andIRONRDP_UDP_OFFERenvironment equivalents.Clap validates that
--udp-offerhas an explicit UDP option before any configuration early return, including RPC mode. An explicit--udp=falseis accepted.ViewerConfig::parse_fromreturns parse errors for programmatic callers while normal command-line parsing retains Clap's standard help and error exits.Validation
All 39 client configuration integration tests pass, including builder defaults/overrides, missing UDP option, RPC mode, and explicit false. Targeted Clippy with warnings denied, workspace formatting, and diff checks pass. The current regular CI suite is green; the separate API automation dependency failure is addressed by #2071.
Earlier Windows interoperability testing motivated these options; no new live Windows run has been performed for this revision.