mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-09-11 23:11:01 +03:00
fix three ways ws + WebRTC could not work in practice
Review of #15684 and hbb_common#579. Each of these left the code reading correct while the feature did not function. - The RelayResponse race classified P2P with `result.2 == "IPv6"`, but that site's futures are only ever the relay ("Relay"/"WebSocket") and the WebRTC branch's own "WebRTC" — so the predicate was constantly false. When the relay landed first the result was still right (the webrtc arm's `others_fut.is_none()` fallback), but when WebRTC connected FIRST it was parked as if it were a relay and the relay was committed on arrival, discarding a live direct connection. That is the LAN case: the better the network, the worse the outcome. Classify by what the label means, via is_direct_transport, and test both orderings — only the relay-first one was covered. - handle_peer_info wrote "force-always-relay=Y" into the peer's saved config whenever force_relay was set, which now includes the WebSocket transport. One ws session therefore turned the peer into a permanent relay-by-policy peer, and relay-by-policy means Relay-only ICE, so WebRTC could never go direct to it again — the flagship path worked exactly once. Persist policy_relay, which is the user's choice; the transport is a property of this client, not of the peer. - The answerer gated on this machine's enable-webrtc option, but that is LocalConfig: the UI process writes it and never syncs it over IPC, while handle_punch_hole runs in the server process, which on Windows resolves LocalConfig under a different profile and reads the private-server default of "N". The gate refused to answer in exactly the self-hosted deployments the transport exists for. Drop it: the answerer follows the request, like the udp/ipv6 legs, and the option still gates the feature where it can — an offer only exists because some controller had it enabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
This commit is contained in:
@@ -225,6 +225,15 @@ impl Drop for OffererGuard {
|
|||||||
/// into one error.
|
/// into one error.
|
||||||
///
|
///
|
||||||
/// `others` must be non-empty (`select_ok` requires it).
|
/// `others` must be non-empty (`select_ok` requires it).
|
||||||
|
/// Whether a transport label names a peer-to-peer path rather than the RustDesk relay.
|
||||||
|
///
|
||||||
|
/// The preference window exists to let one of these beat a relay that connects sooner, so a
|
||||||
|
/// label misclassified here inverts the race: a direct connection is parked as if it were a
|
||||||
|
/// relay, and the relay is then committed the moment it arrives.
|
||||||
|
fn is_direct_transport(typ: &str) -> bool {
|
||||||
|
!matches!(typ, "Relay" | "WebSocket")
|
||||||
|
}
|
||||||
|
|
||||||
async fn race_transports_prefer_webrtc<'a, T: 'a>(
|
async fn race_transports_prefer_webrtc<'a, T: 'a>(
|
||||||
webrtc_fut: BoxFuture<'a, ResultType<T>>,
|
webrtc_fut: BoxFuture<'a, ResultType<T>>,
|
||||||
others: Vec<BoxFuture<'a, ResultType<T>>>,
|
others: Vec<BoxFuture<'a, ResultType<T>>>,
|
||||||
@@ -1098,7 +1107,7 @@ impl Client {
|
|||||||
webrtc_fut,
|
webrtc_fut,
|
||||||
connect_futures,
|
connect_futures,
|
||||||
Self::WEBRTC_PREFER_WINDOW_MS,
|
Self::WEBRTC_PREFER_WINDOW_MS,
|
||||||
|result| result.2 == "IPv6",
|
|result| is_direct_transport(result.2),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -3453,7 +3462,12 @@ impl LoginConfigHandler {
|
|||||||
.insert("other-server-key".to_owned(), c.clone());
|
.insert("other-server-key".to_owned(), c.clone());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if self.force_relay {
|
// policy_relay, not force_relay: this writes the user's relay CHOICE back into the peer's
|
||||||
|
// saved config, and force_relay also carries the WebSocket transport, which is a property
|
||||||
|
// of this client's current setup rather than of the peer. Persisting that turned one ws
|
||||||
|
// session into a permanent relay-by-policy peer — and since relay-by-policy means
|
||||||
|
// Relay-only ICE, WebRTC could never go direct to it again.
|
||||||
|
if self.policy_relay {
|
||||||
config
|
config
|
||||||
.options
|
.options
|
||||||
.insert("force-always-relay".to_owned(), "Y".to_owned());
|
.insert("force-always-relay".to_owned(), "Y".to_owned());
|
||||||
@@ -5282,7 +5296,7 @@ async fn udp_nat_connect(
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod webrtc_race_tests {
|
mod webrtc_race_tests {
|
||||||
use super::{race_transports_prefer_webrtc, request_allows_tcp_punch};
|
use super::{is_direct_transport, race_transports_prefer_webrtc, request_allows_tcp_punch};
|
||||||
use hbb_common::{
|
use hbb_common::{
|
||||||
anyhow::anyhow,
|
anyhow::anyhow,
|
||||||
futures::future::{BoxFuture, FutureExt},
|
futures::future::{BoxFuture, FutureExt},
|
||||||
@@ -5315,6 +5329,37 @@ mod webrtc_race_tests {
|
|||||||
assert!(!request_allows_tcp_punch("webrtc://offer"));
|
assert!(!request_allows_tcp_punch("webrtc://offer"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The transport labels the RelayResponse race actually runs on. Its predicate has to
|
||||||
|
// recognise the WebRTC branch's own label as direct, or a WebRTC connection that completes
|
||||||
|
// BEFORE the relay is parked as if it were a relay and the relay is committed on arrival —
|
||||||
|
// inverting the race exactly on the fast networks where ICE beats a TCP relay connect.
|
||||||
|
#[test]
|
||||||
|
fn transport_labels_are_classified_as_direct_or_relayed() {
|
||||||
|
for direct in ["WebRTC", "TCP", "UDP", "IPv6"] {
|
||||||
|
assert!(is_direct_transport(direct), "{direct} is a direct path");
|
||||||
|
}
|
||||||
|
for relayed in ["Relay", "WebSocket"] {
|
||||||
|
assert!(
|
||||||
|
!is_direct_transport(relayed),
|
||||||
|
"{relayed} goes via the relay"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn direct_result_wins_even_when_it_arrives_first() {
|
||||||
|
// Same ordering as a LAN: the preferred branch connects before the relay does.
|
||||||
|
let got = race_transports_prefer_webrtc(
|
||||||
|
ok_after(10, "WebRTC"),
|
||||||
|
vec![ok_after(120, "Relay")],
|
||||||
|
60_000,
|
||||||
|
|result| is_direct_transport(result),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(got, "WebRTC");
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn webrtc_preferred_over_faster_relay_within_window() {
|
async fn webrtc_preferred_over_faster_relay_within_window() {
|
||||||
let got = race_transports_prefer_webrtc(
|
let got = race_transports_prefer_webrtc(
|
||||||
|
|||||||
@@ -854,13 +854,16 @@ impl RendezvousMediator {
|
|||||||
// transport-forced (ws) and its offer carries every candidate type, so answer with
|
// transport-forced (ws) and its offer carries every candidate type, so answer with
|
||||||
// full ICE and let a direct pair form; without it the offer is Relay-only ICE by
|
// full ICE and let a direct pair form; without it the offer is Relay-only ICE by
|
||||||
// policy, viable (and answerable) only through TURN.
|
// policy, viable (and answerable) only through TURN.
|
||||||
let webrtc_relay_only = ph.force_relay
|
let webrtc_relay_only =
|
||||||
&& !WebRTCStream::endpoint_declares_all_ice(&ph.webrtc_sdp_offer);
|
ph.force_relay && !WebRTCStream::endpoint_declares_all_ice(&ph.webrtc_sdp_offer);
|
||||||
// Unlike the udp/ipv6 legs - which deliberately just follow the request - WebRTC is
|
// Like the udp/ipv6 legs, the answerer follows the request and does not consult this
|
||||||
// gated on this machine's own option too: answering builds a pc that gathers ICE
|
// machine's own enable-webrtc option. That option is LocalConfig, which the UI process
|
||||||
// from this host, so a machine with WebRTC off must not be pulled into it.
|
// writes and never syncs over IPC — this code runs in the server process, which on
|
||||||
|
// Windows resolves LocalConfig under a different profile entirely and would read the
|
||||||
|
// private-server default of "N", silently refusing to answer in exactly the self-hosted
|
||||||
|
// deployments the transport is for. The option still gates the feature where it can:
|
||||||
|
// an offer only exists because a controller had it enabled.
|
||||||
let webrtc_viable = !ph.webrtc_sdp_offer.is_empty()
|
let webrtc_viable = !ph.webrtc_sdp_offer.is_empty()
|
||||||
&& crate::get_webrtc_enabled()
|
|
||||||
&& !Config::is_proxy()
|
&& !Config::is_proxy()
|
||||||
&& (!webrtc_relay_only || WebRTCStream::has_turn_server());
|
&& (!webrtc_relay_only || WebRTCStream::has_turn_server());
|
||||||
let webrtc_sdp_answer = if webrtc_viable {
|
let webrtc_sdp_answer = if webrtc_viable {
|
||||||
|
|||||||
Reference in New Issue
Block a user