Skip to content

feat(server): handle SuppressOutput, RefreshRectangle, FrameAcknowledge PDUs#1239

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/server-pdu-handlers
Open

feat(server): handle SuppressOutput, RefreshRectangle, FrameAcknowledge PDUs#1239
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/server-pdu-handlers

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

@glamberson Greg Lamberson (glamberson) commented Apr 30, 2026

Summary

These three slow-path share-data PDUs are well-defined parts of MS-RDPBCGR and modern Windows clients send them routinely, but the server core has been logging them as Unexpected share data pdu and dropping them on the floor:

  • SuppressOutput (2.2.11.3): client signals it does not want display updates while minimized or fully occluded.
  • RefreshRectangle (2.2.11.2): client asks the server to retransmit specific regions, typically after unminimize.
  • FrameAcknowledge (slow-path): legacy flow-control acknowledgment for non-EGFX update paths.

This PR adds three default-implementation methods to ConnectionHandler so server consumers can react to these signals (pause capture, mark regions dirty, throttle frame rate). Default impls ignore the PDU, preserving current behavior for consumers that have not opted in.

Why a ConnectionHandler extension rather than internal handling

The right behavior for SuppressOutput is to stop encoding frames at the capture source, not the wire. RefreshRectangle similarly needs to invalidate at the capture layer. The server core does not own the capture pipeline; the consumer (via RdpServerDisplay and friends) does. Putting hooks on the existing lifecycle trait keeps the consumer in control while the server core stops dropping useful signals.

The slow-path FrameAcknowledge is independent of EGFX's own DVC-channel FrameAcknowledge handled by ironrdp-egfx. Documented in the doc-comment to avoid confusion.

Adjacent work

Dietmar Maurer (@maurerdietmar) has a add-server-session-handler-trait branch in his fork (not yet a PR) that proposes a similar RdpServerSessionHandler trait with connect(), disconnect(), and suppress_output(allow: bool) callbacks. The shapes are different (this PR extends the existing ConnectionHandler rather than introducing a new trait, and passes the desktop_rect through rather than collapsing to a bool) but the underlying need is the same. Happy to align on naming or structure if Dietmar Maurer (@maurerdietmar) prefers his shape, this PR is the smallest change that closes the dropped-PDU gap.

Coordinates with #1244

#1244 changes the signature of ConnectionHandler::on_disconnected (from Option<&anyhow::Error> to Option<&ServerError>) as part of the typed-error migration (#1242 / #1209). The three new methods this PR adds are independent of on_disconnected and unaffected by that signature change. Either PR can land first; whichever lands second is a trivial rebase to the new trait shape.

Test plan

  • cargo xtask check fmt -v clean
  • cargo xtask check lints -v clean (workspace, all-targets, with helper + __bench features)
  • cargo xtask check tests -v passes
  • Manual smoke against mstsc (suppress output observed when minimizing the RDP window) is downstream test plan; consumer of ConnectionHandler decides what to do with the signal.

Out of scope

Spec-compliant behavior for the three PDUs (actually pausing capture under SuppressOutput, marking regions dirty under RefreshRectangle, throttling under FrameAcknowledge) lives in the consumer. This PR is "stop dropping the signal," not "implement the behavior."

…ge PDUs

These three slow-path share-data PDUs were previously logged as
'Unexpected share data pdu' and dropped. They are well-defined parts
of MS-RDPBCGR and modern Windows clients send them routinely:

- SuppressOutput (2.2.11.3): client signals it does not want display
  updates while minimized or fully occluded.
- RefreshRectangle (2.2.11.2): client asks the server to retransmit
  specific regions, typically after unminimize.
- FrameAcknowledge: slow-path flow-control acknowledgment.

Add three default-implementation methods to ConnectionHandler so
server consumers can react to these signals (pause capture, mark
regions dirty, throttle frame rate). Default impls ignore the PDU,
preserving current behavior for consumers that have not opted in.

Note: the EGFX Frame Acknowledge over DVC is unrelated and stays
handled by ironrdp-egfx; this commit only covers the slow-path
ShareDataPdu variant carried over the IO channel.
@glamberson
Copy link
Copy Markdown
Contributor Author

Rebased onto current master. Force-pushed `77b4e612` -> `a4898b78`.

20 days behind master since 2026-04-30; rebase clean (no conflicts). Author normalized lamco-office -> Greg Lamberson greg@lamco.io. Body extended with a "Coordinates with #1244" section mirroring the note already on #1244 — either PR can land first, the other rebases trivially since the three methods this PR adds are independent of `on_disconnected`'s signature change.

All five xtask gates green on the new head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant