From e97389c505e0dce02242f3ef13013279868c290e Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 23 Dec 2016 01:43:45 -0800 Subject: [PATCH] If network enumeration fails, try binding to the "ANY" address. This isn't as good as being able to enumerate all networks, but it's better than doing nothing; it still will provide STUN/TURN candidates for the default route if one exists. BUG=webrtc:6932 Review-Url: https://codereview.webrtc.org/2599673003 Cr-Commit-Position: refs/heads/master@{#15766} --- webrtc/base/network.h | 1 + webrtc/p2p/client/basicportallocator.cc | 6 ++ .../p2p/client/basicportallocator_unittest.cc | 65 ++++++++++++++----- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/webrtc/base/network.h b/webrtc/base/network.h index 7d509c6a12..623ead06ff 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h @@ -142,6 +142,7 @@ class NetworkManagerBase : public NetworkManager { void GetNetworks(NetworkList* networks) const override; void GetAnyAddressNetworks(NetworkList* networks) override; + // Defaults to true. bool ipv6_enabled() const { return ipv6_enabled_; } void set_ipv6_enabled(bool enabled) { ipv6_enabled_ = enabled; } diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index dbac0d3822..664701e100 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -551,6 +551,12 @@ std::vector BasicPortAllocatorSession::GetNetworks() { network_manager->GetAnyAddressNetworks(&networks); } else { network_manager->GetNetworks(&networks); + // If network enumeration fails, use the ANY address as a fallback, so we + // can at least try gathering candidates using the default route chosen by + // the OS. + if (networks.empty()) { + network_manager->GetAnyAddressNetworks(&networks); + } } networks.erase(std::remove_if(networks.begin(), networks.end(), [this](rtc::Network* network) { diff --git a/webrtc/p2p/client/basicportallocator_unittest.cc b/webrtc/p2p/client/basicportallocator_unittest.cc index 0eeac06deb..4ace7c8a3c 100644 --- a/webrtc/p2p/client/basicportallocator_unittest.cc +++ b/webrtc/p2p/client/basicportallocator_unittest.cc @@ -37,6 +37,7 @@ using rtc::IPAddress; using rtc::SocketAddress; using rtc::Thread; +static const SocketAddress kAnyAddr("0.0.0.0", 0); static const SocketAddress kClientAddr("11.11.11.11", 0); static const SocketAddress kClientAddr2("22.22.22.22", 0); static const SocketAddress kLoopbackAddr("127.0.0.1", 0); @@ -114,6 +115,8 @@ class BasicPortAllocatorTest : public testing::Test, vss_(new rtc::VirtualSocketServer(pss_.get())), fss_(new rtc::FirewallSocketServer(vss_.get())), ss_scope_(fss_.get()), + // Note that the NAT is not used by default. ResetWithStunServerAndNat + // must be called. nat_factory_(vss_.get(), kNatUdpAddr, kNatTcpAddr), nat_socket_factory_(new rtc::BasicPacketSocketFactory(&nat_factory_)), stun_server_(TestStunServer::Create(Thread::Current(), kStunAddr)), @@ -130,6 +133,9 @@ class BasicPortAllocatorTest : public testing::Test, stun_servers.insert(kStunAddr); // Passing the addresses of GTURN servers will enable GTURN in // Basicportallocator. + // TODO(deadbeef): Stop using GTURN by default in this test... Either the + // configuration should be blank by default (preferred), or it should use + // TURN instead. allocator_.reset(new BasicPortAllocator(&network_manager_, stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); @@ -275,6 +281,17 @@ class BasicPortAllocatorTest : public testing::Test, }); } + static int CountCandidates(const std::vector& candidates, + const std::string& type, + const std::string& proto, + const SocketAddress& addr) { + return std::count_if(candidates.begin(), candidates.end(), + [type, proto, addr](const Candidate& c) { + return c.type() == type && c.protocol() == proto && + AddressMatch(c.address(), addr); + }); + } + // Find a candidate and return it. static bool FindCandidate(const std::vector& candidates, const std::string& type, @@ -758,19 +775,6 @@ TEST_F(BasicPortAllocatorTest, TestGatherLowCostNetworkOnly) { EXPECT_TRUE(addr_wifi.EqualIPs(candidates_[0].address())); } -// Tests that we allocator session not trying to allocate ports for every 250ms. -TEST_F(BasicPortAllocatorTest, TestNoNetworkInterface) { - EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - // Waiting for one second to make sure BasicPortAllocatorSession has not - // called OnAllocate multiple times. In old behavior it's called every 250ms. - // When there are no network interfaces, each execution of OnAllocate will - // result in SignalCandidatesAllocationDone signal. - rtc::Thread::Current()->ProcessMessages(1000); - EXPECT_TRUE(candidate_allocation_done_); - EXPECT_EQ(0U, candidates_.size()); -} - // Test that we could use loopback interface as host candidate. TEST_F(BasicPortAllocatorTest, TestLoopbackNetworkInterface) { AddInterface(kLoopbackAddr, "test_loopback", rtc::ADAPTER_TYPE_LOOPBACK); @@ -936,14 +940,39 @@ TEST_F(BasicPortAllocatorTest, TestGetAllPortsPortRange) { EXPECT_TRUE(candidate_allocation_done_); } -// Test that we don't crash or malfunction if we have no network adapters. +// Test that if we have no network adapters, we bind to the ANY address and +// still get non-host candidates. TEST_F(BasicPortAllocatorTest, TestGetAllPortsNoAdapters) { + // Default config uses GTURN and no NAT, so replace that with the + // desired setup (NAT, STUN server, TURN server, UDP/TCP). + ResetWithStunServerAndNat(kStunAddr); + turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); + AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); + AddTurnServers(kTurnUdpIntIPv6Addr, kTurnTcpIntIPv6Addr); + // Disable IPv6, because our test infrastructure doesn't support having IPv4 + // behind a NAT but IPv6 not, or having an IPv6 NAT. + // TODO(deadbeef): Fix this. + network_manager_.set_ipv6_enabled(false); EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); - rtc::Thread::Current()->ProcessMessages(100); - // Without network adapter, we should not get any candidate. - EXPECT_EQ(0U, candidates_.size()); - EXPECT_TRUE(candidate_allocation_done_); + EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); + EXPECT_EQ(4U, ports_.size()); + EXPECT_EQ(1, CountPorts(ports_, "stun", PROTO_UDP, kAnyAddr)); + EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kAnyAddr)); + // Two TURN ports, using UDP/TCP for the first hop to the TURN server. + EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kAnyAddr)); + EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_TCP, kAnyAddr)); + // The "any" address port should be in the signaled ready ports, but the host + // candidate for it is useless and shouldn't be signaled. So we only have + // STUN/TURN candidates. + EXPECT_EQ(3U, candidates_.size()); + EXPECT_PRED4(HasCandidate, candidates_, "stun", "udp", + rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0)); + // Again, two TURN candidates, using UDP/TCP for the first hop to the TURN + // server. + EXPECT_EQ(2, + CountCandidates(candidates_, "relay", "udp", + rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0))); } // Test that when enumeration is disabled, we should not have any ports when