Same pattern as enable-udp-punch / enable-ipv6-punch: empty value reads
as on against the public server and off against a private one (the
injection lives in rustdesk's get_local_option), so self-hosted
deployments opt in once their server / TURN is ready.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Reverts the webrtc_all_ice proto field (64b54ab) in favor of an
`ice_policy: "all"` key inside the webrtc:// envelope JSON, next to the
RTCSessionDescription fields. Same information, better carrier:
- it is a property of the offer itself, so it rides with the offer;
- the rendezvous server never has to know: the envelope is an opaque,
length-bounded string to hbbs, so no forwarding code and no vendored
proto copies to keep in sync;
- serde ignores unknown JSON keys when parsing RTCSessionDescription,
so every skew combination degrades exactly like the proto field did:
absence - not an error - is the old Relay-only reading.
endpoint_declares_all_ice() is the receiving side: parse failure,
foreign scheme or missing key all read as "not declared".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
use_ws() folds into force_relay because a ws tunnel kills classic TCP/UDP
punching — but ICE opens its own sockets and does not care how signaling
reaches the server. Without a signal, the controlled side must treat every
force_relay offer as Relay-only ICE (answer gated on TURN), which locks
WebSocket deployments out of direct WebRTC entirely.
webrtc_all_ice marks an offer that gathered every candidate type: the
controller's force_relay covers only classic punching, not ICE policy.
Absent/false keeps today's semantics on every skew combination (old
controller, old server dropping the field, old controlled side).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
`requester_id = 11` was added and removed in the same rebase batch, never
reached main, and never reached hbbs — whose vendored copy of this file still
stops at field 9. So nothing has ever written or read tag 11, and reserving it
guards a wire format that does not exist.
It was also inconsistent with what this branch already does: `IceCandidate`
retyped tag 2 from `string to_id` to `bytes socket_addr` in place, which is only
sound because none of this proto has shipped. Same premise, so tag 11 is free.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`next()` read every non-FRAG_END header as "more fragments", so a peer whose
framing had diverged was only caught by the MAX_FRAME_LENGTH cap — and a
FRAG_MORE carrying no payload was never caught at all: it adds nothing to the
accumulator, so the cap never trips and the loop spins for as long as the peer
keeps writing, with no error and no teardown. Decide the header's meaning in one
match, so a future header kind cannot be handled in one place and missed in the
other. Neither case is reachable from send_bytes_inner, which emits FRAG_MORE
only for a full MAX_FRAGMENT_PAYLOAD chunk.
Release the accumulator on the error paths rather than truncating it: at the cap
that is ~1 GiB still referenced through the SESSIONS clone.
Doc corrections, all of them overclaims in the previous pass:
- the cancel-safety entry held only for the successful read path.
read_data_channel does await after dequeuing on its ErrShortBuffer and DCEP
branches, and next() awaits pc.close() on its error paths — where
RTCPeerConnection::close latches is_closed before its first await, so a
cancelled close silently turns every later close into a no-op and leaves the
pc in SESSIONS.
- recv_state: cancellation drops the guard mid-message, so it is the
single-reader assumption, not the mutex, that ultimately keeps two readers
from splicing into one accumulator.
- is_relayed: stream.rs promised None before pair selection while webrtc.rs
documented Some(true) under Relay policy; align both.
- get_local_endpoint: examples/webrtc.rs calls it too, not only the tests.
- PunchHole.reserved 11: named the wrong writer — PunchHole is written by the
rendezvous server, not by peers. Reserve the name as well as the tag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rotating on age alone let a single day's file grow without limit, so whoever
can drive a hot log site decided how much disk this uses and no amount of
per-site throttling could bound it. Add a size criterion, which covers every
call site at once — including ones no throttle was added to.
LogThrottle: recover the guard on a poisoned lock rather than returning None.
Poisoning only means another thread panicked while holding it; the guarded
data is two counters that are still usable, and going silent for the rest of
the process is worse than a stale count. AGENTS.md permits handling lock
poisoning directly, and it forbids swallowing the error.
Keep map_or over clippy's is_none_or: that was stabilized in Rust 1.82 and CI
pins 1.75.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
Debug output is written to the log file, so a log site that fires per received
message lets whoever is sending decide how much a machine writes to disk.
Dropping the line instead would hide real faults, so collapse it: one line per
interval carrying the count of everything suppressed since the last one, with
the first occurrence after a quiet period always reported so an isolated fault
is not delayed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
- Cargo.toml: record why webrtc is pinned to 0.13 — >=0.14 pulls sdp 0.10 /
webrtc-util 0.12 using usize::is_multiple_of (needs rustc >=1.87), while
rustdesk CI builds with Rust 1.75 (sciter i128 ABI pin)
- module-level upgrade checklist in src/webrtc.rs listing the version-coupled
webrtc-rs internals this transport relies on (SCTP write backpressure,
64KB message cap, detach() semantics, handler-capture leak cycle,
Disconnected transience, stats-based is_relayed), all verified against
webrtc 0.13 / webrtc-data 0.11 / webrtc-sctp 0.12
- send_bytes: document the bounded-backpressure mechanism (128 KiB PendingQueue
semaphore + cwnd/rwnd cap) and that it is NOT cancel-safe
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- 1-byte-header fragmentation past the 64KB SCTP cap; empty-message and clean-EOF handling
- is_relayed() via selected candidate-pair stats for the direct/relayed flag
- IdPk.dtls_fingerprint + rendezvous webrtc SDP/IceCandidate proto fields
- fix pc leaks: Weak capture breaks the state-handler Arc self-cycle; close pc on new() error paths
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Written by the texture-render watchdog / startup probe in the main repo;
a failed record forces texture rendering off until it is cleared (probe
pass or an explicit toggle of use-texture-render).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two P3 hardening items from the review: a descendant that changes its
own process group escapes the deadline's group kill, and could then leak
or block the parent.
- The deadline path now also sends a pid-targeted SIGKILL to the direct
child, so child.wait() is bounded even if the child left the group and
the group kill missed it.
- The normal-exit path drains stdout and stderr non-blocking instead of
read_to_string: the child has exited so its output is already
buffered, but an escaped grandchild holding a write end would keep the
pipe from EOF and hang a blocking read. The drain is capped so a
descendant that keeps writing cannot spin it. first_buffered_line now
shares that drain.
Verified: a probe child whose grandchild setpgid-escapes and holds the
pipe returns in 25 ms instead of hanging, and a direct child that
escapes and blocks is bounded to the deadline instead of its full
sleep.
Addresses the maintainer's two requests on #580: linux.rs was crowded,
and the new fallback should not touch the base Wayland path.
All the socket-probe machinery moves to src/platform/linux/wayland_probe.rs
- the child entry point, the runtime-dir scan, the privilege drop, the
process-group probe and its buffered-line inspection - leaving
WaylandDisplayInfo and get_wayland_displays in linux.rs. The module and
the fallback call in get_wayland_displays are gated on a new off-by-default
feature 'wayland_probe'; without it get_wayland_displays returns the
connect error exactly as it did before this fallback existed, so a
consumer that does not build the DRM login-screen backend compiles none
of this. The DRM build turns the feature on through scrap/drm.
Addresses fufesou's #580 re-review:
- The child runs in its own process group and the deadline kills the
whole group, so a hung loginctl or a leaked grandchild cannot survive
the child or keep the pipes open past the reads. Verified: a
deliberately leaked sleep grandchild no longer outlives the probe.
- The probe parses compositor-controlled data, so before touching the
socket the child drops to the runtime directory's owner and refuses
to probe at all if the drop fails, rather than parsing untrusted
bytes as root. loginctl still runs as root first, since finding the
seat needs it.
- first_buffered_line now distinguishes an inspected-but-empty pipe
(WouldBlock) from an uninspectable one (fcntl or read error): only a
real absence of the handshake latches the binary unsupported, an
inspection failure does not.
- rustfmt-clean.
A binary that does not dispatch the probe argument runs its normal
startup; a long-running one outlives the deadline and was killed before
the handshake check could run, so PROBE_UNSUPPORTED never latched and
every enumeration cycle spawned a full consumer process again. The
timeout path now judges the child by what it already wrote: a real
probe prints the magic line first and flushes, so its absence after a
whole deadline means this is not a probe. Only the buffered bytes are
read, non-blocking, because an EOF-seeking read could hang on a
grandchild that inherited the write end of the pipe.
Also report the exit status when a failed child left stderr empty,
which panic=abort and signals do, and name the malformed-list case in
the deserialization error.
Two review findings:
- The release profile builds with panic=abort, so the in-thread isolation
was an illusion: an sctk panic on malformed bytes from a scanned socket
aborted the whole server before ProbeBusyGuard could run. The probe now
runs in a child process spawned from the current executable, which the
consumer binary dispatches to wayland_display_probe_child_main before
any other startup work. A panic there kills only the child, and the
deadline now kills the child instead of leaking a blocked thread.
- seat0_runtime_dir ran loginctl through Command::output with no bound,
before the worker and its timeout existed, while the caller held the
DISPLAYS lock. The lookup now runs inside the child, under the same
two second deadline as everything else.
A binary that does not dispatch the probe arg fails a magic line
handshake and the probe latches off for the process lifetime, so the
fallback degrades to the pre-fallback behavior instead of spawning a
full consumer process per enumeration cycle. The consumer wiring is one
early dispatch in core_main:
#[cfg(target_os = "linux")]
if std::env::args().nth(1).as_deref()
== Some(hbb_common::platform::linux::WAYLAND_DISPLAY_PROBE_ARG)
{
hbb_common::platform::linux::wayland_display_probe_child_main();
}
The fallback was silent on success, so a host where it engaged looked identical to one
where it was never reached. Debug level: it runs on a cache miss, not per frame.
Answers the review on this PR. The fallback kept three states that could not
self-heal, and two of them came from the same place: it trusted the environment and
it had no deadline.
B1, no deadline on any part of the new path while the caller holds a process-wide
lock: `connect(2)` parks on a full backlog and sctk's roundtrip polls with no
deadline, so a socket that accepts and never speaks wayland stalls the display
service, the six drm_capturer call sites and the flutter_ffi SyncReturn. The probe
now runs on its own thread with a 2 s deadline, under the 3 s the uinput caller
already budgets, and only one probe can be in flight. That also keeps sctk's
`panic!`/`todo!` on malformed output events off the thread holding the lock, so it
can no longer poison that mutex, and a `Builder` is used because `thread::spawn`
panics when a thread cannot be created.
B2, a newly reachable empty `Ok` latched into the caller's cache for the process
lifetime: an empty list is now an error, so the existing no-cache path still
applies and the next poll retries.
B3, the `WAYLAND_SOCKET` half of the guard could not fire, because
`connect_to_env` removes that variable on its success and bad-fd paths and this is
re-entered every ~1.5 s. Both variables are read at the call site before
connecting, and the answer is latched: a consumed variable cannot turn a process
that was pointed at a compositor into one free to look for another.
C4 and S1 have the same fix: the directory is `/run/user/<uid>` of the active seat0
session rather than `XDG_RUNTIME_DIR`, so no environment value reaches a connect in
a process that can be root, and it works in the default build, which is given no
such variable (C3). The candidates are scanned rather than guessed, since
`wl_display_add_socket_auto` takes the first FREE name up to `wayland-32` and a
greeter accumulates leftovers; that drops the `path.exists()` pre-stat and its
TOCTOU window with it (C7), and empty values are no longer read as names (C6).
C2, the first socket that accepted used to win unconditionally: the loop now
carries on to the next candidate when a socket connects but fails the protocol or
reports no outputs.
C8, `registry_handlers!()` was empty, so a `wl_output` advertised between the
registry snapshot and the roundtrip was dropped and never reached `outputs()`.
C5 is real and is not fixable here: `get_primary_monitor` spawning a timeout-less
xrandr inside the held lock is in the consumer, and I will send it there.
Review of the first version, all three correct.
The important one: `connect_to_env()` honours `WAYLAND_DISPLAY` and `WAYLAND_SOCKET`,
and the fallback ignored that choice. If an explicit endpoint was named and failed,
this could attach to a *different* compositor and read output positions from the wrong
display. It now runs only when neither variable is set, which is also the only case it
was ever justified by.
`XDG_RUNTIME_DIR` is read with `var_os` and rejected when relative, since `.` or
`../tmp` would probe against the working directory. And every failed probe is kept
instead of the last one overwriting the first, so an error naming both sockets says
what happened to both.
The comment is three lines now: the reasoning belongs here, in the commit.
`get_wayland_displays` gives up as soon as `Connection::connect_to_env()` fails,
which is any process that was not handed `WAYLAND_DISPLAY`. A login screen is
the case that matters: rustdesk starts the greeter's `--server` without the
compositor variables on purpose, and the desktop layout is then unavailable for
the whole session even though the compositor is running and its socket sits in
the runtime directory of the very user that process runs as.
Measured at an sddm Plasma Wayland greeter: `/run/user/112/wayland-0` exists,
`connect(2)` to it as that user succeeds, and the registry advertises
`wl_output` and `wl_seat`. With this fallback the greeter reads the same output
layout any other session does.
Strictly a fallback after `connect_to_env` has already failed, so it cannot
change a host where that succeeds; where there is no compositor at all the
socket is simply absent and it fails exactly as before.
- add hide-recording-button to local settings
- add windows-service-video-save-directory to service settings
Signed-off-by: 21pages <sunboeasy@gmail.com>
Add ControlledContext to rendezvous messages so the server can pass a controller-user audit ref to the controlled client.
The controlled client returns the ref when posting audit logs, allowing the server to associate those logs with the controller user.
Signed-off-by: 21pages <sunboeasy@gmail.com>
rustdesk
RustDesk
fufesou
Mariano Abad
FrederickStempfle
21pages