fix(connector): surface actual PDU type when an unexpected Share Control PDU arrives#1236
Merged
Benoît Cortier (CBenoit) merged 2 commits intoDevolutions:masterfrom Apr 29, 2026
Conversation
…rol PDU arrives Three error sites in the connector previously printed only the expected PDU type, dropping the actual type the server sent on the floor. Made debugging interop bugs harder than it needed to be. Specifically the GNOME Remote Desktop interop case (Devolutions#1232, GlassHaven/Haven#109): when Haven connects to a GRD server, GRD's freerdp_peer::Capabilities() callback fails because the client did not advertise the Graphics Pipeline (EGFX). GRD then sends a Server Deactivate All PDU instead of Server Demand Active. The connector saw `ServerDeactivateAll`, didn't match `ServerDemandActive`, and reported "unexpected Share Control Pdu (expected ServerDemandActive)" — with no hint that the server had sent Deactivate All (which itself is a strong signal the server gave up during caps validation). This commit replaces the three offending `general_err!` sites with `reason_err!` calls that include `pdu.as_short_name()` (the helper already exists on `ShareControlPdu`): connection_activation.rs:125 ConnectionActivation::CapabilitiesExchange legacy.rs:152 decode_share_data legacy.rs:213 decode_io_channel Now diagnoses look like: unexpected Share Control PDU during capabilities exchange: got Server Deactivate All PDU (expected Server Demand Active PDU) which immediately points at GRD-style "I rejected your caps and deactivated the share" rather than leaving callers to guess. No behavior change beyond the error string. Existing call sites that match `ServerDemandActive` / `Data` / `ServerDeactivateAll` continue to dispatch the same way.
auto-merge was automatically disabled
April 28, 2026 14:40
Head branch was pushed to by a user without write access
78effb3
into
Devolutions:master
10 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part of #1232.
Why
Three error sites in the connector previously printed only the expected PDU type, dropping the actual type the server sent on the floor. That made interop bugs harder to diagnose than necessary.
Concrete case from #1232 (originally surfaced via Haven#109 on a GNOME Remote Desktop server): when a client connects to GRD without advertising EGFX, GRD's `freerdp_peer::Capabilities()` callback fails — confirmed via journalctl on the server:
```
gnome-remote-desktop-daemon: [RDP] Client did not advertise support for the Graphics Pipeline, closing connection
[ERROR] [rdp_peer_handle_state_demand_active]:
[CONNECTION_STATE_CAPABILITIES_EXCHANGE_DEMAND_ACTIVE] freerdp_peer::Capabilities() callback failed
```
GRD then sends a Server Deactivate All PDU instead of Server Demand Active. The connector saw `ServerDeactivateAll`, didn't match `ServerDemandActive`, and reported "unexpected Share Control Pdu (expected ServerDemandActive)" — with no hint that the server had sent Deactivate All (which itself is a strong signal the server gave up during caps validation, e.g. because of a missing capability).
What
Replaces three `general_err!` sites in the connector with `reason_err!` calls that include `pdu.as_short_name()` (the helper already exists on `ShareControlPdu`):
Now diagnoses look like:
vs. the previous:
which immediately points at GRD-style "I rejected your caps and deactivated the share" rather than leaving callers to guess from network traces.
Scope
Verification