Workaround for issue 3927 to allow localhost IP even if it doesn't match the local turn port

BUG=3927
R=pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/28329004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7941 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/base/virtualsocketserver.cc

@@ -51,6 +51,7 @@ const int NUM_SAMPLES = 1000;
 
 enum {
   MSG_ID_PACKET,
+  MSG_ID_ADDRESS_BOUND,
   MSG_ID_CONNECT,
   MSG_ID_DISCONNECT,
 };
@@ -130,11 +131,14 @@ SocketAddress VirtualSocket::GetRemoteAddress() const {
   return remote_addr_;
 }
 
-// Used by server sockets to set the local address without binding.
 void VirtualSocket::SetLocalAddress(const SocketAddress& addr) {
   local_addr_ = addr;
 }
 
+void VirtualSocket::SetAlternativeLocalAddress(const SocketAddress& addr) {
+  alternative_local_addr_ = addr;
+}
+
 int VirtualSocket::Bind(const SocketAddress& addr) {
   if (!local_addr_.IsNil()) {
     error_ = EINVAL;
@@ -148,6 +152,9 @@ int VirtualSocket::Bind(const SocketAddress& addr) {
   } else {
     bound_ = true;
     was_any_ = addr.IsAnyIP();
+    // Post a message here such that test case could have chance to
+    // process the local address. (i.e. SetAlternativeLocalAddress).
+    server_->msg_queue_->Post(this, MSG_ID_ADDRESS_BOUND);
   }
   return result;
 }
@@ -411,6 +418,8 @@ void VirtualSocket::OnMessage(Message* pmsg) {
         SignalCloseEvent(this, error);
       }
     }
+  } else if (pmsg->message_id == MSG_ID_ADDRESS_BOUND) {
+    SignalAddressReady(this, GetLocalAddress());
   } else {
     ASSERT(false);
   }

webrtc/base/virtualsocketserver.h

@@ -246,6 +246,11 @@ class VirtualSocket : public AsyncSocket, public MessageHandler {
   // Used by server sockets to set the local address without binding.
   void SetLocalAddress(const SocketAddress& addr);
 
+  // Used by TurnPortTest to mimic a case where proxy returns local host address
+  // instead of the original one TurnPort was bound against. Please see WebRTC
+  // issue 3927 for more detail.
+  void SetAlternativeLocalAddress(const SocketAddress& addr);
+
   virtual int Bind(const SocketAddress& addr);
   virtual int Connect(const SocketAddress& addr);
   virtual int Close();

webrtc/p2p/base/turnport.cc

@@ -309,11 +309,24 @@ void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) {
   // 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.
   if (socket->GetLocalAddress().ipaddr() != ip()) {
-    LOG(LS_WARNING) << "Socket is bound to a different address then the "
+    if (socket->GetLocalAddress().IsLoopbackIP()) {
-                    << "local port. Discarding TURN port.";
+      LOG(LS_WARNING) << "Socket is bound to a different address:"
-    OnAllocateError();
+                      << socket->GetLocalAddress().ipaddr().ToString()
-    return;
+                      << ", rather then the local port:" << ip().ToString()
+                      << ". Still allowing it since it's localhost.";
+    } 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;
+    }
   }
 
   if (server_address_.address.IsUnresolved()) {

webrtc/p2p/base/turnport_unittest.cc

@@ -86,6 +86,14 @@ static int GetFDCount() {
 }
 #endif
 
+class TurnPortTestVirtualSocketServer : public rtc::VirtualSocketServer {
+ public:
+  explicit TurnPortTestVirtualSocketServer(SocketServer* ss)
+      : VirtualSocketServer(ss) {}
+
+  using rtc::VirtualSocketServer::LookupBinding;
+};
+
 class TurnPortTest : public testing::Test,
                      public sigslot::has_slots<>,
                      public rtc::MessageHandler {
@@ -93,7 +101,7 @@ class TurnPortTest : public testing::Test,
   TurnPortTest()
       : main_(rtc::Thread::Current()),
         pss_(new rtc::PhysicalSocketServer),
-        ss_(new rtc::VirtualSocketServer(pss_.get())),
+        ss_(new TurnPortTestVirtualSocketServer(pss_.get())),
         ss_scope_(ss_.get()),
         network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
         socket_factory_(rtc::Thread::Current()),
@@ -113,6 +121,21 @@ class TurnPortTest : public testing::Test,
       test_finish_ = true;
   }
 
+  void ConnectSignalAddressReadyToSetLocalhostAsAltenertativeLocalAddress() {
+    rtc::AsyncPacketSocket* socket = turn_port_->socket();
+    rtc::VirtualSocket* virtual_socket =
+        ss_->LookupBinding(socket->GetLocalAddress());
+    virtual_socket->SignalAddressReady.connect(
+        this, &TurnPortTest::SetLocalhostAsAltenertativeLocalAddress);
+  }
+
+  void SetLocalhostAsAltenertativeLocalAddress(
+      rtc::VirtualSocket* socket,
+      const rtc::SocketAddress& address) {
+    SocketAddress local_address("127.0.0.1", 2000);
+    socket->SetAlternativeLocalAddress(local_address);
+  }
+
   void OnTurnPortComplete(Port* port) {
     turn_ready_ = true;
   }
@@ -312,7 +335,7 @@ class TurnPortTest : public testing::Test,
  protected:
   rtc::Thread* main_;
   rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
-  rtc::scoped_ptr<rtc::VirtualSocketServer> ss_;
+  rtc::scoped_ptr<TurnPortTestVirtualSocketServer> ss_;
   rtc::SocketServerScope ss_scope_;
   rtc::Network network_;
   rtc::BasicPacketSocketFactory socket_factory_;
@@ -356,6 +379,22 @@ TEST_F(TurnPortTest, TestTurnTcpAllocate) {
   EXPECT_NE(0, turn_port_->Candidates()[0].address().port());
 }
 
+// Test case for WebRTC issue 3927 where a proxy binds to the local host address
+// instead the address that TurnPort originally bound to. The candidate pair
+// impacted by this behavior should still be used.
+TEST_F(TurnPortTest, TestTurnTcpAllocationWhenProxyChangesAddressToLocalHost) {
+  turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
+  CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
+  EXPECT_EQ(0, turn_port_->SetOption(rtc::Socket::OPT_SNDBUF, 10 * 1024));
+  turn_port_->PrepareAddress();
+  ConnectSignalAddressReadyToSetLocalhostAsAltenertativeLocalAddress();
+  EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
+  ASSERT_EQ(1U, turn_port_->Candidates().size());
+  EXPECT_EQ(kTurnUdpExtAddr.ipaddr(),
+            turn_port_->Candidates()[0].address().ipaddr());
+  EXPECT_NE(0, turn_port_->Candidates()[0].address().port());
+}
+
 // Testing turn port will attempt to create TCP socket on address resolution
 // failure.
 TEST_F(TurnPortTest, DISABLED_TestTurnTcpOnAddressResolveFailure) {