From 5a92577a941493c86987f21e006e1f1a7f78605b Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Wed, 19 Oct 2022 02:43:18 +0900 Subject: [PATCH] Remove fields from remote candidates that could cause crashes in GetStats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typically, remote candidates come from signalling and are deserialized into C++ objects. The network_type field of these candidates is always ADAPTER_TYPE_UNKNOWN. However, in tests it is common to pass SDP and remote candidates as C++ objects. In this case, the network_type property of remote candidates is preserved, so DCHECK might be triggered when GetStats is called. Clearing fields that are not suitable as remote candidates fixes this issue. Bug: None Change-Id: Ida01b0224bce5cf3e87bcad1ddaca35c9f4fffe7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279680 Reviewed-by: Henrik Boström Commit-Queue: Henrik Boström Auto-Submit: Daniel.L (Byoungchan) Lee Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#38436} --- api/candidate.cc | 2 ++ pc/peer_connection.cc | 13 ++++++++++++- pc/rtc_stats_collector.cc | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/api/candidate.cc b/api/candidate.cc index 4d17256c2e..a14dda350c 100644 --- a/api/candidate.cc +++ b/api/candidate.cc @@ -22,6 +22,7 @@ Candidate::Candidate() component_(0), priority_(0), network_type_(rtc::ADAPTER_TYPE_UNKNOWN), + underlying_type_for_vpn_(rtc::ADAPTER_TYPE_UNKNOWN), generation_(0), network_id_(0), network_cost_(0) {} @@ -46,6 +47,7 @@ Candidate::Candidate(int component, password_(password), type_(type), network_type_(rtc::ADAPTER_TYPE_UNKNOWN), + underlying_type_for_vpn_(rtc::ADAPTER_TYPE_UNKNOWN), generation_(generation), foundation_(foundation), network_id_(network_id), diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index ed6cd99913..82e5942e6a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -2605,8 +2605,19 @@ void PeerConnection::AddRemoteCandidate(const std::string& mid, const cricket::Candidate& candidate) { RTC_DCHECK_RUN_ON(signaling_thread()); + if (candidate.network_type() != rtc::ADAPTER_TYPE_UNKNOWN) { + RTC_DLOG(LS_WARNING) << "Using candidate with adapter type set - this " + "should only happen in test"; + } + + // Clear fields that do not make sense as remote candidates. + cricket::Candidate new_candidate(candidate); + new_candidate.set_network_type(rtc::ADAPTER_TYPE_UNKNOWN); + new_candidate.set_relay_protocol(""); + new_candidate.set_underlying_type_for_vpn(rtc::ADAPTER_TYPE_UNKNOWN); + network_thread()->PostTask(SafeTask( - network_thread_safety_, [this, mid = mid, candidate = candidate] { + network_thread_safety_, [this, mid = mid, candidate = new_candidate] { RTC_DCHECK_RUN_ON(network_thread()); std::vector candidates = {candidate}; RTCError error = diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index cc5c7208e3..014ffa43dd 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -934,6 +934,9 @@ const std::string& ProduceIceCandidateStats(int64_t timestamp_us, } else { // We don't expect to know the adapter type of remote candidates. RTC_DCHECK_EQ(rtc::ADAPTER_TYPE_UNKNOWN, candidate.network_type()); + RTC_DCHECK_EQ(0, candidate.relay_protocol().compare("")); + RTC_DCHECK_EQ(rtc::ADAPTER_TYPE_UNKNOWN, + candidate.underlying_type_for_vpn()); } candidate_stats->ip = candidate.address().ipaddr().ToString(); candidate_stats->address = candidate.address().ipaddr().ToString();