Clear address:port in icecandidateerror for tcp servers with private IP

Bug: chromium:1072401 Change-Id: I6af81a2b2b22b5f8d7edb8fb7f66f69b866db1c6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174753 Reviewed-by: Harald Alvestrand <[email protected]> Reviewed-by: Henrik Boström <[email protected]> Commit-Queue: Eldar Rello <[email protected]> Cr-Commit-Position: refs/heads/master@{#31275}
facedawn · May 15, 2020 · fa8019c · fa8019c
1 parent f9c6b68
commit fa8019c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 8 deletions.
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
@@ -367,7 +367,7 @@ void TurnPort::PrepareAddress() {
                      << server_address_.address.ToSensitiveString();
     if (!CreateTurnClientSocket()) {
       RTC_LOG(LS_ERROR) << "Failed to create TURN client socket";
-      OnAllocateError(STUN_ERROR_GLOBAL_FAILURE,
+      OnAllocateError(SERVER_NOT_REACHABLE_ERROR,
                       "Failed to create TURN client socket.");
       return;
     }
@@ -883,12 +883,17 @@ void TurnPort::OnAllocateError(int error_code, const std::string& reason) {
   // port initialization. This way it will not be blocking other port
   // creation.
   thread()->Post(RTC_FROM_HERE, this, MSG_ALLOCATE_ERROR);
+  std::string address = GetLocalAddress().HostAsSensitiveURIString();
+  int port = GetLocalAddress().port();
+  if (server_address_.proto == PROTO_TCP &&
+      server_address_.address.IsPrivateIP()) {
+    address.clear();
+    port = 0;
+  }
   SignalCandidateError(
-      this,
-      IceCandidateErrorEvent(GetLocalAddress().HostAsSensitiveURIString(),
-                             GetLocalAddress().port(),
-                             ReconstructedServerUrl(true /* use_hostname */),
-                             error_code, reason));
+      this, IceCandidateErrorEvent(
+                address, port, ReconstructedServerUrl(true /* use_hostname */),
+                error_code, reason));
 }
 
 void TurnPort::OnRefreshError() {

diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
@@ -979,8 +979,11 @@ void BasicPortAllocatorSession::OnCandidateError(
     const IceCandidateErrorEvent& event) {
   RTC_DCHECK_RUN_ON(network_thread_);
   RTC_DCHECK(FindPort(port));
-
-  SignalCandidateError(this, event);
+  if (event.address.empty()) {
+    candidate_error_events_.push_back(event);
+  } else {
+    SignalCandidateError(this, event);
+  }
 }
 
 Port* BasicPortAllocatorSession::GetBestTurnPortForNetwork(
@@ -1140,6 +1143,10 @@ void BasicPortAllocatorSession::MaybeSignalCandidatesAllocationDone() {
       RTC_LOG(LS_INFO) << "All candidates gathered for " << content_name()
                        << ":" << component() << ":" << generation();
     }
+    for (const auto& event : candidate_error_events_) {
+      SignalCandidateError(this, event);
+    }
+    candidate_error_events_.clear();
     SignalCandidatesAllocationDone(this);
   }
 }

diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h
@@ -269,6 +269,7 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
   std::vector<PortConfiguration*> configs_;
   std::vector<AllocationSequence*> sequences_;
   std::vector<PortData> ports_;
+  std::vector<IceCandidateErrorEvent> candidate_error_events_;
   uint32_t candidate_filter_ = CF_ALL;
   // Policy on how to prune turn ports, taken from the port allocator.
   webrtc::PortPrunePolicy turn_port_prune_policy_;

diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
@@ -6173,6 +6173,35 @@ TEST_P(PeerConnectionIntegrationTest, OnIceCandidateError) {
   EXPECT_NE(caller()->error_event().address, "");
 }
 
+TEST_P(PeerConnectionIntegrationTest, OnIceCandidateErrorWithEmptyAddress) {
+  webrtc::PeerConnectionInterface::IceServer ice_server;
+  ice_server.urls.push_back("turn:127.0.0.1:3478?transport=tcp");
+  ice_server.username = "test";
+  ice_server.password = "test";
+
+  PeerConnectionInterface::RTCConfiguration caller_config;
+  caller_config.servers.push_back(ice_server);
+  caller_config.type = webrtc::PeerConnectionInterface::kRelay;
+  caller_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
+
+  PeerConnectionInterface::RTCConfiguration callee_config;
+  callee_config.servers.push_back(ice_server);
+  callee_config.type = webrtc::PeerConnectionInterface::kRelay;
+  callee_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
+
+  ASSERT_TRUE(
+      CreatePeerConnectionWrappersWithConfig(caller_config, callee_config));
+
+  // Do normal offer/answer and wait for ICE to complete.
+  ConnectFakeSignaling();
+  caller()->AddAudioVideoTracks();
+  callee()->AddAudioVideoTracks();
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  EXPECT_EQ_WAIT(701, caller()->error_event().error_code, kDefaultTimeout);
+  EXPECT_EQ(caller()->error_event().address, "");
+}
+
 TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
        AudioKeepsFlowingAfterImplicitRollback) {
   PeerConnectionInterface::RTCConfiguration config;