From f868b76376fd2c8837893dde683b6b43002564e4 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 15 Jan 2024 13:28:24 +0100 Subject: [PATCH] Fix empty hostname in getStats candidate url and show IPv6 in [] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit observed in a Teams call as "turn::3478?transport=udp". This works for TURN TCP candidates but not for UDP which turned out to be a timing issue as the server_address' hostname got cleared by DNS resolution. Also add [] around IPv6 addresses. Bug: webrtc:15770 Change-Id: I14ba4fb7088d5a75b1d61739fae2b165be910b75 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/333781 Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#41537} --- p2p/base/turn_port.cc | 19 +++++++++---------- p2p/base/turn_port.h | 9 ++++++--- p2p/base/turn_port_unittest.cc | 3 ++- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index f5d8e3d805..ff50636794 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -228,6 +228,7 @@ TurnPort::TurnPort(TaskQueueBase* thread, password, field_trials), server_address_(server_address), + server_url_(ReconstructServerUrl()), tls_alpn_protocols_(tls_alpn_protocols), tls_elliptic_curves_(tls_elliptic_curves), tls_cert_verifier_(tls_cert_verifier), @@ -271,6 +272,7 @@ TurnPort::TurnPort(TaskQueueBase* thread, password, field_trials), server_address_(server_address), + server_url_(ReconstructServerUrl()), tls_alpn_protocols_(tls_alpn_protocols), tls_elliptic_curves_(tls_elliptic_curves), tls_cert_verifier_(tls_cert_verifier), @@ -886,7 +888,7 @@ void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address, ProtoToString(server_address_.proto), // The first hop protocol. "", // TCP candidate type, empty for turn candidates. RELAY_PORT_TYPE, GetRelayPreference(server_address_.proto), - server_priority_, ReconstructedServerUrl(), true); + server_priority_, server_url_, true); } void TurnPort::OnAllocateError(int error_code, absl::string_view reason) { @@ -902,9 +904,8 @@ void TurnPort::OnAllocateError(int error_code, absl::string_view reason) { address.clear(); port = 0; } - SignalCandidateError( - this, IceCandidateErrorEvent(address, port, ReconstructedServerUrl(), - error_code, reason)); + SignalCandidateError(this, IceCandidateErrorEvent(address, port, server_url_, + error_code, reason)); } void TurnPort::OnRefreshError() { @@ -1255,15 +1256,13 @@ bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address, return true; } -std::string TurnPort::ReconstructedServerUrl() { - // draft-petithuguenin-behave-turn-uris-01 - // turnURI = scheme ":" turn-host [ ":" turn-port ] +std::string TurnPort::ReconstructServerUrl() { + // https://www.rfc-editor.org/rfc/rfc7065#section-3.1 + // turnURI = scheme ":" host [ ":" port ] // [ "?transport=" transport ] // scheme = "turn" / "turns" // transport = "udp" / "tcp" / transport-ext // transport-ext = 1*unreserved - // turn-host = IP-literal / IPv4address / reg-name - // turn-port = *DIGIT std::string scheme = "turn"; std::string transport = "tcp"; switch (server_address_.proto) { @@ -1278,7 +1277,7 @@ std::string TurnPort::ReconstructedServerUrl() { break; } rtc::StringBuilder url; - url << scheme << ":" << server_address_.address.hostname() << ":" + url << scheme << ":" << server_address_.address.HostAsURIString() << ":" << server_address_.address.port() << "?transport=" << transport; return url.Release(); } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 686edaf595..b56f862e61 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -302,9 +302,6 @@ class TurnPort : public Port { // pruned (a.k.a. write-timed-out). Returns true if a connection is found. bool FailAndPruneConnection(const rtc::SocketAddress& address); - // Reconstruct the URL of the server which the candidate is gathered from. - std::string ReconstructedServerUrl(); - void MaybeAddTurnLoggingId(StunMessage* message); void TurnCustomizerMaybeModifyOutgoingStunMessage(StunMessage* message); @@ -313,6 +310,12 @@ class TurnPort : public Port { bool payload); ProtocolAddress server_address_; + // Reconstruct the URL of the server which the candidate is gathered from. + // A copy needs to be stored as server_address_ will resolve and clear its + // hostname field. + std::string ReconstructServerUrl(); + std::string server_url_; + TlsCertPolicy tls_cert_policy_ = TlsCertPolicy::TLS_CERT_POLICY_SECURE; std::vector tls_alpn_protocols_; std::vector tls_elliptic_curves_; diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index e7efb5e594..169467d76c 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -878,9 +878,10 @@ TEST_F(TurnPortTest, TestReconstructedServerUrlForUdpIPv6) { turn_server_.AddInternalSocket(kTurnUdpIPv6IntAddr, PROTO_UDP); CreateTurnPort(kLocalIPv6Addr, kTurnUsername, kTurnPassword, kTurnUdpIPv6ProtoAddr); + // Should add [] around the IPv6. TestReconstructedServerUrl( PROTO_UDP, - "turn:2400:4030:1:2c00:be30:abcd:efab:cdef:3478?transport=udp"); + "turn:[2400:4030:1:2c00:be30:abcd:efab:cdef]:3478?transport=udp"); } TEST_F(TurnPortTest, TestReconstructedServerUrlForTcp) {