Skip to content

feat(agent): add clipboard image support - #1877

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/agent-clipboard-image
Open

feat(agent): add clipboard image support#1877
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/agent-clipboard-image

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

Extends the daemon clipboard-get/clipboard-set operations added in #1863 to
images: clipboard-get-image writes the last image received from the remote
clipboard to a PNG file, clipboard-set-image reads a PNG file and advertises
it to the remote as CF_DIB/CF_DIBV5.

The daemon stores images as PNG bytes and converts to/from the wire's DIB
byte layout via ironrdp-cliprdr-format's existing png_to_cf_dib(v5) and
dib(v5)_to_png, so no new pixel-format code is needed here. Both DIB variants
are advertised when offering an image (DIBV5 is richer but not every peer
understands it); on paste, DIBV5 is preferred over DIB, and image is
preferred over text when the remote offers both.

Replaces ClipboardState's two Option fields with a small
ClipboardContent enum (Text/Image) so local and remote clipboard content is
a single logical item, matching how each clipboard-set* call replaces
whatever was there before. Adds a PendingPaste field to the CLIPRDR backend
to track which format was requested, since FormatDataResponse does not
itself carry the format it answers and MS-RDPECLIP allows only one
outstanding request at a time.

New Request::ClipboardGetImage/ClipboardSetImage and
Payload::ClipboardImage wire variants, bounded at 64 MiB (matching
ironrdp-cliprdr-format's own internal cap) both in the CLI parser and on
wire decode. clipboard_set_image validates the PNG up front by attempting
the DIB conversion, so a bad set fails immediately rather than only once the
remote asks for it.

Adds the two new variants to ironrdp-activex's exhaustive match on Request,
same treatment as the existing ClipboardGet/ClipboardSet arm (unavailable
through the ActiveX automation surface). Adds wire round-trip and
Debug-redaction test coverage for all four clipboard Request/Payload
variants, including the two existing text ones that had none before.

Behavior change: clipboard-get now prints (empty) if the last remote copy
was an image rather than text, since local/remote content is a single
logical item. Previously it always reflected the last text.

Known limitation: FormatDataResponse does not carry a request-correlation
ID, so a stale response to a superseded paste request (a second remote
copy landing before the first paste response arrives) is fed through
whatever PendingPaste variant is current at the time. The failure mode is
graceful: the mismatched conversion (dib_to_png/dibv5_to_png on the wrong
byte layout) returns an error and the response is dropped, not
misinterpreted as valid data.

cargo xtask check fmt/lints/tests/typos/locks all pass, including new wire
round-trip and Debug-redaction coverage for both DIB and DIBV5 paths. Not
yet verified against a live remote client; that check is still owed before
merge.

@github-actions github-actions Bot added maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/ffi Affects native or .NET bindings size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure labels Sep 2, 2026
@glamberson

Copy link
Copy Markdown
Contributor Author

Two PRs are stacked on this: #1878 (HTML support) is filed against master with a cumulative diff, since GitHub-native stacked PRs are not usable across a fork boundary. #1878 depends on the ClipboardContent enum introduced here.

@CBenoit Benoît Cortier (CBenoit) added the ai-review/allow-oversized Allows normal automated review of an oversized pull request label Sep 2, 2026
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior risk/medium Behavioral change that does not substantially alter a core public API scope/cross-cutting Spans multiple architectural boundaries and removed risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny labels Sep 2, 2026
@CBenoit Benoît Cortier (CBenoit) added ai-review/allow-oversized Allows normal automated review of an oversized pull request and removed ai-review/allow-oversized Allows normal automated review of an oversized pull request labels Sep 2, 2026

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I’m trying out the new helmcode-based reviewer on this PR, but the code is looking good to me.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

RPC size mismatch, stale-response correlation, and concurrent advertisement races can break clipboard operations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds PNG clipboard image support to the agent daemon and CLI using CLIPRDR DIB conversions.

Changes:

  • Adds image clipboard RPC variants and CLI commands.
  • Supports DIB/DIBV5 advertisement and conversion.
  • Adds wire round-trip and redaction tests.
File summaries
File Description
crates/ironrdp-testsuite-extra/tests/agent.rs Extends RPC codec tests.
crates/ironrdp-rpc/src/wire/mod.rs Adds optional byte encoding.
crates/ironrdp-rpc/src/ipc.rs Defines image RPC messages and limits.
crates/ironrdp-daemon/src/daemon.rs Handles image clipboard requests.
crates/ironrdp-daemon/src/clipboard.rs Implements DIB image clipboard behavior.
crates/ironrdp-daemon/Cargo.toml Adds the format-conversion dependency.
crates/ironrdp-agent/src/help.rs Documents image commands.
crates/ironrdp-agent/src/cli.rs Implements image file operations.
crates/ironrdp-activex/src/rpc.rs Marks image operations unavailable.
Cargo.lock Records the dependency update.
Review details
  • Files reviewed: 9/10 changed files
  • Comments generated: 6
  • Review effort level: Balanced

Comment thread crates/ironrdp-agent/src/cli.rs Outdated
Comment thread crates/ironrdp-daemon/src/clipboard.rs Outdated
Comment thread crates/ironrdp-daemon/src/daemon.rs Outdated
Comment thread crates/ironrdp-rpc/src/ipc.rs Outdated
Comment thread crates/ironrdp-rpc/src/ipc.rs Outdated
Comment thread crates/ironrdp-testsuite-extra/tests/agent.rs
Comment thread crates/ironrdp-rpc/src/ipc.rs Outdated
Comment thread crates/ironrdp-daemon/src/daemon.rs
Comment thread crates/ironrdp-daemon/src/clipboard.rs Outdated
@CBenoit Benoît Cortier (CBenoit) added ai-review/allow-oversized Allows normal automated review of an oversized pull request and removed ai-review/allow-oversized Allows normal automated review of an oversized pull request labels Sep 3, 2026
@github-actions github-actions Bot added risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny and removed risk/medium Behavioral change that does not substantially alter a core public API labels Sep 3, 2026
Extends the daemon clipboard-get/clipboard-set operations added in Devolutions#1863 to
images: clipboard-get-image writes the last image received from the remote
clipboard to a PNG file, clipboard-set-image reads a PNG file and advertises
it to the remote as CF_DIB/CF_DIBV5.

The daemon stores images as PNG bytes and converts to/from the wire's DIB
byte layout via ironrdp-cliprdr-format's existing png_to_cf_dib(v5) and
dib(v5)_to_png. Both DIB variants are advertised when offering an image
(DIBV5 is richer but not every peer understands it); on paste, DIBV5 is
preferred over DIB, and image is preferred over text when the remote offers
both.

Replaces ClipboardState's two Option<String> fields with a small
ClipboardContent enum (Text/Image) so local and remote clipboard content is
a single logical item, matching how each clipboard-set* call replaces
whatever was there before.

New Request::ClipboardGetImage/ClipboardSetImage and Payload::ClipboardImage
wire variants. Adds the two new variants to ironrdp-activex's exhaustive
match on Request, same treatment as the existing ClipboardGet/ClipboardSet
arm (unavailable through the ActiveX automation surface).

Behavior change: clipboard-get now prints (empty) if the last remote copy
was an image rather than text, since local/remote content is a single
logical item. Previously it always reflected the last text.

Review round (CBenoit, Copilot): Seven issues addressed.

- MAX_CLIPBOARD_IMAGE_BYTES is now derived from ironrdp-rpc's own
  transport::MAX_MESSAGE_LEN (16 MiB) minus framing headroom, instead of an
  unrelated 64 MiB that the RPC transport could never actually carry.
- Fixed a real, pre-existing bug in ironrdp-cliprdr-format::bitmap::
  decode_png: It requested ALPHA | EXPAND but not STRIP_16, and this png
  crate version declares a GRAY_TO_RGB transform flag but never implements
  it, so grayscale and 16-bit-per-channel PNGs decoded to a buffer layout
  the RGBA-only conversion downstream silently misread. Added STRIP_16, a
  manual GrayscaleAlpha-to-RGBA expansion, a MAX_BUFFER_SIZE re-check for
  that expansion, and grayscale/16-bit regression tests.
- The CLIPRDR backend now serializes paste requests: on_remote_copy no
  longer issues a second SendInitiatePaste while one is outstanding (a
  stale response could otherwise be misinterpreted as the answer to a
  newer request, since FormatDataResponse carries no correlation ID). A
  newer copy is queued and issued once the outstanding one resolves, or
  after a 5-second timeout if the peer never answers.
- The daemon now checks a converted remote image against
  MAX_CLIPBOARD_IMAGE_BYTES before storing it, so a legitimate but
  oversized image from the remote can't get stuck in a state where
  clipboard-get-image can never send it back.
- clipboard_set/clipboard_set_image now write local and advertise to a
  connected session as one atomic step under the clipboard mutex, so
  concurrent IPC callers can't interleave their state write and their
  advertisement.
- clipboard-set-image reads at most MAX_CLIPBOARD_IMAGE_BYTES + 1 bytes via
  Read::take instead of loading the whole file before checking its size.
- Extended the Debug-redaction test to also cover the pre-existing text
  variants, which the original PR body claimed but the test didn't
  exercise.

cargo xtask check fmt/lints/tests/typos/locks all pass. Not yet verified
against a live remote client; that check is still owed before merge.
@github-actions github-actions Bot added the scope/core Touches the core architectural tier label Sep 3, 2026
Greg Lamberson (glamberson) added a commit to lamco-admin/IronRDP that referenced this pull request Sep 3, 2026
Depends on Devolutions#1877 (clipboard image support), which this branch is stacked on:
it introduces the ClipboardContent enum this PR extends with an Html
variant. Diff is cumulative against master per the usual stacking note; the
image-only diff is Devolutions#1877's own.

Extends the daemon clipboard-get/clipboard-set operations to HTML fragments:
clipboard-get-html prints the last HTML fragment received from the remote
clipboard, clipboard-set-html sets one and advertises it to the remote as
the registered HTML Format.

Stores the fragment as plain text (not yet CF_HTML-wrapped) and converts at
the point of use via ironrdp-cliprdr-format's existing
plain_html_to_cf_html/cf_html_to_plain_html, so no new markup handling is
needed here, mirroring the image PR's use of the crate's existing bitmap
conversions.

HTML Format is a registered, not fixed-ID, clipboard format: the offering
side picks a private-range ID (0xC000 here) and pairs it with the name; the
receiver learns the ID-to-name mapping from the FormatList. When receiving,
matches the remote's offer by name and uses whatever ID it assigned, not the
locally-chosen one, since those only apply to formats this backend itself
offers.

Extends the remote-copy priority order to image, then HTML, then text
(richest representation first), and the PendingPaste tracking added in
Devolutions#1877 with an Html variant so a returned FormatDataResponse is decoded
correctly regardless of which of the three was requested. A remote HTML
fragment over the same 256 KiB bound is dropped and logged rather than
stored, mirroring the oversized-image drop Devolutions#1877 added for the same reason:
an unbounded stored value would otherwise fail encoding once handed back to
clipboard-get-html over the RPC transport.

New Request::ClipboardGetHtml/ClipboardSetHtml and Payload::ClipboardHtml
wire variants, bounded at 256 KiB in the CLI parser and on wire decode.
Extends ironrdp-activex's exhaustive match on Request with the same
treatment as the other clipboard operations. Extends the wire round-trip
and Debug-redaction test coverage added in Devolutions#1877 to the two new variants.

cargo xtask check fmt/lints/tests/typos/locks all pass. Not yet verified
against a live remote client, same as Devolutions#1877.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review/allow-oversized Allows normal automated review of an oversized pull request kind/protocol Affects RDP or related protocol behavior maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier scope/cross-cutting Spans multiple architectural boundaries scope/ffi Affects native or .NET bindings size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure

Development

Successfully merging this pull request may close these issues.

3 participants