admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland of Allow the localhost IP address even if it does not match the tcp port address (patchset #1 id:1 of https://codereview.webrtc.org/1979463003/ )

Browse Source 
Reason for revert:
Relanding this change since the revert didn't make a difference.

Original issue's description:
> 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=
>
> Committed: https://crrev.com/1cbf0a73eb4b475e8beb878ea3a4d650191f0c08
> Cr-Commit-Position: refs/heads/master@{#12728}

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/1979073002
Cr-Commit-Position: refs/heads/master@{#12746}
This commit is contained in:
tommi 2016-05-14 03:19:31 -07:00 committed by Commit bot
parent d49c30cd2f
commit 5ce1a2a629
5 changed files with 115 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
webrtc/base/virtualsocketserver.cc
View File

@ -578,7 +578,9 @@ AsyncSocket* VirtualSocketServer::CreateAsyncSocket(int family, int type) {
}
VirtualSocket* VirtualSocketServer::CreateSocketInternal(int family, int type) {
return new VirtualSocket(this, family, type, true);
VirtualSocket* socket = new VirtualSocket(this, family, type, true);
SignalSocketCreated(socket);
return socket;
}
void VirtualSocketServer::SetMessageQueue(MessageQueue* msg_queue) {

3
webrtc/base/virtualsocketserver.h
View File

@ -122,6 +122,9 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
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);

39
webrtc/p2p/base/tcpport.cc
View File

@ -387,28 +387,35 @@ void TCPConnection::OnConnect(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.
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;
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.";
} else {
LOG_J(LS_WARNING, this) << "Dropping connection as TCP socket bound to IP "
<< socket_ip.ToSensitiveString()
<< socket_addr.ipaddr().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) {

85
webrtc/p2p/base/tcpport_unittest.cc Normal file
View File

@ -0,0 +1,85 @@
/*
* 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);
}

1
webrtc/p2p/p2p.gyp
View File

@ -164,6 +164,7 @@
'base/transport_unittest.cc',
'base/transportcontroller_unittest.cc',
'base/transportdescriptionfactory_unittest.cc',
'base/tcpport_unittest.cc',
'base/turnport_unittest.cc',
'client/basicportallocator_unittest.cc',
'stunprober/stunprober_unittest.cc',