Revert of Allow the localhost IP address even if it does not match the tcp port address (patchset #4 id:120001 of https://codereview.webrtc.org/1914803002/ )
Reason for revert:
Speculatively reverting due to failures on the memcheck bot (and possibly other bots):
https://build.chromium.org/p/client.webrtc/builders/Linux%20Memcheck/builds/5910/steps/video_engine_tests/logs/EndToEndTest.SendsAndReceivesH264
Original issue's description:
> This fixes an issue similar to
> https://bugs.chromium.org/p/webrtc/issues/detail?id=3927
> where the localhost IP does not match the turn port address.
> The issue here is in the TCP port.
>
> BUG=
> R=pthatcher@webrtc.org
>
> Committed: https://crrev.com/6705012904e6cbbefce6fbce0a3f615b7aeafd8f
> Cr-Commit-Position: refs/heads/master@{#12707}
TBR=pthatcher@webrtc.org,deadbeef@webrtc.org,honghaiz@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review-Url: https://codereview.webrtc.org/1979463003
Cr-Original-Commit-Position: refs/heads/master@{#12728}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 1cbf0a73eb4b475e8beb878ea3a4d650191f0c08
diff --git a/base/virtualsocketserver.cc b/base/virtualsocketserver.cc
index c76fe42..c6952b4 100644
--- a/base/virtualsocketserver.cc
+++ b/base/virtualsocketserver.cc
@@ -578,9 +578,7 @@
}
VirtualSocket* VirtualSocketServer::CreateSocketInternal(int family, int type) {
- VirtualSocket* socket = new VirtualSocket(this, family, type, true);
- SignalSocketCreated(socket);
- return socket;
+ return new VirtualSocket(this, family, type, true);
}
void VirtualSocketServer::SetMessageQueue(MessageQueue* msg_queue) {
diff --git a/base/virtualsocketserver.h b/base/virtualsocketserver.h
index 897ba9e..a83be28 100644
--- a/base/virtualsocketserver.h
+++ b/base/virtualsocketserver.h
@@ -122,9 +122,6 @@
bool CloseTcpConnections(const SocketAddress& addr_local,
const SocketAddress& addr_remote);
- // For testing purpose only. Fired when a client socket is created.
- sigslot::signal1<VirtualSocket*> SignalSocketCreated;
-
protected:
// Returns a new IP not used before in this network.
IPAddress GetNextIP(int family);
diff --git a/p2p/base/tcpport.cc b/p2p/base/tcpport.cc
index 5ccb810..59c4f31 100644
--- a/p2p/base/tcpport.cc
+++ b/p2p/base/tcpport.cc
@@ -387,35 +387,28 @@
// 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.
- const rtc::SocketAddress& socket_addr = socket->GetLocalAddress();
- if (socket_addr.ipaddr() == port()->ip()) {
- LOG_J(LS_VERBOSE, this) << "Connection established to "
- << socket->GetRemoteAddress().ToSensitiveString();
- } else if (IPIsAny(port()->ip())) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket_addr.ipaddr().ToString()
- << ", rather then the local port:"
- << port()->ip().ToString()
- << ". Still allowing it since it's any address"
- << ", possibly caused by multi-routes being disabled.";
- } else if (socket_addr.IsLoopbackIP()) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket_addr.ipaddr().ToString()
- << ", rather then the local port:"
- << port()->ip().ToString()
- << ". Still allowing it since it's localhost.";
+ const rtc::IPAddress& socket_ip = socket->GetLocalAddress().ipaddr();
+ if (socket_ip == port()->ip() || IPIsAny(port()->ip())) {
+ if (socket_ip == port()->ip()) {
+ LOG_J(LS_VERBOSE, this) << "Connection established to "
+ << socket->GetRemoteAddress().ToSensitiveString();
+ } else {
+ LOG(LS_WARNING) << "Socket is bound to a different address:"
+ << socket->GetLocalAddress().ipaddr().ToString()
+ << ", rather then the local port:"
+ << port()->ip().ToString()
+ << ". Still allowing it since it's any address"
+ << ", possibly caused by multi-routes being disabled.";
+ }
+ set_connected(true);
+ connection_pending_ = false;
} else {
LOG_J(LS_WARNING, this) << "Dropping connection as TCP socket bound to IP "
- << socket_addr.ipaddr().ToSensitiveString()
+ << socket_ip.ToSensitiveString()
<< ", different from the local candidate IP "
<< port()->ip().ToSensitiveString();
OnClose(socket, 0);
- return;
}
-
- // Connection is established successfully.
- set_connected(true);
- connection_pending_ = false;
}
void TCPConnection::OnClose(rtc::AsyncPacketSocket* socket, int error) {
diff --git a/p2p/base/tcpport_unittest.cc b/p2p/base/tcpport_unittest.cc
deleted file mode 100644
index 77fb790..0000000
--- a/p2p/base/tcpport_unittest.cc
+++ /dev/null
@@ -1,85 +0,0 @@
-/*
- * Copyright 2016 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.
- */
-
-#include "webrtc/base/gunit.h"
-#include "webrtc/base/physicalsocketserver.h"
-#include "webrtc/base/thread.h"
-#include "webrtc/base/virtualsocketserver.h"
-#include "webrtc/p2p/base/basicpacketsocketfactory.h"
-#include "webrtc/p2p/base/tcpport.h"
-
-using rtc::SocketAddress;
-using cricket::Connection;
-using cricket::Port;
-using cricket::TCPPort;
-using cricket::ICE_UFRAG_LENGTH;
-using cricket::ICE_PWD_LENGTH;
-
-static int kTimeout = 1000;
-static const SocketAddress kLocalAddr("11.11.11.11", 1);
-static const SocketAddress kRemoteAddr("22.22.22.22", 2);
-
-class TCPPortTest : public testing::Test, public sigslot::has_slots<> {
- public:
- TCPPortTest()
- : main_(rtc::Thread::Current()),
- pss_(new rtc::PhysicalSocketServer),
- ss_(new rtc::VirtualSocketServer(pss_.get())),
- ss_scope_(ss_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
- socket_factory_(rtc::Thread::Current()),
- username_(rtc::CreateRandomString(ICE_UFRAG_LENGTH)),
- password_(rtc::CreateRandomString(ICE_PWD_LENGTH)) {
- network_.AddIP(rtc::IPAddress(INADDR_ANY));
- }
-
- void ConnectSignalSocketCreated() {
- ss_->SignalSocketCreated.connect(this, &TCPPortTest::OnSocketCreated);
- }
-
- void OnSocketCreated(rtc::VirtualSocket* socket) {
- LOG(LS_INFO) << "socket created ";
- socket->SignalAddressReady.connect(
- this, &TCPPortTest::SetLocalhostAsAlternativeLocalAddress);
- }
-
- void SetLocalhostAsAlternativeLocalAddress(rtc::VirtualSocket* socket,
- const SocketAddress& address) {
- SocketAddress local_address("127.0.0.1", 2000);
- socket->SetAlternativeLocalAddress(local_address);
- }
-
- TCPPort* CreateTCPPort(const SocketAddress& addr) {
- return TCPPort::Create(main_, &socket_factory_, &network_, addr.ipaddr(), 0,
- 0, username_, password_, true);
- }
-
- protected:
- rtc::Thread* main_;
- rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
- rtc::scoped_ptr<rtc::VirtualSocketServer> ss_;
- rtc::SocketServerScope ss_scope_;
- rtc::Network network_;
- rtc::BasicPacketSocketFactory socket_factory_;
- std::string username_;
- std::string password_;
-};
-
-TEST_F(TCPPortTest, TestTCPPortWithLocalhostAddress) {
- rtc::scoped_ptr<TCPPort> lport(CreateTCPPort(kLocalAddr));
- rtc::scoped_ptr<TCPPort> rport(CreateTCPPort(kRemoteAddr));
- lport->PrepareAddress();
- rport->PrepareAddress();
- // Start to listen to new socket creation event.
- ConnectSignalSocketCreated();
- Connection* conn =
- lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
- EXPECT_TRUE_WAIT(conn->connected(), kTimeout);
-}
diff --git a/p2p/p2p.gyp b/p2p/p2p.gyp
index 08fac69..a15fa52 100644
--- a/p2p/p2p.gyp
+++ b/p2p/p2p.gyp
@@ -162,7 +162,6 @@
'base/transport_unittest.cc',
'base/transportcontroller_unittest.cc',
'base/transportdescriptionfactory_unittest.cc',
- 'base/tcpport_unittest.cc',
'base/turnport_unittest.cc',
'client/fakeportallocator.h',
'client/portallocator_unittest.cc',