Skip to content

feat(cliprdr): add a clipboard sync loop detector - #1739

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/cliprdr-loop-detector
Open

feat(cliprdr): add a clipboard sync loop detector#1739
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/cliprdr-loop-detector

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

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.

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.

Add 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. It takes an explicit
now_ms on every call that needs the time 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.

Tests live in ironrdp-testsuite-core since this crate sets
[lib] test = false.
@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/core Touches the core architectural tier size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure labels Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

1 participant