Fix AllocationSequence to handle the case when TurnPort stops using shared socket.

webrtc/p2p/client/basicportallocator.cc

Index: webrtc/p2p/client/basicportallocator.cc
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 22abf33e93639d90b276ca9d6c5cc025fca3a6e0..413c968df5b3273c46427c3d9ca5b61b87f7e6e1 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -1072,7 +1072,6 @@ void AllocationSequence::OnReadPacket(
     const rtc::PacketTime& packet_time) {
   ASSERT(socket == udp_socket_.get());
 
-  bool turn_port_found = false;
 
   // Try to find the TurnPort that matches the remote address. Note that the
   // message could be a STUN binding response if the TURN server is also used as
@@ -1080,13 +1079,12 @@ void AllocationSequence::OnReadPacket(
   // a STUN binding response, so we pass the message to TurnPort regardless of
   // the message type. The TurnPort will just ignore the message since it will
   // not find any request by transaction ID.
-  for (std::vector<TurnPort*>::const_iterator it = turn_ports_.begin();
-       it != turn_ports_.end(); ++it) {
-    TurnPort* port = *it;
+  for (TurnPort* port : turn_ports_) {
     if (port->server_address().address == remote_addr) {
-      port->HandleIncomingPacket(socket, data, size, remote_addr, packet_time);
-      turn_port_found = true;
-      break;
+      if (port->HandleIncomingPacket(socket, data, size, remote_addr,
+                                     packet_time)) {
+        return;
+      }
     }
   }
 
@@ -1095,10 +1093,10 @@ void AllocationSequence::OnReadPacket(
 
     // Pass the packet to the UdpPort if there is no matching TurnPort, or if
     // the TURN server is also a STUN server.
-    if (!turn_port_found ||
-        stun_servers.find(remote_addr) != stun_servers.end()) {
-      udp_port_->HandleIncomingPacket(
-          socket, data, size, remote_addr, packet_time);
+    if (stun_servers.find(remote_addr) != stun_servers.end()) {

[honghaiz3, 2016/04/22 01:49:03]

I think this is incorrect. 
In the current code, a packet coming from the other peer directly will be routed
through the UDP port (because of the condition (!turn_port_found). 
With the new change, that packet will be dropped.

[honghaiz3, 2016/04/22 03:52:33]

One possible approach is to use the existing logic; if the source addr is not
found in turn servers or it is found in stun servers, pass it to the udp port.
But then every turn channel packet is sent to udp port (and will be dropped
there). 
A better approach is, if the source address is not found in turn servers or (it
is found there but not consumed and it is also found in stun servers), pass it
to the udp port. 

[Sergey Ulanov, 2016/04/22 18:05:52]

Implemented this suggestion in Patch Set 4.
I'm not sure it it's actually necessary to check if the address in the list of
STUN servers. I think the logic could be simplified:
 1. Find a turn port with the given remote_addr and try passing the new packet
there.
 2. If the packet is not handled by any of the turn ports then pass it to the
udp_port_.

This would simplify the code and I don't think it would make any difference
otherwise. UDPPort::OnReadPacket() should be able to handle the packet
appropriately. I've implemented this approach in Patch Set 5. WDYT?

[honghaiz3, 2016/04/22 19:17:27]

I think patch set 4 is slightly safer. 
Although patch set 5 looks nicer, it leaves a potential hole: 
For those packets whose source address is found in the turn port addresses but
not consumed by turn port, they will always be passed to the stun port. It has
no problem if it is also found in the stun_server addresses (just like patch set
4). But if it is not found in the stun server addresses, the packets will be
treated as app data in the stun port, which may cause problems later (although
in our implementation, they will be dropped). 

+      RTC_DCHECK(udp_port_->SharedSocket());
+      udp_port_->HandleIncomingPacket(socket, data, size, remote_addr,
+                                      packet_time);
     }
   }
 }