Skip to content

fix(dvc): drop data for a channel that is not open instead of failing - #2005

Open
meanaverage (meanaverage) wants to merge 2 commits into
Devolutions:masterfrom
meanaverage:fix/dvc-drop-data-for-unopened-channel
Open

meanaverage (meanaverage) wants to merge 2 commits into
Devolutions:masterfrom
meanaverage:fix/dvc-drop-data-for-unopened-channel

Conversation

@meanaverage

@meanaverage meanaverage (meanaverage) commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A server can send data on a dynamic channel before it has seen the client decline that channel in its Create Response. GNOME Remote Desktop (46.3, headless mode) does this for AUDIO_PLAYBACK_DVC: a client without an audio channel answers the Create Request with a failure status, but the server's first data PDU is already on the wire.

DrdynvcClient::process_data treated that as an error (access to non existing DVC channel). The error ends the active session, so the connection drops about a second after it is established.

Change

Log the data (debug!, with the channel id, as DrdynvcServer does for a declined channel) and drop it. Data for open channels is routed exactly as before; the tunnel / Soft-Sync checks are untouched.

Testing

  • New dvc::client::data_for_a_channel_that_is_not_open_is_dropped in ironrdp-testsuite-core. It fails on master with the error above and passes with this change; the other DVC tests pass.
  • cargo xtask check fmt, lints, locks, typos pass.
  • End to end: with this change and EGFX enabled in the web client (as in fix(egfx): stop the desktop from tearing when the server resizes graphics output #1977), ironrdp-web connects to GNOME Remote Desktop 46.3 (headless, Ubuntu 24.04) and stays connected. Without it, the session ends right after the audio channel is refused.

Related: #1446

Prepared with AI assistance; I reviewed the change and ran the tests above.

@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior risk/medium Behavioral change that does not substantially alter a core public API scope/core Touches the core architectural tier size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure needs-review A human reviewer is the current next actor labels Sep 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated review will not run because this contributor is not yet eligible under the automation policy.

Contributors become eligible after one qualifying IronRDP pull request is merged into master. Maintainer review is required.

@github-actions github-actions Bot removed the needs-review A human reviewer is the current next actor label Oct 1, 2026
@CBenoit Benoît Cortier (CBenoit) added the needs-author-action The pull request author is the current next actor label Oct 1, 2026
A server can send data on a dynamic channel before it sees the client
decline that channel in its Create Response. GNOME Remote Desktop does this
for AUDIO_PLAYBACK_DVC right after the Create Request, so a client without
an audio channel failed `process` with "access to non existing DVC channel"
and the session ended about a second after connecting.

Such data has nowhere to go: log it and drop it.
As with a declined channel on the server side, this is not a fault, and a
server may keep sending such data, so a warning per PDU would flood the log.
@meanaverage
meanaverage (meanaverage) force-pushed the fix/dvc-drop-data-for-unopened-channel branch from 067114b to 6407c22 Compare October 1, 2026 18:22
@github-actions github-actions Bot added automation-failed Exact-head automated classification or review failed or was unavailable risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny and removed needs-author-action The pull request author is the current next actor risk/medium Behavioral change that does not substantially alter a core public API labels Oct 1, 2026

This branch was successfully deployed

1 active deployment
llm-providers — 6407c22f Deployed Oct 1, 2026 by meanaverage via Classify pull request #1565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation-failed Exact-head automated classification or review failed or was unavailable kind/protocol Affects RDP or related protocol behavior risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure

Development

Successfully merging this pull request may close these issues.

2 participants