feat: wire RDPDR backends into client connections by mamoreau-devolutions · Pull Request #1600 · Devolutions/IronRDP

Marc-André Moreau (mamoreau-devolutions) · 2026-08-09T13:30:56Z

Build a fresh RDPDR backend product for every connection attempt.

Attach RDPDR only when its product has filesystem devices, advertise
RDPSND for Windows interoperability, and deliver deferred responses.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

Build a fresh RDPDR backend product for every connection attempt. Attach RDPDR only when its product has filesystem devices, advertise RDPSND for Windows interoperability, and deliver deferred responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds per-connection RDPDR backend construction and integrates drive redirection into client connections.

Changes:

Introduces RDPDR factory, product, and drive APIs.
Wires RDPDR/RDPSND channels across transports.
Polls deferred RDPDR responses with test coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`crates/ironrdp-rdpdr/src/lib.rs`	Re-exports backend factory APIs.
`crates/ironrdp-rdpdr/src/backend/mod.rs`	Defines factory, product, and drive types.
`crates/ironrdp-client/src/rdp.rs`	Builds and drives RDPDR channels per connection.
`crates/ironrdp-client/Cargo.toml`	Adds RDPSND to the RDPDR feature.

Expose the Windows backend through the portable factory contract so client integrations can create a fresh backend and matching device announcement for each connection attempt. Mark the re-exported RDPDR dependency as public for semver tracking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

#1791) …sion Regression introduced in 1fbc9ba ("feat: wire RDPDR backends into client connections", #1600). Before that commit, the RDPDR static channel was always attached (using NoopRdpdrBackend when no real backend was needed), regardless of whether a native RdpdrBackendFactory was available. #1600 changed this so the channel is only attached when build_rdpdr_channel() returns Some(_), i.e. when a factory is supplied and produces at least one device. On platforms without a native RDPDR factory (currently: everything except Windows, since ironrdp-rdpdr-native is a target `cfg(windows)` dependencies entry and attach_windows_rdpdr_backend() is gated `#[cfg(windows)]`), rdpdr_factory is always None, so the RDPDR channel is now never attached at all. This silently broke RDPSND audio playback on the client side: with the RDPDR channel entirely absent from the static channel set, the locally created "Remote Audio" playback device stopped appearing (confirmed via git bisect, first bad commit 1fbc9ba, reproduced on Linux). The sound feature itself, the rdpsnd_backend_kind() selection, and the enable_audio_playback flag in the Client Info PDU are unaffected by #1600 and behave the same before/after — only the presence of the RDPDR channel differs. Fix: fall back to attaching Rdpdr::new(NoopRdpdrBackend, ...) whenever config.channels.rdpdr.enabled is true but build_rdpdr_channel() returned None, restoring pre-#1600 behavior on platforms without a native RDPDR factory.

* feat(client): enforce gateway redirection policy (#1760) Honor restrictive gateway redirection flags without enabling new channels. * feat(mstsgu): handle gateway consent messages (#1762) Decode gateway consent messages as UTF-16LE during tunnel creation. Accept consent by default, and let callbacks decline it before tunnel authorization and channel setup. * feat(mstsgu): add extended-auth packet codecs (#1761) Preserve advertised handshake extended-auth flags, including unknown bits, and encode/decode extended-auth packet blobs with exact lengths. Add wire vectors for flag combinations, packet round trips, malformed blobs, and u16 blob boundaries. * feat(activex): prompt for gateway credentials (#1763) Prompt for non-persistent credentials for a configured RD Gateway. Keep them scoped to the connection attempt and leave cancellation disconnected. * feat(mstsgu): preserve gateway HRESULT errors (#1774) Preserve nonzero gateway control HRESULTs for caller diagnostics. Known codes retain established labels; unknown codes remain lossless. * feat(mstsgu): apply certificate validation to gateways (#1775) Apply the RDP client's certificate-validation policy and callback to every gateway HTTPS connection. Existing gateway callers retain their compatibility default. * feat(rdpdr): make the PDU layer bidirectional for a future server (#1779) ## Summary - ironrdp-rdpdr only decodes what a client receives from a server (6 PacketIds) and only encodes what a client sends to a server. A server needs the opposite. This makes the PDU layer bidirectional: decode() added to every response type the client currently only encodes, and encode() added to every request type the client currently only decodes. - CoreCapability::decode already branched on packet_id for both directions and just needed the client-capability arm wired into RdpdrPdu::decode_body, alongside three new decodes (ClientNameRequest, ClientDeviceListAnnounce, ClientDeviceListRemove). - CoreDeviceIoCompletion is deliberately not wired into the automatic RdpdrPdu::decode dispatch. Fourteen different response variants share that one PacketId (see the header() method's match), and nothing on the wire disambiguates which one a given completion answers. That requires knowing the MajorFunction (and, for DirectoryControl, MinorFunction) of the original request, which only a future IRP-tracking layer will have. Added RdpdrPdu::decode_io_completion as an explicit entry point for that layer to call once it exists, taking the major/minor function and, for the three completions whose body layout depends on it, the FileInformationClass the original request asked for. - DeviceControlResponse's output_buffer is a Box<dyn rpce::Encode> on the encode side, since it carries IOCTL-specific NDR content only the issuer of the original request can interpret. That can't be decoded back into a concrete type. Decode reads it as opaque bytes through a private RawOutputBuffer implementing the same trait, so the decoded value has the same shape without pretending to know a structure it can't actually know. - Filled two response-decode gaps that were there before this PR touched the file, found while checking the new decode paths against MS-RDPEFS directly: FileStandardInformation and FileAttributeTagInformation are two of the exact three classes 2.2.3.3.8 allows for a Query Information response (the third, Basic, already round-tripped) and had encode with no decode. FileDirectoryInformation, FileFullDirectoryInformation, FileBothDirectoryInformation, and FileNamesInformation are the complete set 2.2.3.3.10 allows for a Query Directory response and were entirely unwired in FileInformationClass::decode, so directory listing had no working decode path at all. - VersionAndIdPdu::decode always tagged PacketId::CoreClientidConfirm as ServerClientIdConfirm. Per MS-RDPEFS 2.2.1.1, that PacketId is shared between Server Client ID Confirm (2.2.2.6) and Client Announce Reply (2.2.2.3): correct for a client, which never decodes its own reply, but wrong for a server decoding the client's reply. Added decode_client_announce_reply as an explicit entry point for that case, mirroring decode_io_completion's precedent for the same class of PacketId ambiguity, and left decode() itself untouched. - DeviceControlRequest<T> had decode and decode_with_input_buffer but no encode, the one server-sent I/O request missing it. Adding encode required tightening IoCtlCode to also require Into<u32>, which surfaced that ScardIoCtlCode (the trait's only other implementor) was missing that conversion too. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Round-trip tests added for every new decode()/encode() pair in ironrdp-testsuite-core/tests/rdpdr/mod.rs, including one per Query Information and Query Directory class, plus the two additions above. ## Notes PDU-codec layer only. The RdpdrServer state machine, IRP tracking, and ironrdp-server integration (a factory trait plus attach_channels wiring, the same shape as the recently landed RdpeiServerFactory) are a follow-up PR once this lands, so the server-relevant surface can be reviewed in a size a reviewer can actually hold in their head at once. * feat(server)!: give mouse button events a position, fix X1/X2 and middle-button/hwheel handling (#1769) ## Summary - Fixes #1466: `MouseEvent`'s button variants were position-less, so `From<MousePdu>` and `From<MouseXPdu>` silently discarded the wire PDU's x/y whenever a button flag was set (position only survived for a pure Move). A client that sends a tap as a single button PDU with no preceding Move (confirmed: Windows App on iOS/iPadOS, touch mode) clicks at the stale cursor position rather than where it tapped. - While fixing that, found two more bugs in the same conversion layer, both confirmed against MS-RDPBCGR: `From<MousePdu>` never checked `MIDDLE_BUTTON_OR_WHEEL` or `HORIZONTAL_WHEEL`, so middle-click and horizontal wheel silently fell through to the Move fallback. `From<MouseXPdu>` mapped `PointerXFlags::BUTTON1`/`BUTTON2` to Left/Right, but per 2.2.8.1.2.2.4 these are "Extended mouse button 1 (also referred to as button 4)" and "...2 (...button 5)", the X1/X2 side buttons. `MouseRelPdu`'s own sibling `From` impl already distinguishes these correctly via `XBUTTON1`/`XBUTTON2`, so this was an inconsistency, not a deliberate choice. - Checked FreeRDP, xrdp, KDE krdp, and GNOME Remote Desktop before settling on a shape. All four check explicit MOVE flags rather than falling through on else, all four handle middle button and both wheel flags, none conflate X1/X2 with primary buttons. xrdp's own source has a maintainer comment confirming real clients exercise the HWHEEL gap: "As mstsc does MOUSE not MOUSEX for horizontal scrolling, PTRFLAGS_HWHEEL must be handled here." - `MouseEvent::Button { x, y, button, pressed }` replaces the ten position-less `LeftPressed`/`LeftReleased`/.../`Button5Released` variants, scoped to the PDU types that actually carry absolute position (`MousePdu`, `MouseXPdu`, `ainput::MousePdu`, which already carried x/y on every event but never applied it to buttons either). `MouseRelPdu` gets `MouseEvent::ButtonRel { button, pressed }` instead, since it only has relative deltas. Both share a new `MouseButton` enum (`Left`/`Right`/`Middle`/`X1`/`X2`). Added `MouseEvent::HorizontalScroll` to pair with the existing `VerticalScroll`. Breaking anyway, so `MouseEvent` and `MouseButton` are both `#[non_exhaustive]`. - There's independent, unmerged prior art for the middle-button half of this on branch `probakowski/xrdp` (commit `e924272`, ad hoc xrdp-interop debugging, never opened as a PR) that arrived at the same `MIDDLE_BUTTON_OR_WHEEL` fix. Crediting it here since I found it while researching this. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. ## Notes Single file, `crates/ironrdp-server/src/handler.rs`. Grepped the whole workspace for `MouseEvent::` usage; this file is the only one that constructs or matches it, no other call sites need updating for this PR. Downstream, I'm updating my own `lamco-rdp-input`/`lamco-rdp-server` consumers to the new shape as a follow-up. * feat(server): wire MS-RDPEI server-side into ironrdp-server (#1773) ## Summary - MS-RDPEI (multitouch and pen) got server-side plumbing in `ironrdp-rdpei` (the recent Input DVC PR), but nothing in `ironrdp-server` ever registers the `RdpeiServer`/`RdpeiHandler` it added. Grepped the whole crate for `rdpei` before starting: zero references. This wires it in. - Follows the exact shape of the existing `SoundServerFactory`/`with_sound_factory` (the simplest of the three existing opt-in factories, and RDPEI's needs are equally simple): a new `RdpeiServerFactory` trait, a `with_rdpei_factory` builder method, and registration in `attach_channels` alongside the other optional DVC factories. The factory builds and returns the whole configured `RdpeiServer`, not just a handler, so it can reach `with_protocol_version`/`with_supported_features` to advertise non-default capabilities. - Re-exports everything a downstream `RdpeiHandler` implementation needs from the crate root: `RdpeiServerFactory`, `RdpeiHandler`, `RdpeiServer`, and the touch/pen/ready PDU types plus the nested payload types their callbacks embed (`CsReadyFlags`, `PenContact` and its fields/flags, `PenFrame`, `TouchContactFields`/`TouchContactDataFlags`). ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Adds a regression test in `ironrdp-testsuite-core` (`ironrdp-server` has `[lib] test = false`, so an inline test would never run in CI) covering that a registered `RdpeiServerFactory` is invoked during per-connection channel setup. ## Notes Two crates: `ironrdp-server` (`Cargo.toml`, `Cargo.lock`, `builder.rs`, `lib.rs`, `server.rs`, the new `rdpei.rs`) and `ironrdp-testsuite-core` (`Cargo.toml`, `tests/server/mod.rs`, the new `tests/server/rdpei.rs`). `RdpServer::new` is `pub(crate)`, only ever called from the builder, so adding the new parameter there has no external API impact beyond the new builder method itself. * feat(cliprdr): add a chunked file-contents fetch primitive (#1742) ## Summary - MS-RDPECLIP's file-contents protocol is receiver-driven: fetching a file means sending a sequence of byte-range FileContentsRequests and reassembling the FileContentsResponses. Nothing in this crate helps with that sequencing, so every consumer that wants a whole file ends up writing its own request/response loop by hand. - Adds ChunkedFetch: a small state machine that produces the next request to send and consumes each response, tracking offset and buffered bytes so the caller doesn't have to. The size phase is optional: construct with a known size (the common case, since it's usually already available from an earlier FileGroupDescriptorW exchange) to skip straight to range requests, or without one to query it first. - Every request carries an explicit clip_data_id, the id CliprdrBackend::on_remote_file_list already hands the caller for the file's locked snapshot. request_file_contents can auto-fill this from current_lock_id when left unset, but a fetch spans multiple requests and a FormatList between them can expire the old lock and install a new one under the same field, so auto-filling would let a fetch already in flight silently retarget to whichever lock happens to be current at send time. - A caller-provided max_total_size bounds how large a file this type will try to buffer in memory, checked as soon as the size is known (either the caller-supplied total_size, or the peer's own SIZE response), independent of what that size claims to be. - Two defensive cases beyond the happy path: an empty range response before the file is complete fails the fetch rather than re-requesting the same range forever, since the protocol has no "not ready yet" signal that would legitimately produce one, and a response longer than what's left is clamped rather than trusted, since accepting it as-is would grow the buffer past the file's total size. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Tests live in ironrdp-testsuite-core since this crate sets [lib] test = false. ## Notes No public API break: this is a new module with entirely new public types, no existing signature changes. * feat(error)!: record byte offset on decode and encode error variants (#1266) ## Summary Records a byte offset on every `DecodeErrorKind` and `EncodeErrorKind` variant that can know one, so decode and encode errors surface the position in the input stream where the failure was detected. Reshaped twice after review; see "Review history" below if you reviewed an earlier shape. Contributes to the structured-fuzzing roadmap in #1120 by giving crash-replay analysis and Wireshark-style malformed-PDU reporting the byte-offset dimension that source `Location` (#1262) alone does not provide. ## API Variants that gain `offset: Option<usize>`: - `DecodeErrorKind::NotEnoughBytes { received, expected, offset }` - `DecodeErrorKind::InvalidField { field, reason, offset }` - `DecodeErrorKind::UnexpectedMessageType { got, offset }` - `DecodeErrorKind::UnsupportedVersion { got, offset }` - `DecodeErrorKind::UnsupportedValue { name, value, offset }` - `EncodeErrorKind` mirrors the same shape for the encode side `*::Other` is unchanged. `OtherErr` and `other_err!` keep their current signatures. Five traits take an additional `offset: Option<usize>` parameter, and `invalid_field_err_with_source` gains one in the same position: - `NotEnoughBytesErr::not_enough_bytes(context, received, expected, offset)` - `InvalidFieldErr::invalid_field(context, field, reason, offset)` - `UnexpectedMessageTypeErr::unexpected_message_type(context, got, offset)` - `UnsupportedVersionErr::unsupported_version(context, got, offset)` - `UnsupportedValueErr::unsupported_value(context, name, value, offset)` ## What `None` means `None` is not a placeholder for a position that happened to be unavailable. It marks errors that have no position in the input stream at all: a constructor validating its arguments, an integer conversion, a cache lookup that missed. Those are permanent, so `Display` omits the position rather than naming a byte the failure did not occur at, and a structured consumer can distinguish them from a genuine failure at byte zero. The `*_err!` macros therefore have three forms: - `in: $cursor` derives the position from `$cursor.pos()`. Preferred whenever a `ReadCursor` or `WriteCursor` is in scope. - `at: $offset` passes a known position explicitly. - the bare form records no position, for errors that do not arise from a stream. `cast_length!` and `cast_int!` default to the bare form, since an integer conversion does not happen at a stream offset, and accept `in:` or `at:` for the cases where one is genuinely known. ## `ensure_size!` reports the cursor position `ensure_size!` and `ensure_fixed_part_size!` previously discarded the position and recorded zero. That mattered more than it looks: **1033 of their call sites in this workspace pass a cursor**, so a fixed-part or size check failing after earlier fields had been consumed was attributed to the start of the PDU rather than to where it happened. The committed expect-test snapshots show the correction directly. Several that recorded `offset: 0` now record `Some(16)` and `Some(20)`. There is one caller that genuinely has no cursor, `encode_string` in `ironrdp-pdu/src/utils.rs`, which writes into a `&mut [u8]`. Rather than let `in:` silently yield a position-less error for it, slices use a distinct `ensure_size!(slice: ..)` form, so passing a slice where a cursor was meant is a compile error. ## Display behavior With a position: ``` invalid `serverRandom` at offset 47: bad length not enough bytes provided to decode at offset 12: received 2 bytes, expected 4 bytes invalid message type (3) at offset 35 ``` Without one, the position is omitted entirely rather than rendered as zero: ``` invalid `NumMonitors`: Too many monitors out of range integral type conversion ``` `Other` and the `[ctx @ file:line]` context and location prefix from #1262 are unchanged. ## Review history Filed 2026-05-12 with an `Option<ErrorPosition>` slot on `Error<Kind>`'s `ErrorMeta` plus composable `at_offset` / `in_field` builders. Reshaped after the first review on two grounds: the foundational `ironrdp-error` crate stays kind-agnostic, so position metadata belongs on the decode and encode variants rather than in `ErrorMeta`; and `Error::set_context` already answers the "which symbolic field" question, so no parallel `in_field` channel was introduced. The second review found that the reshaped version used `0` as both the unknown-offset sentinel and a valid position, and that `ensure_size!` discarded cursor positions it had. Both are addressed above: `Option<usize>` separates the two meanings, and `ensure_size!` reports the position. ## Stacking #1275 is stacked on this and populates offsets at the call sites that have a cursor. It is rebuilt on top of this shape rather than rebased, because the `at: 0` sites its patch targeted no longer exist in that form. Rebased onto current master 2026-08-05. One conflict, in `crates/ironrdp-rdpeusb/src/pdu/completion/mod.rs`: #1456 replaced a runtime payload invariant with a typed enum and removed the check, so the annotation this PR had added there was dropped with it. The file's other two annotations are unaffected. Rebased again onto `b911eaba` on 2026-08-07, no conflicts. Rebased onto `a05ceab5` on 2026-08-10. One conflict, in `crates/ironrdp-pdu/src/nego.rs`: #1582 implemented `RDP_NEG_CORRELATION_INFO` and deleted both of the `CORRECTION_INFO_PRESENT flag is set, but not supported by IronRDP` call sites, so the two annotations this PR had added to them went with the sites. The file's other two annotations are unaffected, and the six error sites #1582 added in their place are annotated in #1275 rather than here. ## Why this is one PR The automation flags this as size/XL and asks for it to be split. It is already split, along the only seam that yields two pull requests which each compile: this PR changes the type and the arity, #1275 populates real offsets at the call sites that have a cursor. What is left here does not divide further. The type and macro change is four files in `ironrdp-core`: `decode.rs`, `encode.rs`, `error.rs` and `macros.rs`. The other 53 files, across 13 crates, are call sites that have to move in the same commit, because adding a field to a struct variant breaks exhaustive destructuring in every crate that matches on it. `ironrdp-activex` failed with `E0027` the first time this branch was built against it. A pull request holding the four files without the fifty-three does not build, and neither does one holding part of the fifty-three. The note suggests stacked pull requests and adds that they require every branch to live in this repository. This branch is on a fork, so that path is closed, and its stated fallback of separate pull requests is the case above. ## CI `cargo xtask check fmt | lints | locks | typos | tests` and `cargo xtask wasm check` all pass on the pinned 1.94.1 toolchain, with the fuzz workspace built before the lock check. `cargo semver-checks -p ironrdp-core` reports the three expected breaking classes (variant field added, function arity, trait method arity); master baselines clean, so all three belong to this PR and are covered by the `!`. Refs #1257 (Change 2), #1120. * feat(error)!: populate decode/encode error offsets from cursor positions (#1275) ## Summary The workspace sweep that follows #1266. Decode and encode error construction sites now pass the cursor, so the reported position is the byte the decoder or encoder actually stopped at. Stacked on #1266 and merges after it. ## What "no position" means here #1266 makes `offset` an `Option<usize>` where `None` means the error has no position in the input stream at all, rather than a position that happened to be unavailable. This PR is the other half of that: it walks the workspace and gives a real position to every site that has one, so the sites left reporting `None` are the ones that genuinely never had one. Those are constructors validating their arguments, integer conversions, cache lookups that missed, accessors on already-decoded structures, and the declared-size checks described below. They report nothing rather than byte zero, and that is now their permanent answer rather than a gap awaiting another sweep. There are no `at: 0` sites left anywhere in the workspace. ## The rule The position is attached where the cursor identifies the bytes being complained about. It is omitted where the complaint is about a size the peer declared, computed from data already consumed, because there the cursor points at a byte that is not the problem. `ConnectionRequest::x224_body_decode` in `ironrdp-pdu/src/nego.rs` has both kinds next to each other. Its four `TPDU header variable part` checks all test a quantity derived from the declared `variable_part_size`, so all four stay position-less. The message-type check, the `RDP_NEG_REQ` length check and the two `RDP_NEG_CORRELATION_INFO` header checks each test bytes they just read, so each reports where it stopped. ## Coverage 1738 construction sites derive their position from a cursor via `in: <cursor>` and 10 more pass a known offset with `at:`. 117 remain position-less, 24 of them `cast_length!` and `cast_int!` which default to that form. This PR is what moves 604 of them. The base already had 1143 sites passing a cursor, since `ensure_size!(in: ..)` was the existing idiom, and 721 reporting nothing. Those counts come from matching each invocation of `invalid_field_err!`, `not_enough_bytes_err!`, `unexpected_message_type_err!`, `unsupported_version_err!`, `unsupported_value_err!`, `cast_length!`, `cast_int!`, `ensure_size!` and `ensure_fixed_part_size!` across `crates/` with balanced parentheses, so multi-line invocations are counted once. `ironrdp-graphics` has its own crate-local `ensure_size!` returning `RleError`, on `from:`, `into:` and `dst:` arms; those 22 sites are a different macro and are not in scope. `ironrdp-rpc` arrived after this PR was filed and is included: all 76 of its `ensure_size!` sites pass a cursor, 23 of its 24 `invalid_field_err!` sites carry a position, and all 7 of its `cast_length!` sites pass `in: dst`. The one that does not is a connection-state tag check, which has no stream position to report. Three local helper macros hardcoded an offset and were invoked from call sites with differing cursor availability. Each was adapted so the caller decides: - `validate_dimensions!` in `ironrdp-displaycontrol/src/pdu/mod.rs`. Two callers: the constructor has no stream, decode passes `src`. - `check_masks_alignment!` in `ironrdp-pdu/src/basic_output/pointer/mod.rs`. Four callers, all inside encode or decode methods. - `per_field_err!` in `ironrdp-pdu/src/mcs.rs`. This one captures the cursor position into the returned closure, so a PER read or write failure carries the offset of the field that failed rather than of the enclosing PDU. All 26 callers pass `in: src` or `in: dst` depending on which chain they sit in. ## `encode_string` takes a cursor `encode_string` previously took `dst: &mut [u8]`, which meant its size check had no position to report. It now takes `&mut WriteCursor<'_>`, so the check reports where in the output the shortfall occurred. Callers that were passing a slice pass the cursor directly and use `write_padding!` for fixed-width fields. This is the only user-visible API change in this PR. After it, the `ensure_size!(slice: ..)` form that #1266 introduces has no remaining caller in the workspace; it stays available for downstream code that genuinely holds a slice. ## Test snapshots Expect-test snapshots updated to record real positions. The interesting ones are the entries that previously read `offset: 0` and now read `Some(16)` or `Some(20)`: those are size checks failing after earlier fields were consumed, so the committed tests had been recording the wrong position rather than merely an uninformative one. ## Diff 129 files, +1566 / -1238, one commit on top of #1266. That is the whole stack as GitHub shows it; this PR's own commit is 100 files of it. Rebased onto the current #1266 tip 2026-08-05, replayed with `git rebase --onto` and no conflicts. The one-line change against the earlier diffstat is a `rdpeusb` annotation dropped in #1266, where #1456 replaced a runtime payload invariant with a typed enum and removed the call site being annotated. Replayed onto the current #1266 tip again on 2026-08-10, no conflicts. #1582 landed `RDP_NEG_CORRELATION_INFO` support in `nego.rs` while this was queued, adding six error sites to a file the sweep already covers. Three are annotated here under the rule above; the three that test a derived size stay position-less, alongside the one that was already there. Rebased onto current master again 2026-08-19. One real conflict: `ClientCoreData::decode` in `ironrdp-pdu/src/gcc/core_data/client.rs`. My own #1689 (merged the day before) had already converted `keyboardType` from a closed enum to a permissive wrapper so an undocumented-but-valid value like Korean keyboards round-trips instead of failing the whole Client Core Data parse. This PR's stack still carried the pre-#1689 fallible read for that field; resolved by keeping #1689's permissive read and dropping the now-obsolete offset annotation there, since there is no error path left to annotate. Separately, `ironrdp-rdpemt` (landed via #1626 the day after this PR's previous head was last verified) calls `DecodeError::invalid_field` and `DecodeError::unexpected_message_type` directly rather than through the `invalid_field_err!` macro, so the earlier sweep never saw those 13 call sites across 6 files. Added `Some(src.pos())` to each, matching the same convention used everywhere else in this PR; every site sits inside a `decode(src: &mut ReadCursor<'_>)` function so the cursor was always in scope. ## CI `cargo xtask check fmt | lints | locks | typos | tests` and `cargo xtask wasm check` all pass on the pinned 1.94.1 toolchain, with the fuzz workspace built before the lock check. Refs #1257 (Change 2), #1120. * feat(server): add rdpeusb server integration (#1417) Related discussion: #1516. *Currently blocked by:* - #1682 — `ironrdp-usb`, the protocol-independent USB layer - #1683 — `ironrdp-rdpeusb::usb`, the RDPEUSB adapter, which is also blocked by #1682 > **Reviewing this PR:** because #1682 and #1683 have not merged yet, the *Files changed* tab shows the cumulative diff. The incremental diff for this PR alone is here: > https://github.com/uchouT/IronRDP/compare/ironrdp-rdpeusb-usb...ironrdp-server This PR carries the design rationale for all three, since it is the first place where the whole stack is visible at once. [qemu-rdp](https://gitlab.com/uchouT/qemu-display/-/tree/feat/usbredir) now redirects USB mass storage over RDP end to end, verified against a FreeRDP client built from source. These three PRs split the reusable part out into IronRDP, so that any RDP server can offer USB redirection instead of reimplementing the channel. ## The layering #1516 proposed splitting this into three layers. The implementation followed that split; this section records what each layer ended up owning. ```text ironrdp-usb USB semantics: descriptors, control requests, transfers. ↑ Knows nothing about any transport. ironrdp-rdpeusb RDPEUSB adapter: TS_URB forms, USBD conventions, ↑ capability negotiation, channel state. ironrdp-server Server facade: request lifetime, cancellation, routing. ↑ Speaks ironrdp-usb types outward. application A usbredir bridge, like qemu-rdp, macrdp, or any other consumer. ``` The rule used to place a concept is: **it may live in a layer only if its contract can be explained using that layer's vocabulary alone** — USB, or MS-RDPEUSB, or a generic server facade. A concept that can only be justified by what one downstream consumer happens to need stays in that consumer. That rule is not free, so two worked examples: - `CompletionData` (#1683) and `InterfaceAlloc` (this PR) were both added because the server layer needed them, and both stayed in `ironrdp-rdpeusb` because both are explainable purely in RDPEUSB terms: one is the union of the four completion result types the channel defines, the other allocates the interface IDs RDPEUSB requires per redirected device. Being needed downstream was neither sufficient on its own nor disqualifying. - Going the other way: usbredir packet types and IDs, its synchronous/asynchronous ordering rules, its inflight scheduling, and its stream policy all stayed out of these crates, because none of them can be stated without naming usbredir. ## What the facade exposes Behind an optional `usb` feature: - **`DeviceFactory`** creates a per-device backend when the client opens a redirection channel; **`UsbRedirDevice`** receives the device announcement, device text, and channel close. - **`UsbDeviceHandle`** drives the device in `ironrdp-usb` terms: `get_descriptor`, `get_configuration`, `select_configuration`, `get_interface`, `select_interface`, `control_transfer`, `bulk_transfer`, `interrupt_transfer`, `isochronous_transfer`, `clear_halt`, `reset_device`, and `current_frame_number`, plus `query_device_text_request` and `retract_request`. - **`PendingRequest`**, **`PendingHandle`**, **`CompletionFut`**, and **`RawPending`** own the request lifetime: a caller can await a completion, hold a cancellation handle, or drop the request. Dropping a submitted request emits `CANCEL_REQUEST`, so an abandoned transfer does not stay in flight on the client. - **`RdpServerBuilder::with_usb_factory`** opts a server in. The channel is created only when the client negotiates the capability. No `TS_URB`, RDPEUSB processor, or `ServerEvent` wiring appears in any of those signatures. ## What stays in this layer, and why Request identifiers, the pending-request map, cancellation, device closure, and completion routing all live here rather than in `ironrdp-rdpeusb`. Keeping them out of the protocol crate is what lets #1683 stay sans-I/O and independently testable, and it means a second consumer of the facade gets the lifetime handling for free without inheriting RDPEUSB details. ## Also in this PR `refactor(rdpeusb): expose InterfaceAlloc at the crate root` — `InterfaceAlloc` was a private helper of `UrbdrcListener`, so a server driving the channel had no way to allocate the interface IDs it must assign to redirected devices. It is now public at the crate root, with the initial value expressed through `Default`. This is a pure relocation: the allocation range and the exhaustion behaviour are unchanged. ## TODOs Stated up front rather than left to be discovered: 1. **The `usb` feature is not linted in CI.**. Locally, `cargo clippy -p ironrdp-server --features usb --all-targets -- -D warnings` is clean. Whether to add the feature to the CI matrix is a maintainer call, so I have not touched CI configuration here. 2. **RDPEUSB types still reaches the consumer.** `RdpUsbDeviceAnnounceInfo` carries the raw `DeviceAnnounce`, so a consumer that wants the device speed reads `DeviceSpeed`, an RDPEUSB PDU type. The goal is for a downstream bridge to need no `ironrdp-rdpeusb` dependency at all. I would rather agree on the facade shape than guess at it, so it is left as a follow-up. --------- Signed-off-by: uchouT <i@uchout.moe> * fix(client): restore Noop RDPDR channel fallback, fixing audio regres… (#1791) …sion Regression introduced in 1fbc9bab ("feat: wire RDPDR backends into client connections", #1600). Before that commit, the RDPDR static channel was always attached (using NoopRdpdrBackend when no real backend was needed), regardless of whether a native RdpdrBackendFactory was available. #1600 changed this so the channel is only attached when build_rdpdr_channel() returns Some(_), i.e. when a factory is supplied and produces at least one device. On platforms without a native RDPDR factory (currently: everything except Windows, since ironrdp-rdpdr-native is a target `cfg(windows)` dependencies entry and attach_windows_rdpdr_backend() is gated `#[cfg(windows)]`), rdpdr_factory is always None, so the RDPDR channel is now never attached at all. This silently broke RDPSND audio playback on the client side: with the RDPDR channel entirely absent from the static channel set, the locally created "Remote Audio" playback device stopped appearing (confirmed via git bisect, first bad commit 1fbc9bab, reproduced on Linux). The sound feature itself, the rdpsnd_backend_kind() selection, and the enable_audio_playback flag in the Client Info PDU are unaffected by #1600 and behave the same before/after — only the presence of the RDPDR channel differs. Fix: fall back to attaching Rdpdr::new(NoopRdpdrBackend, ...) whenever config.channels.rdpdr.enabled is true but build_rdpdr_channel() returned None, restoring pre-#1600 behavior on platforms without a native RDPDR factory. * feat(server)!: introduce typed ServerError on the public API boundary (#1242) ## Summary First in a staged migration toward a typed public error story for `ironrdp-server`, addressing #1209. Introduces: ```rust pub type ServerError = ironrdp_error::Error<ServerErrorKind>; pub type ServerResult<T> = Result<T, ServerError>; ``` `ServerErrorKind` is a `#[non_exhaustive]` enum with concretely typed variants (`Encode`, `Io`, `Channel`, `Unsupported`, `Reason`, `Custom`). Sources are attached through `ironrdp_error::Error::with_source` rather than embedded as `Box<dyn Error>` in variant data. This mirrors the shape of `ConnectorErrorKind` in `ironrdp-connector`, so the connection-management layer stays internally consistent. ## Why this shape The audit before drafting found that `ironrdp-connector`, `ironrdp-session`, `ironrdp-pdu`, `ironrdp-mstsgu`, and `ironrdp-core` (`EncodeError`/`DecodeError`) all use the same `ironrdp_error::Error<Kind>` pattern. The `thiserror`-based bare enums elsewhere in the workspace (`GccError`, `BulkError`, etc.) are leaf-layer errors at the PDU/codec level; the connection-management layer sister crate to `ironrdp-server` is the right template. ## Scope of this PR Public function signatures only: - `RdpServer::run` → `ServerResult<()>` - `RdpServer::run_connection<S>` → `ServerResult<()>` - `RdpServer::run_connection_with<S>` → `ServerResult<()>` - `TlsIdentityCtx::init_from_paths` → `ServerResult<Self>` - `TlsIdentityCtx::make_acceptor` → `ServerResult<TlsAcceptor>` - `EchoServerHandle::send_request` → `ServerResult<()>` Internal call sites continue to use `anyhow::Result`. A private `from_anyhow_with_context` helper bridges at the public boundary, tagging each call site with the operation that failed. The `ConnectionHandler::on_disconnected(error: Option<&anyhow::Error>)` parameter is unchanged in this PR. `run_connection_with` keeps its anyhow body in a private `run_connection_with_inner`, which the accept loop calls directly so `on_disconnected` still receives an `anyhow::Error`. That helper is transitional and its removal is inside this stack rather than deferred indefinitely: #1244 converts `on_disconnected` to `ServerError` and deletes it, with the accept loop calling `run_connection` from there. `TlsIdentityCtx::init_from_paths` and `make_acceptor` use the same wrapper-plus-private-body shape, matching the `run` / `run_inner` pair this PR already introduces. ## Companion ext traits ```rust pub trait ServerErrorExt { fn encode(error: EncodeError) -> Self; fn io(context: &'static str, error: io::Error) -> Self; fn channel(context: &'static str) -> Self; fn unsupported(context: &'static str) -> Self; fn reason(context: &'static str, reason: impl Into<String>) -> Self; fn custom<E>(context: &'static str, error: E) -> Self where E: core::error::Error + Sync + Send + 'static; } pub trait ServerResultExt { fn with_context(self, context: &'static str) -> Self; } ``` Mirrors `ConnectorErrorExt` / `ConnectorResultExt`, trimmed to the constructors and the one result-side helper that have call sites in this stack (`decode` and `with_source` did not, so neither shipped). ## Stack ordering This is step 1 of 4. The full chain (must merge in PR-number order): | Step | PR | Scope | |------|-----|-------| | **1** | **this PR** | Add `ServerError` / `ServerErrorKind` / ext traits, convert 5 public functions, internal anyhow stays via private bridge | | 2 | #1243 | Convert encoder/helper/echo internals (anyhow → typed) | | 3 | #1244 | Convert server.rs internals + `on_disconnected` parameter (breaking) | | 4 | #1245 | Convert display traits, drop anyhow dep, finish (breaking) | All four close #1209 together. Each step is independently reviewable but is rebased onto its predecessor as a Stacked branch; please merge in the order above so the rebases land cleanly. ## Breaking change Marked with `!` in the conventional commit. Consumers of the five listed public functions need to update their `Result` types. Pre-1.0, and per @elmarco's note on #1209: "I am ok with breaking API at this point :)". ## 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 - `cargo build --workspace --all-targets` clean (one example consumer in `crates/ironrdp/examples/server.rs` updated to convert at its own boundary) Closes part of #1209. * feat(server): convert encoder/helper/echo internals from anyhow to ServerError (#1243) ## Summary Second step of the staged migration toward a typed public error story for `ironrdp-server` (#1209). Stacks on #1242. Replaces `anyhow` construction sites in modules whose internal flow does **not** pass through the `ConnectionHandler::on_disconnected` callback (which still takes `&anyhow::Error` and is the subject of #1244): - **`encoder/mod.rs`**: ~15 `anyhow!` / `.context()` / `bail!` sites converted to typed `ServerError` variants (`Encode`, `Reason`, `Custom`). `EncodeError` sources go through `ServerError::encode` (matching `ConnectorErrorExt::encode`). `spawn_blocking` `JoinError`, qoi codec errors, and zstd error codes go through `ServerError::custom` or `ServerError::reason` as appropriate. - **`helper.rs`**: TLS cert/key loading paths construct `ServerError::io` for `std::io::Error` sources, `ServerError::reason` for missing-key cases, `ServerError::custom` for `x509-cert` and PEM parsing errors. Removes the `from_anyhow` bridge and the inner-fn split introduced in #1242. - **`echo.rs`**: `build_echo_request` returns `ServerResult`, builds errors via `ServerError::custom` directly. `send_request` keeps its already-typed `Channel` and `Reason` variants. ## What this PR does NOT touch - `server.rs` internals (the heavy `.context()` chain in `run_inner` and `run_connection_inner`) stay on `anyhow` because they propagate into `ConnectionHandler::on_disconnected(error: Option<&anyhow::Error>)`. #1244 will (a) change that parameter to `Option<&ServerError>`, and (b) convert the server.rs internal sites in the same change. - `RdpServerDisplay` / `RdpServerDisplayUpdates` traits (which return `anyhow::Result`) stay unchanged. #1245 will convert those, drop the `anyhow` dependency entirely, and complete the migration. ## Why split this way `server.rs` and the display traits are both `anyhow`-flavored at the boundary; converting them in a single PR with `on_disconnected` keeps the type story coherent. Splitting them now would either require a temporary `anyhow ↔ ServerError` two-way bridge (ugly) or leave the public trait inconsistent with the rest of the crate (worse). Better to ship this batch (encoder + helper + echo all internal-only) and then take server.rs + on_disconnected as one coherent step. ## Stack ordering This is step 2 of 4. The full chain (must merge in PR-number order): | Step | PR | Scope | |------|-----|-------| | 1 | #1242 | Add `ServerError` / `ServerErrorKind` / ext traits, convert 5 public functions, internal anyhow stays via private bridge | | **2** | **this PR** | Convert encoder/helper/echo internals (anyhow → typed) | | 3 | #1244 | Convert server.rs internals + `on_disconnected` parameter (breaking) | | 4 | #1245 | Convert display traits, drop anyhow dep, finish (breaking) | Branch is stacked on `feat/server-typed-error` (#1242). Rebase onto master after #1242 merges. ## 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 - `cargo build --workspace --all-targets` clean Tracking: #1209. * feat(viewer): add /microphone command-line switch (#1790) Expose a `--microphone` CLI flag in ironrdp-viewer to enable the AUDIO_INPUT (MS-RDPEAI) dynamic virtual channel, redirecting the local microphone into the RDP session. It wires into the existing `ConfigBuilder::with_audio_capture(true)` and is off by default, matching `ChannelConfig::default().audio_capture == false`. The switch name is in pair with FreeRDP's own `/microphone` option, for consistency. * feat(server)!: convert server.rs internals + on_disconnected to ServerError (#1244) ## Summary Third step of the staged migration started in #1242 and continued in #1243. Combines the `on_disconnected` signature change with the `server.rs` internal site conversion since both touch anyhow-flowing code; doing them together avoids a temporary `anyhow ↔ ServerError` two-way bridge during the intermediate state. ## Public API change ```diff fn on_disconnected( &mut self, peer: SocketAddr, duration: Duration, - error: Option<&anyhow::Error>, + error: Option<&ServerError>, ) -> PostConnectionAction { ... } ``` This is a **breaking change** for handler implementations of `ConnectionHandler`. Pre-1.0, and per @elmarco's note on #1209: "I am ok with breaking API at this point :)". ## Internal changes - The two `run_inner` / `run_connection_inner` wrapper functions introduced in #1242 are folded back into `run` / `run_connection`. The accept loop calls the public method directly; `result.as_ref().err()` now feeds the new `ServerError`-typed parameter naturally. - ~25 `.context()` / `bail!` sites in `run`, `run_connection`, `accept_finalize`, `handle_io_channel_data`, `handle_x224`, `handle_input_backlog`, and the `encode_share_data_pdu` / `deactivate_all` helpers replaced with typed `ServerError` variants. Pattern alignment with `ConnectorErrorExt`: - `EncodeError` sources → `ServerError::encode` - `DecodeError` sources → `ServerError::decode` - `std::io::Error` sources → `ServerError::io` - `Option<channel>` with `.ok_or_else` → `ServerError::channel` - `bail!("Fastpath output not supported!")` → `ServerError::unsupported` - everything else → `ServerError::custom` with a static context. ## What this PR does NOT touch - The `RdpServerDisplay::updates()` and `RdpServerDisplayUpdates::next_update()` trait methods still return `anyhow::Result`. Their conversion is #1245, which also drops the `anyhow` dependency entirely. - The `from_anyhow` private helper introduced in #1242 is retained only at one boundary: the `display.updates()` call site in `run_connection` wraps its anyhow result via `from_anyhow` until #1245 lands. ## Stack ordering This is step 3 of 4. The full chain (must merge in PR-number order): | Step | PR | Scope | |------|-----|-------| | 1 | #1242 | Add `ServerError` / `ServerErrorKind` / ext traits, convert 5 public functions, internal anyhow stays via private bridge | | 2 | #1243 | Convert encoder/helper/echo internals (anyhow → typed) | | **3** | **this PR** | Convert server.rs internals + `on_disconnected` parameter (breaking) | | 4 | #1245 | Convert display traits, drop anyhow dep, finish (breaking) | Branch is stacked on `feat/server-typed-error-internal` (#1243), which is stacked on `feat/server-typed-error` (#1242). ## Coordinate with #1239 The `ConnectionHandler` trait was extended in #1239 (SuppressOutput / RefreshRectangle / FrameAcknowledge handlers). If this PR lands first, #1239 needs a trivial rebase to the new trait shape (the new methods stay; only `on_disconnected`'s signature changes around them). If #1239 lands first, this PR rebases onto its trait shape. Either order works. ## 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 - `cargo build --workspace --all-targets` clean Tracking: #1209. * feat(server)!: convert display traits to ServerResult, drop anyhow dep (#1245) ## Summary Final step of the staged migration started in #1242 and continued in #1243 / #1244. Completes the typed error story for `ironrdp-server` by converting the last consumer-facing trait surface and dropping the `anyhow` dependency entirely. Closes #1209. ## Public API changes ```diff pub trait RdpServerDisplayUpdates { - async fn next_update(&mut self) -> anyhow::Result<Option<DisplayUpdate>>; + async fn next_update(&mut self) -> ServerResult<Option<DisplayUpdate>>; } pub trait RdpServerDisplay: Send { - async fn updates(&mut self) -> anyhow::Result<Box<dyn RdpServerDisplayUpdates>>; + async fn updates(&mut self) -> ServerResult<Box<dyn RdpServerDisplayUpdates>>; } ``` These are **breaking changes** for handler implementations of the two display traits. ## Internal changes - `from_anyhow` private bridge and `AnyhowError` wrapper struct removed from `error.rs`. - `anyhow` dependency removed from `ironrdp-server/Cargo.toml`. - `builder.rs` `NoopDisplayUpdates` / `NoopDisplay` impls and the docstring examples in `display.rs` and `README.md` updated to match the new trait shapes. - `crates/ironrdp/examples/server.rs` and `crates/ironrdp-testsuite-extra/tests/main.rs` updated to return `ServerResult` from their `RdpServerDisplay/Updates` impls. - `benches/src/perfenc.rs` updated to construct `ServerError` variants instead of `anyhow::Error` and converts at its own `anyhow::Result` main boundary via `.map_err(|e| anyhow::anyhow!(e))`. - `urbdrc.rs`, the `usb`-gated USB device facade, converted from `anyhow` to `ServerResult`/`ServerError` (69 sites). This file predates the typed error migration and was never touched by it, so removing the `anyhow` dependency broke it under the `usb` feature. Typed external errors from `ironrdp-usb`/`ironrdp-rdpeusb` are wrapped with `ServerError::custom`; hand-rolled invariant checks (former `bail!`/`ensure!`) use `ServerError::reason`. ## After this PR `ironrdp-server` has **no anyhow dependency** and the public surface is fully typed against `ServerError`, including the `usb`-gated facade. The full chain: | Step | PR | Scope | |------|-----|-------| | 1 | #1242 | Add `ServerError` / `ServerErrorKind` / ext traits, convert 5 public functions, internal anyhow stays via private bridge | | 2 | #1243 | Convert encoder/helper/echo internals | | 3 | #1244 | Convert server.rs internals + `on_disconnected` parameter | | **4** | **this PR** | Convert display traits and the usb facade, drop anyhow dep, finish | ## Stacking note Stacked on `feat/server-typed-error-server` (#1244). Rebase on landing in PR-number order. ## 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 (including doctests) - `cargo xtask check features --case workspace/powerset-runtime` clean (44/44, covers `ironrdp-server` across the full feature powerset including `usb`) - `cargo build --workspace --all-targets` clean Closes #1209. * feat(activex): expand OLE clipboard snapshots (#1794) Expose bounded read-only snapshots for text, locale, DIB/DIBV5, and Windows HTML formats. Validate FORMATETC and payloads, return independent HGLOBAL media, and keep file, write, and advisory paths disabled. * feat(activex): support public RemoteApp launch (#1782) Expose the inherited Remote Program interfaces through raw and late-bound COM calls with shared control lifetime and strict argument validation. Queue one preconnect launch or connected-session RAIL executes, and report server or local execution results through OnRemoteProgramResult. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mamoreau-devolutions <242016029+mamoreau-devolutions@users.noreply.github.com> * feat(mstsgu): support outbound gateway proxies (#1776) Route all MS-TSGU HTTPS legs through configured HTTP CONNECT or SOCKS5 proxies while preserving gateway TLS validation and authentication. Honor HTTPS_PROXY/https_proxy and NO_PROXY/no_proxy with bounded CONNECT response handling and credential redaction. * test(extra): cover signed RPCH send requests (#1801) Cover authenticated, padded TsProxySendToServer framing so the replay extractor preserves inner RDP payloads. * feat(activex): support advanced session settings (#1781) Wire administrative-session GCC data, opaque load-balance routing tokens, and RDPSND quality selection through the ActiveX settings objects and client configuration. Keep unrelated transport, cache, video, device, and security policy slots as explicit E_NOTIMPL failures. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mamoreau-devolutions <242016029+mamoreau-devolutions@users.noreply.github.com> * feat(mstsgu): authenticate gateway sessions (#1802) Negotiate MS-TSGU SSPI NTLM authentication and reauthenticate the original session on a short-lived transport without interrupting application data. Reject unsupported smart-card and pluggable exchanges explicitly. * feat(vmconnect): add current-user CredSSP (#1766) Authenticate Hyper-V hosts with the caller's Windows logon token through native SSPI. Keep reusable host passwords out of process configuration and require nonce-bound CredSSP public-key verification. * test(extra): replay signed RPCH send after pipe setup (#1806) Model a non-null channel context and preceding TsProxySetupReceivePipe call in the signed RPCH replay fixture. This lets decrypted captures reproduce the live failure boundary without enabling a transport. * feat(server): expose measured bandwidth to the embedder (#1734) ## Summary - After #1470/#1471, the server can tell the client its measured bandwidth over the wire (Network Characteristics Result), but nothing exposes that figure outside the crate: `snapshot()` returns `RttSnapshot` (min/max/avg/sample_count only) and `autodetect_rtt_handle()` carries RTT alone. An embedder's own health or flow-control layer that wants the same bandwidth figure the client already received has no way to read it. - Mirrors the existing `autodetect_rtt_handle` shape exactly: a new `autodetect_bandwidth` `Arc<AtomicU32>` field (`u32::MAX` sentinel until the first measurement, matching `autodetect_rtt`), an `autodetect_bandwidth_handle()` accessor, and a `with_autodetect_bandwidth_handle()` builder method for injecting a shared instance. Adds `AutoDetectManager::bandwidth_kbps()` as the underlying getter; the field already existed internally with no public accessor. - The store site needed a before/after comparison rather than a plain `else` branch on `handle_response`'s result: that function reports a matched RTT sample through its return value but only updates bandwidth internally, so a naive `else` would also fire, and mislabel the store as a fresh bandwidth measurement, on any unmatched or stray RTT response once a bandwidth figure had been recorded at least once. Comparing `bandwidth_kbps()` before and after the call only stores and logs when a measurement genuinely completed. - Adds three tests to the existing autodetect test file, mirroring the RTT-handle tests already there: `bandwidth_kbps()` reflecting a completed measurement, the handle's sentinel default, and the injected-handle round trip. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass, including the 3 new tests (22/22 passing in `server::autodetect`). ## Notes No public API break: `RdpServer::new` is crate-private, and the public surface (`RdpServerBuilder`) only gains an additive optional field and a new method. * feat(activex): redirect dynamic drives (#1780) Keep drive capability active for hotplug-only sessions and route ActiveX drive rescans and selection changes through the live RDPDR channel. Preserve stable drive IDs, defer removals until server acknowledgement, and keep generic devices, printers, and ports explicitly unsupported. Smartcard redirection remains independent. * feat(activex): expose camera configurations (#1795) Enumerate Windows camera device interfaces through the published MSTSC camera COM contracts while preserving stable configuration identity across rescans. Reject every redirection enablement request with E_NOTIMPL because the Windows MS-RDPECAM plugin is private to mstscax and IronRDP has no capture backend yet. * feat(vmconnect): implement framebuffer channel (#1767) Implement Hyper-V's private framebuffer-redirection DVC for same-host VMConnect sessions. Validate the channel handshake, open the host-created synchronization and mapping objects, and expose the shared 32-bpp DIB as client frames. * feat(activex): add location redirection (#1778) Implement MS-RDPEL v1 codecs and the location DVC state machine, then route the ActiveX methods through the bounded client input queue. Preserve mstsc-compatible validation and altitude caching while surfacing inactive sessions, channel readiness, queue pressure, and encoding failures. Coordinates are caller-supplied only and are never logged or persisted. * feat(client): wire local VMConnect features (#1768) Wire VMConnect modes through the client configuration and RDP connector. Support current-user host authentication without a password, preserve guest credential boundaries, and attach Hyper-V framebuffer redirection only for eligible local Windows sessions. * feat(agent): expose Hyper-V VMConnect (#1770) Expose Hyper-V VMConnect through the agent CLI and daemon build. Add VMConnect mode and current-user options, default an omitted host to localhost, and enable the client VMConnect feature in the daemon. * fix(server)!: drop the tokio and tokio_rustls re-exports (#1796) ## Summary `ironrdp-server` re-exported both `tokio` and `tokio_rustls` wholesale so consumers did not need their own `tokio` dependency. This pins every consumer's `tokio` version to whatever `ironrdp-server` picks, with no way to upgrade independently. Drops the re-export. ```diff -pub use {tokio, tokio_rustls}; ``` ## Public API changes - `ironrdp_server::tokio` and `ironrdp_server::tokio_rustls` no longer exist. Consumers using either path need their own `tokio` and `tokio-rustls` dependencies. - `tokio-rustls` stays marked `# public` in `Cargo.toml`: `TlsAcceptor` genuinely appears in public signatures (`with_tls`, `with_hybrid`, `TransportTls::Tls`). `tokio`'s `# public` marker is dropped since no public signature returns or takes a bare `tokio` type; it was only public via the re-export. ## Internal changes - The one workspace consumer, `crates/ironrdp/examples/server.rs`, now imports `tokio` directly instead of through the facade. - `tokio` added as a direct dev-dependency of the `ironrdp` facade crate, and to its existing `#[cfg(test)] use { ... as _ }` unused-dependency silencer, the same pattern already used there for every other example-only dependency. ## Test plan `cargo xtask check fmt/lints/tests/typos/locks` clean. `cargo xtask check features --case workspace/powerset-runtime` clean (44/44, covers `ironrdp-server`). * feat(rdpdr): add RdpdrServer orchestration on top of the bidirectional PDU layer (#1783) ## Depends on #1779 This branch is stacked on `feat/rdpdr-server-pdu-decode` (#1779) and only makes sense once that lands: `RdpdrServer` is built entirely on the decode/encode surface #1779 adds. #1779's own Notes section already flags this as the intended follow-up. ## Summary - `RdpdrServer` is a state machine (`Initializing`, `AwaitingAnnounce`, `CapabilityExchange`, `Active`) driven by `SvcProcessor::process`, following the handshake sequence MS-RDPEFS 3.1.5/3.2.5 describe. `CapabilityExchange` and `Active` both accept `CoreDevicelistAnnounce` since real clients don't always send it strictly during capability exchange. A client re-sending its Announce Reply while already `Active` re-initializes (issue #195's duplicate-init case) rather than erroring: devices and in-flight IRPs clear, the backend sees a fresh `on_device_announce` batch. - Outstanding drive I/O is tracked by `CompletionId` in an `IrpTracker`, mirroring `ironrdp-rdpeusb`'s `RequestIdAllocator`/`pending_io` precedent. Each `PendingIrp` variant carries exactly the context `RdpdrPdu::decode_io_completion` needs (`MajorFunction`, `MinorFunction`, and the file/filesystem information class where the response layout depends on it). An unrecognized `CompletionId` is logged and ignored, not treated as an error. - `RdpdrServerBackend` has one completion callback per `PendingIrp` variant (fourteen, matching `MajorFunction` exactly except the deliberately unsupported `SetVolumeInformation`) plus `on_device_announce`, `on_device_remove`, `on_client_name`. `NoopRdpdrServerBackend` accepts every device and no-ops the rest. - Fourteen `drive_*` methods build and send each request type. Thirteen share a `DriveRequestBody` trait and a `send_device_io_request` helper; `device_control` needs its own wrapper since `DeviceControlRequest`'s encode doesn't include the input buffer that follows it on the wire, matching its decode-side counterpart. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Full handshake, per-drive-method completion round trips (all fourteen), device accept/reject, duplicate-init, and orphaned-completion-id tests added in `ironrdp-testsuite-core/tests/rdpdr/mod.rs`. ## Notes `ironrdp-server` integration (a factory trait plus `attach_channels` wiring, matching `RdpeiServerFactory`'s recently landed shape) is a further follow-up once this lands. * fix(dvc): log channel name on DVC creation failure (#1765) ## Summary - When a client rejects a DVC create request, `DrdynvcServer::process()` only logged the raw PDU (channel_id and status code). The channel's name is already in scope one line earlier, from building the original create request, but the failure branch never surfaces it. - I found this while debugging a real server log: a channel_id 0 creation failure with no way to tell which channel it was without reading the source and tracing registration order by hand. - Added the channel name to the failure log line using structured tracing fields, and raised the level from implicit debug to warn, since a channel creation failure is a real, actionable event most deployments would want visible at default verbosity rather than something requiring debug or trace logging to notice. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. ## Notes Five-line diff, `crates/ironrdp-dvc/src/server.rs` only. No behavior change beyond the added log line. * feat(viewer): add current-user VMConnect (#1771) Expose current-user Hyper-V VMConnect authentication through the viewer CLI. Document the local Windows invocation while preserving existing credential requirements for ordinary RDP sessions. * feat(server): wire RdpdrServer into ironrdp-server (#1784) ## Depends on #1783 Stacked on `feat/rdpdr-server-core` (#1783), which itself depends on #1779. Neither PDU codec bidirectionality nor RdpdrServer's orchestration is reachable from ironrdp-server without this. ## Summary - Wires RdpdrServer into ironrdp-server, mirroring SoundServerFactory and RdpsndServer exactly. RDPDR is a static virtual channel with the same build-a-backend-then-attach shape as audio, not a dynamic channel like EGFX and not a combined backend-plus-factory like clipboard. - RdpdrServerFactory extends the ServerEventSender supertrait rather than inlining a set_sender method, matching the live convention SoundServerFactory and CliprdrServerFactory already use. build_backend is named to match SoundServerFactory::build_backend. - Server-initiated drive I/O needs the same async relay every other channel uses. Added RdpdrServerMessage, one variant per RdpdrServer drive_* method, and a ServerEvent::Rdpdr(RdpdrServerMessage) arm in dispatch_server_events that looks up the live RdpdrServer instance, calls the matching drive_* method, and writes the encoded result, the same shape as the existing Rdpsnd arm. - Adding a fourteen-variant enum to ServerEvent pushed AutoReconnectCookieHandle::set's Result past clippy's result_large_err threshold, a method this change otherwise has nothing to do with. Suppressed locally with an explained reason rather than boxing the Rdpdr payload, which would have changed ServerEvent's public shape for a size concern from one unrelated method. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Added an exhaustive-match test over every RdpdrServerMessage variant in ironrdp-testsuite-core/tests/server/rdpdr.rs (no wildcard arm), so a future variant added to one side of the dispatch without the other fails to compile rather than silently falling through. ## Notes This closes out the RDPDR server-side contribution's three-part sequence (PDU codec, RdpdrServer orchestration, ironrdp-server wiring). * feat(cliprdr): add a clipboard sync loop detector (#1739) ## Summary - An embedder bridging CLIPRDR to a local OS clipboard commonly hits a feedback loop: content copied on one side syncs to the other, the other side's own change notification fires for that same content, and the embedder syncs it back, forever. ironrdp-cliprdr has no visibility into the local OS clipboard on either side by design (that lives entirely in the embedder's CliprdrBackend implementation), so it can't detect this on its own, but nothing in the crate gives an embedder a building block to break the cycle either. Every consumer bridging to a real desktop clipboard ends up needing to write the same hash-and-time-window correlation logic themselves. - Adds LoopDetector: hashes recent format lists and content by source (Remote/Local) within a configurable time window, so an embedder can ask "would syncing this out right now just echo what the other side just sent" before acting, plus an optional per-source rate limit as a belt-and-suspenders guard against update storms. - Every method that needs the time takes an explicit now_ms rather than reading a clock itself, matching CliprdrBackend::now_ms()/elapsed_ms() already on this crate for the same wasm/test-determinism reasons. - Hashing uses DefaultHasher rather than adding a crypto dependency: nothing here defends against an adversary, it only needs to avoid mistaking two different clipboard payloads for the same one within one process's own recent history. - would_cause_loop takes an explicit source parameter, matching its siblings would_cause_content_loop/should_skip_sync. An earlier version of this algorithm I'd written elsewhere hardcoded that direction on would_cause_loop specifically, which was an inconsistency with its own siblings rather than an intentional asymmetry; fixed here. ## Validation `cargo xtask check fmt/lints/tests/typos/locks` all pass. Tests live in ironrdp-testsuite-core since this crate sets [lib] test = false. ## Notes No public API break: this is a new module with entirely new public types, no existing signature changes. * feat(session): render the EGFX graphics pipeline output (#1461) ## Summary - The `ironrdp-egfx` compositor exposes changed output regions via `drain_output()`, but nothing consumed them, so an EGFX session decoded frames and dropped them. - Drains the compositor from `ActiveStage::process`: composites each completed-frame `OutputUpdate` into the `DecodedImage` and emits `ActiveStageOutput::GraphicsUpdate`, so every session consumer renders EGFX with no new code. No-op when the graphics DVC is not registered. - Adds a by-type mutable DVC accessor `DrdynvcClient::get_dvc_mut`, mirrored on the session (the x224 processor and `ActiveStage`), matching the existing `get_dvc`. - All regions drained in one pass are composited first by `composite_graphics_updates`, then surfaced as a single `GraphicsUpdate` covering their union. A consumer may redraw whatever region an update names, and `ironrdp-client` rebuilds the whole framebuffer for each one, so emitting per region would copy the desktop once per rectangle. A single `RDPGFX_SOLIDFILL_PDU` or `RDPGFX_CACHE_TO_SURFACE_PDU` can name up to `u16::MAX` of them. ## Validation - `cargo xtask check fmt/lints/tests/typos/locks` all pass, including the dependency guard. - Four tests cover the co…

Marc-André Moreau (mamoreau-devolutions) marked this pull request as ready for review August 10, 2026 10:50

Copilot AI balanced review requested due to automatic review settings August 10, 2026 10:50

Marc-André Moreau (mamoreau-devolutions) temporarily deployed to llm-providers August 10, 2026 10:51 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Marc-André Moreau (mamoreau-devolutions) August 10, 2026 10:51

View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/ironrdp-client/src/rdp.rs

Comment thread crates/ironrdp-client/src/rdp.rs

Comment thread crates/ironrdp-client/src/rdp.rs

Marc-André Moreau (mamoreau-devolutions) temporarily deployed to llm-providers August 10, 2026 12:28 — with GitHub Actions Inactive

Marc-André Moreau (mamoreau-devolutions) merged commit 1fbc9ba into master Aug 10, 2026
39 checks passed

Marc-André Moreau (mamoreau-devolutions) deleted the copilot/rdpdr-client-integration branch August 10, 2026 12:51

This was referenced Aug 9, 2026

chore(release): prepare for publishing #1599

Closed

chore(release): prepare for publishing #1619

Closed

chore(release): prepare for publishing #1653

Closed

Mariusz Białończyk (manio) mentioned this pull request Aug 24, 2026

fix(client): restore Noop RDPDR channel fallback, fixing audio regres… #1791

Merged

Navigation Menu

Repository navigation

feat: wire RDPDR backends into client connections - #1600

feat: wire RDPDR backends into client connections#1600
Marc-André Moreau (mamoreau-devolutions) merged 2 commits into
masterfrom
copilot/rdpdr-client-integration

Marc-André Moreau (mamoreau-devolutions) commented Aug 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

Marc-André Moreau (mamoreau-devolutions) commented Aug 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants