Issue 4780: disabling multiple_routes breaks Turn/Tcp.

webrtc/p2p/base/turnport.cc

 /*
  *  Copyright 2012 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 315 matching lines @@
   return true;
 }
 
 void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) {
   ASSERT(server_address_.proto == PROTO_TCP);
   // Do not use this port if the socket bound to a different address than
   // the one we asked for. This is seen in Chrome, where TCP sockets cannot be
   // given a binding address, and the platform is expected to pick the
   // correct local address.
 
-  // Further, to workaround issue 3927 in which a proxy is forcing TCP bound to
-  // localhost only, we're allowing Loopback IP even if it's not the same as the
-  // local Turn port.
+  // Further, 2 exceptions exist. 1. issue 3927 in which a proxy is forcing TCP
+  // bound to localhost only, we're allowing Loopback IP even if it's not the
+  // same as the local Turn port. 2. issue 4780 when multiple_routes is
+  // disabled, the local ip is bound to any address.

[pthatcher1, 2015/06/18 21:25:30]

I think we could make this comment more clear.   Maybe like this:

However, there are two situations in which we allow the bound address to differ
from the requested address:
1.  The bound address is the loopback address.  This happens when a proxy forces
TCP to bind to only the localhost address (see issue 3927).
2.  The bound address is the "any address".  This happens when multiple_routes
is disabled (see issue 4780).

[guoweis_webrtc, 2015/06/19 17:37:49]

Done.

   if (socket->GetLocalAddress().ipaddr() != ip()) {
-    if (socket->GetLocalAddress().IsLoopbackIP()) {
+    if (socket->GetLocalAddress().IsLoopbackIP() ||
+        IPIsAny(ip())) {
       LOG(LS_WARNING) << "Socket is bound to a different address:"
                       << socket->GetLocalAddress().ipaddr().ToString()
                       << ", rather then the local port:" << ip().ToString()
-                      << ". Still allowing it since it's localhost.";
+                      << ". Still allowing it.";

[pthatcher1, 2015/06/18 21:25:31]

I think it would be better to have two different log messages:

if (socket->GetLocalAddress().ipaddr() != ip()) {
  if (socket->GetLocalAddress().IsLoopbackIP()) {
    LOG(LS_WARNING) << "... since it's localhost.";
  } else if (IPIsAny(ip())) {
    LOG(LS_WARNING) << "... since the option to use multiple routes is
disabled.";
  } else {
    LOG(LS_WARNING) << "...";
    OnAllocateError();
    return;
  }
}
  }

[guoweis_webrtc, 2015/06/19 17:37:49]

Done.

     } else {
       LOG(LS_WARNING) << "Socket is bound to a different address:"
                       << socket->GetLocalAddress().ipaddr().ToString()
                       << ", rather then the local port:" << ip().ToString()
                       << ". Discarding TURN port.";
       OnAllocateError();
       return;
     }
   }
 
@@ skipping 973 matching lines @@
   // bind request as per http://tools.ietf.org/html/rfc5766#section-11.3
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       // Send channel bind request with fresh nonce.
       SendChannelBindRequest(0);
     }
   }
 }
 
 }  // namespace cricket