stats: do not expose ufrag before the first setRemoteDescription
spec issue: https://github.com/w3c/webrtc-stats/issues/789
fiddle: https://jsfiddle.net/fippo/hjrdg056/5
before SRD both ufrags should be undefined
after SRD the pc2 ufrag should be visible in the candidate stats
BUG=webrtc:375571816
Change-Id: I7133e8cd2d0d8a6e0b658173d564b4e5670c4ca5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366562
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#44064}
diff --git a/api/candidate.cc b/api/candidate.cc
index 7711329..2a2427e 100644
--- a/api/candidate.cc
+++ b/api/candidate.cc
@@ -196,7 +196,8 @@
}
Candidate Candidate::ToSanitizedCopy(bool use_hostname_address,
- bool filter_related_address) const {
+ bool filter_related_address,
+ bool filter_ufrag) const {
Candidate copy(*this);
if (use_hostname_address) {
rtc::IPAddress ip;
@@ -219,6 +220,10 @@
copy.set_related_address(
rtc::EmptySocketAddressWithFamily(copy.address().family()));
}
+ if (filter_ufrag) {
+ copy.set_username("");
+ }
+
return copy;
}
diff --git a/api/candidate.h b/api/candidate.h
index 180b1c9..ae54d74 100644
--- a/api/candidate.h
+++ b/api/candidate.h
@@ -229,8 +229,16 @@
// to the wildcard address (i.e. 0.0.0.0 for IPv4 and :: for IPv6). Note that
// setting both booleans to false returns an identical copy to the original
// candidate.
+ // The username fragment may be filtered, e.g. for prflx candidates before
+ // any remote ice parameters have been set.
+ [[deprecated("Use variant with filter_ufrag")]] Candidate ToSanitizedCopy(
+ bool use_hostname_address,
+ bool filter_related_address) const {
+ return ToSanitizedCopy(use_hostname_address, filter_related_address, false);
+ }
Candidate ToSanitizedCopy(bool use_hostname_address,
- bool filter_related_address) const;
+ bool filter_related_address,
+ bool filter_ufrag) const;
// Computes and populates the `foundation()` field.
// Foundation: An arbitrary string that is the same for two candidates
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index b9ca32c..f36ac33 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -22,7 +22,6 @@
#include <set>
#include <string>
#include <tuple>
-#include <type_traits>
#include <utility>
#include <vector>
@@ -504,7 +503,7 @@
RTC_LOG(LS_INFO) << "Received remote ICE parameters: ufrag="
<< ice_params.ufrag << ", renomination "
<< (ice_params.renomination ? "enabled" : "disabled");
- IceParameters* current_ice = remote_ice();
+ const IceParameters* current_ice = remote_ice();
if (!current_ice || *current_ice != ice_params) {
// Keep the ICE credentials so that newer connections
// are prioritized over the older ones.
@@ -1144,7 +1143,7 @@
const IceParameters* P2PTransportChannel::FindRemoteIceFromUfrag(
absl::string_view ufrag,
- uint32_t* generation) {
+ uint32_t* generation) const {
RTC_DCHECK_RUN_ON(network_thread_);
const auto& params = remote_ice_parameters_;
auto it = std::find_if(
@@ -2282,8 +2281,14 @@
// Remove the address for prflx remote candidates. See
// https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats.
use_hostname_address |= c.is_prflx();
+ // Filter remote ufrag of peer-reflexive candidates before any ICE parameters
+ // are known.
+ uint32_t remote_generation = 0;
+ bool filter_ufrag =
+ c.is_prflx() &&
+ FindRemoteIceFromUfrag(c.username(), &remote_generation) == nullptr;
return c.ToSanitizedCopy(use_hostname_address,
- false /* filter_related_address */);
+ false /* filter_related_address */, filter_ufrag);
}
void P2PTransportChannel::LogCandidatePairConfig(
diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index c2c8478..334f9a0 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -30,7 +30,6 @@
#include <string>
#include <vector>
-#include "absl/functional/any_invocable.h"
#include "absl/strings/string_view.h"
#include "api/array_view.h"
#include "api/async_dns_resolver.h"
@@ -357,7 +356,7 @@
// Returns the latest remote ICE parameters or nullptr if there are no remote
// ICE parameters yet.
- IceParameters* remote_ice() {
+ const IceParameters* remote_ice() const {
RTC_DCHECK_RUN_ON(network_thread_);
return remote_ice_parameters_.empty() ? nullptr
: &remote_ice_parameters_.back();
@@ -365,7 +364,7 @@
// Returns the remote IceParameters and generation that match `ufrag`
// if found, and returns nullptr otherwise.
const IceParameters* FindRemoteIceFromUfrag(absl::string_view ufrag,
- uint32_t* generation);
+ uint32_t* generation) const;
// Returns the index of the latest remote ICE parameters, or 0 if no remote
// ICE parameters have been received.
uint32_t remote_ice_generation() {
diff --git a/p2p/base/port_allocator.cc b/p2p/base/port_allocator.cc
index 6eab1d3..a9b177f 100644
--- a/p2p/base/port_allocator.cc
+++ b/p2p/base/port_allocator.cc
@@ -10,15 +10,24 @@
#include "p2p/base/port_allocator.h"
+#include <cstdint>
#include <iterator>
+#include <memory>
#include <optional>
#include <set>
#include <utility>
+#include <vector>
#include "absl/strings/string_view.h"
+#include "api/candidate.h"
+#include "api/transport/enums.h"
#include "p2p/base/ice_credentials_iterator.h"
+#include "p2p/base/port.h"
+#include "p2p/base/port_interface.h"
+#include "p2p/base/transport_description.h"
#include "rtc_base/checks.h"
#include "rtc_base/crypto_random.h"
+#include "rtc_base/socket_address.h"
namespace cricket {
@@ -319,7 +328,8 @@
((c.is_stun() && filter_stun_related_address) ||
(c.is_relay() && filter_turn_related_address) ||
(c.is_prflx() && filter_prflx_related_address));
- return c.ToSanitizedCopy(use_hostname_address, filter_related_address);
+ return c.ToSanitizedCopy(use_hostname_address, filter_related_address,
+ /*filter_ufrag=*/false);
}
} // namespace cricket
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index ddffa16..8c2f1a4 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1031,7 +1031,10 @@
candidate_stats->related_port =
static_cast<int32_t>(related_address.port());
}
- candidate_stats->username_fragment = candidate.username();
+ const std::string& username = candidate.username();
+ if (!username.empty()) {
+ candidate_stats->username_fragment = username;
+ }
if (candidate.protocol() == "tcp") {
candidate_stats->tcp_type = candidate.tcptype();
}
