fix(dvc): switch a Soft-Sync tunnel that also lists declined channels - #2007
Conversation
A Soft-Sync request lists every dynamic channel the server intends to move, including channels the client answered with NO_LISTENER. Windows lists CoreInput, MouseCursor, Video and Geometry next to the graphics pipeline. Any unopened channel in a list made the client drop that whole list, so the tunnel was never switched, and the channels the client had opened stayed on TCP while the server was already sending them on the tunnel. Skip unopened channels one by one and switch the tunnel for the rest.
|
This pull request may overlap with #2008. Both touch the DRDYNVC client's Soft-Sync handling in crates/ironrdp-dvc/src/client.rs for the same scenario: Windows moving dynamic channels onto the tunnel while referencing channels the client declined with NO_LISTENER. This PR skips unknown channel IDs in the Soft-Sync list; PR 2008 handles Create/Close and NO_LISTENER response routing around the same tunnel-binding logic, so shared scope merits human review. This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide. Note LLM-assisted content (no human feedback). |
There was a problem hiding this comment.
🟢 PR #2007 rewrites process_soft_sync_request so channel IDs the client never opened (answered NO_LISTENER) are logged and skipped instead of voiding the whole tunnel list, and every available listed tunnel is acknowledged. Independent review confirms correctness: the unavailable-tunnel guard (MS-RDPEDYC 3.2.5.3.1) is preserved, tunnel_channels only maps opened channels, per-channel routing checks in process_tunnel and the TCP data guard stay consistent, and the request decoder already rejects duplicate channel IDs and tunnel types. Acknowledging tunnels whose listed channels were all declined matches the in-repo server validation and avoids stranding opened channels on TCP while the server sends on the tunnel. The new test reproduces the Windows 11 list shape. Both valid specialists (protocol, skeptical) reported zero findings; the code-compressor specialist failed and produced none. No findings are published.
Reduced coverage: optional reviewer code-compressor was unavailable.
|
On the overlap notice: #2007 and #2008 are complementary and merge cleanly in either order. #2007 decides which channels the Soft-Sync response binds when the request also lists channels the client declined. #2008 handles the Create, Close and graphics traffic that arrives on the tunnel after the switch. Against Windows 11 the graphics pipeline needs both to work over the tunnel. |
3b3a17b
into
Devolutions:master
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_requestdropped 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
dvc::client::soft_sync_skips_channels_the_client_did_not_openinironrdp-testsuite-core.Checks
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --features helper,__bench --locked -- -D warningscargo test --locked -p ironrdp-testsuite-core -p ironrdp-testsuite-extra, plus the lib tests of the crates touched herecargo test --workspace --lockedon a branch that merges this PR with the other Windows interop PRs from this seriestyposon the changed filesSeries
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
masterand can be reviewed and merged on its own. I also checked that all of them merge cleanly together in this order.