Connection resurrected with incorrect candidate type.

webrtc/p2p/base/p2ptransportchannel.cc

Index: webrtc/p2p/base/p2ptransportchannel.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 95696e1e2c8d422773d1f916d5034794b82a686c..b843b7a75ddb7ab2a30275164eb511cc59927cf2 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -478,13 +478,15 @@ void P2PTransportChannel::OnUnknownAddress(
     remote_password = remote_ice_pwd_;
   }
 
-  Candidate new_remote_candidate;
+  Candidate remote_candidate;
+  bool remote_candidate_is_new = false;

[pthatcher1, 2015/06/09 23:58:03]

Perhaps more clear as:

bool remote_candidate_is_new = (candidate == nullptr);
if (candidate_is_new) {
  ...
} else {
  ..
}

[guoweis_webrtc, 2015/06/10 18:12:41]

Done.

   if (candidate != NULL) {
-    new_remote_candidate = *candidate;
+    remote_candidate = *candidate;
     if (ufrag_per_port) {
-      new_remote_candidate.set_address(address);
+      remote_candidate.set_address(address);
     }
   } else {
+    remote_candidate_is_new = true;
     // Create a new candidate with this address.
     std::string type;
     if (port->IceProtocol() == ICEPROTO_RFC5245) {
@@ -501,17 +503,17 @@ void P2PTransportChannel::OnUnknownAddress(
       }
     }
 
-    new_remote_candidate =
+    remote_candidate =
         Candidate(component(), ProtoToString(proto), address, 0,
                   remote_username, remote_password, type, 0U, "");
 
     // From RFC 5245, section-7.2.1.3:
     // The foundation of the candidate is set to an arbitrary value, different
     // from the foundation for all other remote candidates.
-    new_remote_candidate.set_foundation(
-        rtc::ToString<uint32>(rtc::ComputeCrc32(new_remote_candidate.id())));
+    remote_candidate.set_foundation(
+        rtc::ToString<uint32>(rtc::ComputeCrc32(remote_candidate.id())));
 
-    new_remote_candidate.set_priority(new_remote_candidate.GetPriority(
+    remote_candidate.set_priority(remote_candidate.GetPriority(
         ICE_TYPE_PREFERENCE_PRFLX, port->Network()->preference(), 0));
   }
 
@@ -534,7 +536,9 @@ void P2PTransportChannel::OnUnknownAddress(
                                      STUN_ERROR_REASON_BAD_REQUEST);
       return;
     }
-    new_remote_candidate.set_priority(priority_attr->value());
+    if (remote_candidate_is_new) {
+      remote_candidate.set_priority(priority_attr->value());
+    }

[pthatcher1, 2015/06/09 23:58:03]

Can you move this up into the rest of the "remote_candidate_is_new" code?  That
way we'll have one set_priority instead of two:

if (port->IceProtocol() == ICEPROTO_RFC5245) {
  ...
  remote_candidate.set_priority(priority_attr->value())
} else {
  remote_candidate.set_priority(remote_candidate.GetPriority(
    ICE_TYPE_PREFERENCE_PRFLX, port->Network()->preference(), 0));
}

[guoweis_webrtc, 2015/06/10 18:12:41]

Done.

 
     // RFC5245, the agent constructs a pair whose local candidate is equal to
     // the transport address on which the STUN request was received, and a
@@ -545,10 +549,10 @@ void P2PTransportChannel::OnUnknownAddress(
     // When ports are muxed, this channel might get multiple unknown address
     // signals. In that case if the connection is already exists, we should
     // simply ignore the signal othewise send server error.
-    if (port->GetConnection(new_remote_candidate.address())) {
+    if (port->GetConnection(remote_candidate.address())) {
       if (port_muxed) {
         LOG(LS_INFO) << "Connection already exists for peer reflexive "
-                     << "candidate: " << new_remote_candidate.ToString();
+                     << "candidate: " << remote_candidate.ToString();
         return;
       } else {
         ASSERT(false);
@@ -560,7 +564,7 @@ void P2PTransportChannel::OnUnknownAddress(
     }
 
     Connection* connection = port->CreateConnection(
-        new_remote_candidate, cricket::PortInterface::ORIGIN_THIS_PORT);
+        remote_candidate, cricket::PortInterface::ORIGIN_THIS_PORT);
     if (!connection) {
       ASSERT(false);
       port->SendBindingErrorResponse(stun_msg, address,
@@ -569,8 +573,9 @@ void P2PTransportChannel::OnUnknownAddress(
       return;
     }
 
-    LOG(LS_INFO) << "Adding connection from peer reflexive candidate: "
-                 << new_remote_candidate.ToString();
+    LOG(LS_INFO) << "Adding connection from "
+                 << (remote_candidate_is_new ? "peer reflexive" : "resurrected")
+                 << " candidate: " << remote_candidate.ToString();
     AddConnection(connection);
     connection->ReceivedPing();
 
@@ -585,7 +590,7 @@ void P2PTransportChannel::OnUnknownAddress(
     // Check for connectivity to this address. Create connections
     // to this address across all local ports. First, add this as a new remote
     // address
-    if (!CreateConnections(new_remote_candidate, port, true)) {
+    if (!CreateConnections(remote_candidate, port, true)) {
       // Hopefully this won't occur, because changing a destination address
       // shouldn't cause a new connection to fail
       ASSERT(false);