feat(server): handle SuppressOutput, RefreshRectangle, FrameAcknowledge PDUs#1239
Open
Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Open
Conversation
…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.
4 tasks
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.
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 pduand dropping them on the floor:This PR adds three default-implementation methods to
ConnectionHandlerso 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
ConnectionHandlerextension rather than internal handlingThe 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
RdpServerDisplayand 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-traitbranch in his fork (not yet a PR) that proposes a similarRdpServerSessionHandlertrait withconnect(),disconnect(), andsuppress_output(allow: bool)callbacks. The shapes are different (this PR extends the existingConnectionHandlerrather than introducing a new trait, and passes thedesktop_rectthrough 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 -vcleancargo xtask check lints -vclean (workspace, all-targets, with helper + __bench features)cargo xtask check tests -vpassesConnectionHandlerdecides 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."