Skip to content

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

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

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

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

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.

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.
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.

2 participants