From e2da234e2767ad6e6433fbeee998b0f681100981 Mon Sep 17 00:00:00 2001 From: "buildbot@webrtc.org" Date: Wed, 23 Jul 2014 21:09:01 +0000 Subject: [PATCH] (Auto)update libjingle 71766184-> 71775619 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6768 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/portallocatorfactory.cc | 10 +- talk/app/webrtc/webrtcsession_unittest.cc | 10 +- talk/examples/call/callclient.cc | 4 +- talk/p2p/base/p2ptransportchannel_unittest.cc | 19 +- talk/p2p/base/port.h | 3 + talk/p2p/base/port_unittest.cc | 4 +- talk/p2p/base/stunport.cc | 198 +++++++++++++----- talk/p2p/base/stunport.h | 85 ++++++-- talk/p2p/base/stunport_unittest.cc | 63 +++++- talk/p2p/client/basicportallocator.cc | 57 +++-- talk/p2p/client/basicportallocator.h | 26 ++- talk/p2p/client/connectivitychecker.cc | 14 +- talk/p2p/client/connectivitychecker.h | 2 +- .../client/connectivitychecker_unittest.cc | 7 +- talk/p2p/client/httpportallocator.cc | 16 +- talk/p2p/client/portallocator_unittest.cc | 21 +- 16 files changed, 389 insertions(+), 150 deletions(-) diff --git a/talk/app/webrtc/portallocatorfactory.cc b/talk/app/webrtc/portallocatorfactory.cc index a436c44b2..7263c5dc4 100644 --- a/talk/app/webrtc/portallocatorfactory.cc +++ b/talk/app/webrtc/portallocatorfactory.cc @@ -55,19 +55,15 @@ PortAllocatorFactory::~PortAllocatorFactory() {} cricket::PortAllocator* PortAllocatorFactory::CreatePortAllocator( const std::vector& stun, const std::vector& turn) { - std::vector stun_hosts; + cricket::ServerAddresses stun_hosts; typedef std::vector::const_iterator StunIt; for (StunIt stun_it = stun.begin(); stun_it != stun.end(); ++stun_it) { - stun_hosts.push_back(stun_it->server); + stun_hosts.insert(stun_it->server); } - talk_base::SocketAddress stun_addr; - if (!stun_hosts.empty()) { - stun_addr = stun_hosts.front(); - } scoped_ptr allocator( new cricket::BasicPortAllocator( - network_manager_.get(), socket_factory_.get(), stun_addr)); + network_manager_.get(), socket_factory_.get(), stun_hosts)); for (size_t i = 0; i < turn.size(); ++i) { cricket::RelayCredentials credentials(turn[i].username, turn[i].password); diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index a1d48cf5e..460d4a4e2 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -306,12 +306,16 @@ class WebRtcSessionTest : public testing::Test { cricket::STUN_SERVER_PORT)), stun_server_(Thread::Current(), stun_socket_addr_), turn_server_(Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr), - allocator_(new cricket::BasicPortAllocator( - &network_manager_, stun_socket_addr_, - SocketAddress(), SocketAddress(), SocketAddress())), mediastream_signaling_(channel_manager_.get()), ice_type_(PeerConnectionInterface::kAll) { tdesc_factory_->set_protocol(cricket::ICEPROTO_HYBRID); + + cricket::ServerAddresses stun_servers; + stun_servers.insert(stun_socket_addr_); + allocator_.reset(new cricket::BasicPortAllocator( + &network_manager_, + stun_servers, + SocketAddress(), SocketAddress(), SocketAddress())); allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_ENABLE_BUNDLE); diff --git a/talk/examples/call/callclient.cc b/talk/examples/call/callclient.cc index 849455eaa..c691db37a 100644 --- a/talk/examples/call/callclient.cc +++ b/talk/examples/call/callclient.cc @@ -490,8 +490,10 @@ void CallClient::InitMedia() { // TODO: Decide if the relay address should be specified here. talk_base::SocketAddress stun_addr("stun.l.google.com", 19302); + cricket::ServerAddresses stun_servers; + stun_servers.insert(stun_addr); port_allocator_ = new cricket::BasicPortAllocator( - network_manager_, stun_addr, talk_base::SocketAddress(), + network_manager_, stun_servers, talk_base::SocketAddress(), talk_base::SocketAddress(), talk_base::SocketAddress()); if (portallocator_flags_ != 0) { diff --git a/talk/p2p/base/p2ptransportchannel_unittest.cc b/talk/p2p/base/p2ptransportchannel_unittest.cc index 498fde9dc..f65f502dc 100644 --- a/talk/p2p/base/p2ptransportchannel_unittest.cc +++ b/talk/p2p/base/p2ptransportchannel_unittest.cc @@ -50,6 +50,7 @@ using cricket::kMinimumStepDelay; using cricket::kDefaultStepDelay; using cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; using cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET; +using cricket::ServerAddresses; using talk_base::SocketAddress; static const int kDefaultTimeout = 1000; @@ -152,12 +153,14 @@ class P2PTransportChannelTestBase : public testing::Test, ep1_.role_ = cricket::ICEROLE_CONTROLLING; ep2_.role_ = cricket::ICEROLE_CONTROLLED; + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); ep1_.allocator_.reset(new cricket::BasicPortAllocator( - &ep1_.network_manager_, kStunAddr, kRelayUdpIntAddr, - kRelayTcpIntAddr, kRelaySslTcpIntAddr)); + &ep1_.network_manager_, + stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); ep2_.allocator_.reset(new cricket::BasicPortAllocator( - &ep2_.network_manager_, kStunAddr, kRelayUdpIntAddr, - kRelayTcpIntAddr, kRelaySslTcpIntAddr)); + &ep2_.network_manager_, + stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); } protected: @@ -806,13 +809,17 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { // Ideally we want to use TURN server for both GICE and ICE, but in case // of GICE, TURN server usage is not producing results reliabally. // TODO(mallinath): Remove Relay and use TURN server for all tests. + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); GetEndpoint(0)->allocator_.reset( new cricket::BasicPortAllocator(&(GetEndpoint(0)->network_manager_), - kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(), + stun_servers, + talk_base::SocketAddress(), talk_base::SocketAddress(), talk_base::SocketAddress())); GetEndpoint(1)->allocator_.reset( new cricket::BasicPortAllocator(&(GetEndpoint(1)->network_manager_), - kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(), + stun_servers, + talk_base::SocketAddress(), talk_base::SocketAddress(), talk_base::SocketAddress())); cricket::RelayServerConfig relay_server(cricket::RELAY_GTURN); diff --git a/talk/p2p/base/port.h b/talk/p2p/base/port.h index 39d4e25b9..96132645d 100644 --- a/talk/p2p/base/port.h +++ b/talk/p2p/base/port.h @@ -31,6 +31,7 @@ #include #include #include +#include #include "talk/base/asyncpacketsocket.h" #include "talk/base/network.h" @@ -109,6 +110,8 @@ struct ProtocolAddress { : address(a), proto(p), secure(sec) { } }; +typedef std::set ServerAddresses; + // Represents a local communication mechanism that can be used to create // connections to similar mechanisms of the other client. Subclasses of this // one add support for specific mechanisms like local UDP ports. diff --git a/talk/p2p/base/port_unittest.cc b/talk/p2p/base/port_unittest.cc index dcddc46f9..f4f9935ff 100644 --- a/talk/p2p/base/port_unittest.cc +++ b/talk/p2p/base/port_unittest.cc @@ -453,9 +453,11 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { } StunPort* CreateStunPort(const SocketAddress& addr, talk_base::PacketSocketFactory* factory) { + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); StunPort* port = StunPort::Create(main_, factory, &network_, addr.ipaddr(), 0, 0, - username_, password_, kStunAddr); + username_, password_, stun_servers); port->SetIceProtocolType(ice_protocol_); return port; } diff --git a/talk/p2p/base/stunport.cc b/talk/p2p/base/stunport.cc index 6e18fc505..0a051cbba 100644 --- a/talk/p2p/base/stunport.cc +++ b/talk/p2p/base/stunport.cc @@ -69,10 +69,10 @@ class StunBindingRequest : public StunRequest { LOG(LS_ERROR) << "Binding address has bad family"; } else { talk_base::SocketAddress addr(addr_attr->ipaddr(), addr_attr->port()); - port_->OnStunBindingRequestSucceeded(addr); + port_->OnStunBindingRequestSucceeded(server_addr_, addr); } - // We will do a keep-alive regardless of whether this request suceeds. + // We will do a keep-alive regardless of whether this request succeeds. // This should have almost no impact on network usage. if (keep_alive_) { port_->requests_.SendDelayed( @@ -92,7 +92,7 @@ class StunBindingRequest : public StunRequest { << " reason='" << attr->reason() << "'"; } - port_->OnStunBindingOrResolveRequestFailed(); + port_->OnStunBindingOrResolveRequestFailed(server_addr_); if (keep_alive_ && (talk_base::TimeSince(start_time_) <= RETRY_TIMEOUT)) { @@ -107,7 +107,7 @@ class StunBindingRequest : public StunRequest { << port_->GetLocalAddress().ToSensitiveString() << " (" << port_->Network()->name() << ")"; - port_->OnStunBindingOrResolveRequestFailed(); + port_->OnStunBindingOrResolveRequestFailed(server_addr_); if (keep_alive_ && (talk_base::TimeSince(start_time_) <= RETRY_TIMEOUT)) { @@ -120,10 +120,60 @@ class StunBindingRequest : public StunRequest { private: UDPPort* port_; bool keep_alive_; - talk_base::SocketAddress server_addr_; + const talk_base::SocketAddress server_addr_; uint32 start_time_; }; +UDPPort::AddressResolver::AddressResolver( + talk_base::PacketSocketFactory* factory) + : socket_factory_(factory) {} + +UDPPort::AddressResolver::~AddressResolver() { + for (ResolverMap::iterator it = resolvers_.begin(); + it != resolvers_.end(); ++it) { + it->second->Destroy(true); + } +} + +void UDPPort::AddressResolver::Resolve( + const talk_base::SocketAddress& address) { + if (resolvers_.find(address) != resolvers_.end()) + return; + + talk_base::AsyncResolverInterface* resolver = + socket_factory_->CreateAsyncResolver(); + resolvers_.insert( + std::pair( + address, resolver)); + + resolver->SignalDone.connect(this, + &UDPPort::AddressResolver::OnResolveResult); + + resolver->Start(address); +} + +bool UDPPort::AddressResolver::GetResolvedAddress( + const talk_base::SocketAddress& input, + int family, + talk_base::SocketAddress* output) const { + ResolverMap::const_iterator it = resolvers_.find(input); + if (it == resolvers_.end()) + return false; + + return it->second->GetResolvedAddress(family, output); +} + +void UDPPort::AddressResolver::OnResolveResult( + talk_base::AsyncResolverInterface* resolver) { + for (ResolverMap::iterator it = resolvers_.begin(); + it != resolvers_.end(); ++it) { + if (it->second == resolver) { + SignalDone(it->first, resolver->GetError()); + return; + } + } +} + UDPPort::UDPPort(talk_base::Thread* thread, talk_base::PacketSocketFactory* factory, talk_base::Network* network, @@ -134,7 +184,6 @@ UDPPort::UDPPort(talk_base::Thread* thread, requests_(thread), socket_(socket), error_(0), - resolver_(NULL), ready_(false), stun_keepalive_delay_(KEEPALIVE_DELAY) { } @@ -149,7 +198,6 @@ UDPPort::UDPPort(talk_base::Thread* thread, requests_(thread), socket_(NULL), error_(0), - resolver_(NULL), ready_(false), stun_keepalive_delay_(KEEPALIVE_DELAY) { } @@ -172,9 +220,6 @@ bool UDPPort::Init() { } UDPPort::~UDPPort() { - if (resolver_) { - resolver_->Destroy(true); - } if (!SharedSocket()) delete socket_; } @@ -189,11 +234,11 @@ void UDPPort::PrepareAddress() { void UDPPort::MaybePrepareStunCandidate() { // Sending binding request to the STUN server if address is available to // prepare STUN candidate. - if (!server_addr_.IsNil()) { - SendStunBindingRequest(); + if (!server_addresses_.empty()) { + SendStunBindingRequests(); } else { // Port is done allocating candidates. - SetResult(true); + MaybeSetPortCompleteOrError(); } } @@ -254,12 +299,13 @@ void UDPPort::OnReadPacket( const talk_base::SocketAddress& remote_addr, const talk_base::PacketTime& packet_time) { ASSERT(socket == socket_); + ASSERT(!remote_addr.IsUnresolved()); // Look for a response from the STUN server. // Even if the response doesn't match one of our outstanding requests, we // will eat it because it might be a response to a retransmitted packet, and // we already cleared the request when we got the first response. - if (!server_addr_.IsUnresolved() && remote_addr == server_addr_) { + if (server_addresses_.find(remote_addr) != server_addresses_.end()) { requests_.CheckResponse(data, size); return; } @@ -275,76 +321,114 @@ void UDPPort::OnReadyToSend(talk_base::AsyncPacketSocket* socket) { Port::OnReadyToSend(); } -void UDPPort::SendStunBindingRequest() { +void UDPPort::SendStunBindingRequests() { // We will keep pinging the stun server to make sure our NAT pin-hole stays // open during the call. - // TODO: Support multiple stun servers, or make ResolveStunAddress find a - // server with the correct family, or something similar. ASSERT(requests_.empty()); - if (server_addr_.IsUnresolved()) { - ResolveStunAddress(); + + for (ServerAddresses::const_iterator it = server_addresses_.begin(); + it != server_addresses_.end(); ++it) { + SendStunBindingRequest(*it); + } +} + +void UDPPort::ResolveStunAddress(const talk_base::SocketAddress& stun_addr) { + if (!resolver_) { + resolver_.reset(new AddressResolver(socket_factory())); + resolver_->SignalDone.connect(this, &UDPPort::OnResolveResult); + } + + resolver_->Resolve(stun_addr); +} + +void UDPPort::OnResolveResult(const talk_base::SocketAddress& input, + int error) { + ASSERT(resolver_.get()); + + talk_base::SocketAddress resolved; + if (error != 0 || + !resolver_->GetResolvedAddress(input, ip().family(), &resolved)) { + LOG_J(LS_WARNING, this) << "StunPort: stun host lookup received error " + << error; + OnStunBindingOrResolveRequestFailed(input); + return; + } + + server_addresses_.erase(input); + + if (server_addresses_.find(resolved) == server_addresses_.end()) { + server_addresses_.insert(resolved); + SendStunBindingRequest(resolved); + } +} + +void UDPPort::SendStunBindingRequest( + const talk_base::SocketAddress& stun_addr) { + if (stun_addr.IsUnresolved()) { + ResolveStunAddress(stun_addr); + } else if (socket_->GetState() == talk_base::AsyncPacketSocket::STATE_BOUND) { // Check if |server_addr_| is compatible with the port's ip. - if (IsCompatibleAddress(server_addr_)) { - requests_.Send(new StunBindingRequest(this, true, server_addr_)); + if (IsCompatibleAddress(stun_addr)) { + requests_.Send(new StunBindingRequest(this, true, stun_addr)); } else { // Since we can't send stun messages to the server, we should mark this // port ready. - OnStunBindingOrResolveRequestFailed(); + LOG(LS_WARNING) << "STUN server address is incompatible."; + OnStunBindingOrResolveRequestFailed(stun_addr); } } } -void UDPPort::ResolveStunAddress() { - if (resolver_) - return; - - resolver_ = socket_factory()->CreateAsyncResolver(); - resolver_->SignalDone.connect(this, &UDPPort::OnResolveResult); - resolver_->Start(server_addr_); -} - -void UDPPort::OnResolveResult(talk_base::AsyncResolverInterface* resolver) { - ASSERT(resolver == resolver_); - if (resolver_->GetError() != 0 || - !resolver_->GetResolvedAddress(ip().family(), &server_addr_)) { - LOG_J(LS_WARNING, this) << "StunPort: stun host lookup received error " - << resolver_->GetError(); - OnStunBindingOrResolveRequestFailed(); +void UDPPort::OnStunBindingRequestSucceeded( + const talk_base::SocketAddress& stun_server_addr, + const talk_base::SocketAddress& stun_reflected_addr) { + if (bind_request_succeeded_servers_.find(stun_server_addr) != + bind_request_succeeded_servers_.end()) { return; } + bind_request_succeeded_servers_.insert(stun_server_addr); - SendStunBindingRequest(); -} - -void UDPPort::OnStunBindingRequestSucceeded( - const talk_base::SocketAddress& stun_addr) { - if (ready_) // Discarding the binding response if port is already enabled. - return; - - if (!SharedSocket() || stun_addr != socket_->GetLocalAddress()) { - // If socket is shared and |stun_addr| is equal to local socket + if (!SharedSocket() || stun_reflected_addr != socket_->GetLocalAddress()) { + // If socket is shared and |stun_reflected_addr| is equal to local socket // address then discarding the stun address. // For STUN related address is local socket address. - AddAddress(stun_addr, socket_->GetLocalAddress(), + AddAddress(stun_reflected_addr, socket_->GetLocalAddress(), socket_->GetLocalAddress(), UDP_PROTOCOL_NAME, STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, false); } - SetResult(true); + MaybeSetPortCompleteOrError(); } -void UDPPort::OnStunBindingOrResolveRequestFailed() { - if (ready_) // Discarding failure response if port is already enabled. +void UDPPort::OnStunBindingOrResolveRequestFailed( + const talk_base::SocketAddress& stun_server_addr) { + if (bind_request_failed_servers_.find(stun_server_addr) != + bind_request_failed_servers_.end()) { + return; + } + bind_request_failed_servers_.insert(stun_server_addr); + MaybeSetPortCompleteOrError(); +} + +void UDPPort::MaybeSetPortCompleteOrError() { + if (ready_) return; - // If socket is shared, we should process local udp candidate. - SetResult(SharedSocket()); -} + // Do not set port ready if we are still waiting for bind responses. + const size_t servers_done_bind_request = bind_request_failed_servers_.size() + + bind_request_succeeded_servers_.size(); + if (server_addresses_.size() != servers_done_bind_request) { + return; + } -void UDPPort::SetResult(bool success) { // Setting ready status. ready_ = true; - if (success) { + + // The port is "completed" if there is no stun server provided, or the bind + // request succeeded for any stun server, or the socket is shared. + if (server_addresses_.empty() || + bind_request_succeeded_servers_.size() > 0 || + SharedSocket()) { SignalPortComplete(this); } else { SignalPortError(this); diff --git a/talk/p2p/base/stunport.h b/talk/p2p/base/stunport.h index c45d6af33..367db22cf 100644 --- a/talk/p2p/base/stunport.h +++ b/talk/p2p/base/stunport.h @@ -82,9 +82,12 @@ class UDPPort : public Port { return socket_->GetLocalAddress(); } - const talk_base::SocketAddress& server_addr() const { return server_addr_; } - void set_server_addr(const talk_base::SocketAddress& addr) { - server_addr_ = addr; + const ServerAddresses server_addresses() const { + return server_addresses_; + } + void + set_server_addresses(const ServerAddresses& addresses) { + server_addresses_ = addresses; } virtual void PrepareAddress(); @@ -140,29 +143,64 @@ class UDPPort : public Port { // This method will send STUN binding request if STUN server address is set. void MaybePrepareStunCandidate(); - void SendStunBindingRequest(); + void SendStunBindingRequests(); private: + // A helper class which can be called repeatedly to resolve multiple + // addresses, as opposed to talk_base::AsyncResolverInterface, which can only + // resolve one address per instance. + class AddressResolver : public sigslot::has_slots<> { + public: + explicit AddressResolver(talk_base::PacketSocketFactory* factory); + ~AddressResolver(); + + void Resolve(const talk_base::SocketAddress& address); + bool GetResolvedAddress(const talk_base::SocketAddress& input, + int family, + talk_base::SocketAddress* output) const; + + // The signal is sent when resolving the specified address is finished. The + // first argument is the input address, the second argument is the error + // or 0 if it succeeded. + sigslot::signal2 SignalDone; + + private: + typedef std::map ResolverMap; + + void OnResolveResult(talk_base::AsyncResolverInterface* resolver); + + talk_base::PacketSocketFactory* socket_factory_; + ResolverMap resolvers_; + }; + // DNS resolution of the STUN server. - void ResolveStunAddress(); - void OnResolveResult(talk_base::AsyncResolverInterface* resolver); + void ResolveStunAddress(const talk_base::SocketAddress& stun_addr); + void OnResolveResult(const talk_base::SocketAddress& input, int error); + + void SendStunBindingRequest(const talk_base::SocketAddress& stun_addr); // Below methods handles binding request responses. - void OnStunBindingRequestSucceeded(const talk_base::SocketAddress& stun_addr); - void OnStunBindingOrResolveRequestFailed(); + void OnStunBindingRequestSucceeded( + const talk_base::SocketAddress& stun_server_addr, + const talk_base::SocketAddress& stun_reflected_addr); + void OnStunBindingOrResolveRequestFailed( + const talk_base::SocketAddress& stun_server_addr); // Sends STUN requests to the server. void OnSendPacket(const void* data, size_t size, StunRequest* req); // TODO(mallinaht) - Move this up to cricket::Port when SignalAddressReady is // changed to SignalPortReady. - void SetResult(bool success); + void MaybeSetPortCompleteOrError(); - talk_base::SocketAddress server_addr_; + ServerAddresses server_addresses_; + ServerAddresses bind_request_succeeded_servers_; + ServerAddresses bind_request_failed_servers_; StunRequestManager requests_; talk_base::AsyncPacketSocket* socket_; int error_; - talk_base::AsyncResolverInterface* resolver_; + talk_base::scoped_ptr resolver_; bool ready_; int stun_keepalive_delay_; @@ -171,17 +209,18 @@ class UDPPort : public Port { class StunPort : public UDPPort { public: - static StunPort* Create(talk_base::Thread* thread, - talk_base::PacketSocketFactory* factory, - talk_base::Network* network, - const talk_base::IPAddress& ip, - int min_port, int max_port, - const std::string& username, - const std::string& password, - const talk_base::SocketAddress& server_addr) { + static StunPort* Create( + talk_base::Thread* thread, + talk_base::PacketSocketFactory* factory, + talk_base::Network* network, + const talk_base::IPAddress& ip, + int min_port, int max_port, + const std::string& username, + const std::string& password, + const ServerAddresses& servers) { StunPort* port = new StunPort(thread, factory, network, ip, min_port, max_port, - username, password, server_addr); + username, password, servers); if (!port->Init()) { delete port; port = NULL; @@ -192,7 +231,7 @@ class StunPort : public UDPPort { virtual ~StunPort() {} virtual void PrepareAddress() { - SendStunBindingRequest(); + SendStunBindingRequests(); } protected: @@ -200,12 +239,12 @@ class StunPort : public UDPPort { talk_base::Network* network, const talk_base::IPAddress& ip, int min_port, int max_port, const std::string& username, const std::string& password, - const talk_base::SocketAddress& server_address) + const ServerAddresses& servers) : UDPPort(thread, factory, network, ip, min_port, max_port, username, password) { // UDPPort will set these to local udp, updating these to STUN. set_type(STUN_PORT_TYPE); - set_server_addr(server_address); + set_server_addresses(servers); } }; diff --git a/talk/p2p/base/stunport_unittest.cc b/talk/p2p/base/stunport_unittest.cc index 5850027ec..0965712f0 100644 --- a/talk/p2p/base/stunport_unittest.cc +++ b/talk/p2p/base/stunport_unittest.cc @@ -30,15 +30,18 @@ #include "talk/base/physicalsocketserver.h" #include "talk/base/scoped_ptr.h" #include "talk/base/socketaddress.h" +#include "talk/base/ssladapter.h" #include "talk/base/virtualsocketserver.h" #include "talk/p2p/base/basicpacketsocketfactory.h" #include "talk/p2p/base/stunport.h" #include "talk/p2p/base/teststunserver.h" +using cricket::ServerAddresses; using talk_base::SocketAddress; static const SocketAddress kLocalAddr("127.0.0.1", 0); -static const SocketAddress kStunAddr("127.0.0.1", 5000); +static const SocketAddress kStunAddr1("127.0.0.1", 5000); +static const SocketAddress kStunAddr2("127.0.0.1", 4000); static const SocketAddress kBadAddr("0.0.0.1", 5000); static const SocketAddress kStunHostnameAddr("localhost", 5000); static const SocketAddress kBadHostnameAddr("not-a-real-hostname", 5000); @@ -58,8 +61,10 @@ class StunPortTest : public testing::Test, ss_scope_(ss_.get()), network_("unittest", "unittest", talk_base::IPAddress(INADDR_ANY), 32), socket_factory_(talk_base::Thread::Current()), - stun_server_(new cricket::TestStunServer( - talk_base::Thread::Current(), kStunAddr)), + stun_server_1_(new cricket::TestStunServer( + talk_base::Thread::Current(), kStunAddr1)), + stun_server_2_(new cricket::TestStunServer( + talk_base::Thread::Current(), kStunAddr2)), done_(false), error_(false), stun_keepalive_delay_(0) { } @@ -68,10 +73,16 @@ class StunPortTest : public testing::Test, bool error() const { return error_; } void CreateStunPort(const talk_base::SocketAddress& server_addr) { + ServerAddresses stun_servers; + stun_servers.insert(server_addr); + CreateStunPort(stun_servers); + } + + void CreateStunPort(const ServerAddresses& stun_servers) { stun_port_.reset(cricket::StunPort::Create( talk_base::Thread::Current(), &socket_factory_, &network_, kLocalAddr.ipaddr(), 0, 0, talk_base::CreateRandomString(16), - talk_base::CreateRandomString(22), server_addr)); + talk_base::CreateRandomString(22), stun_servers)); stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_); stun_port_->SignalPortComplete.connect(this, &StunPortTest::OnPortComplete); @@ -89,7 +100,9 @@ class StunPortTest : public testing::Test, &network_, socket_.get(), talk_base::CreateRandomString(16), talk_base::CreateRandomString(22))); ASSERT_TRUE(stun_port_ != NULL); - stun_port_->set_server_addr(server_addr); + ServerAddresses stun_servers; + stun_servers.insert(server_addr); + stun_port_->set_server_addresses(stun_servers); stun_port_->SignalPortComplete.connect(this, &StunPortTest::OnPortComplete); stun_port_->SignalPortError.connect(this, @@ -115,11 +128,17 @@ class StunPortTest : public testing::Test, protected: static void SetUpTestCase() { + talk_base::InitializeSSL(); // Ensure the RNG is inited. talk_base::InitRandom(NULL, 0); + + } + static void TearDownTestCase() { + talk_base::CleanupSSL(); } void OnPortComplete(cricket::Port* port) { + ASSERT_FALSE(done_); done_ = true; error_ = false; } @@ -138,7 +157,8 @@ class StunPortTest : public testing::Test, talk_base::Network network_; talk_base::BasicPacketSocketFactory socket_factory_; talk_base::scoped_ptr stun_port_; - talk_base::scoped_ptr stun_server_; + talk_base::scoped_ptr stun_server_1_; + talk_base::scoped_ptr stun_server_2_; talk_base::scoped_ptr socket_; bool done_; bool error_; @@ -147,14 +167,14 @@ class StunPortTest : public testing::Test, // Test that we can create a STUN port TEST_F(StunPortTest, TestBasic) { - CreateStunPort(kStunAddr); + CreateStunPort(kStunAddr1); EXPECT_EQ("stun", port()->Type()); EXPECT_EQ(0U, port()->Candidates().size()); } // Test that we can get an address from a STUN server. TEST_F(StunPortTest, TestPrepareAddress) { - CreateStunPort(kStunAddr); + CreateStunPort(kStunAddr1); PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); ASSERT_EQ(1U, port()->Candidates().size()); @@ -209,7 +229,7 @@ TEST_F(StunPortTest, TestKeepAliveResponse) { // Test that a local candidate can be generated using a shared socket. TEST_F(StunPortTest, TestSharedSocketPrepareAddress) { - CreateSharedStunPort(kStunAddr); + CreateSharedStunPort(kStunAddr1); PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); ASSERT_EQ(1U, port()->Candidates().size()); @@ -232,3 +252,28 @@ TEST_F(StunPortTest, TestSharedSocketPrepareAddressInvalidHostname) { SendData(data.c_str(), data.length()); // No crash is success. } + +// Test that candidates can be allocated for multiple STUN servers. +TEST_F(StunPortTest, TestMultipleGoodStunServers) { + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr1); + stun_servers.insert(kStunAddr2); + CreateStunPort(stun_servers); + EXPECT_EQ("stun", port()->Type()); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + EXPECT_EQ(2U, port()->Candidates().size()); +} + +// Test that candidates can be allocated for multiple STUN servers, one of which +// is not reachable. +TEST_F(StunPortTest, TestMultipleStunServersWithBadServer) { + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr1); + stun_servers.insert(kBadAddr); + CreateStunPort(stun_servers); + EXPECT_EQ("stun", port()->Type()); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + EXPECT_EQ(1U, port()->Candidates().size()); +} diff --git a/talk/p2p/client/basicportallocator.cc b/talk/p2p/client/basicportallocator.cc index aa1651749..46fbf4994 100644 --- a/talk/p2p/client/basicportallocator.cc +++ b/talk/p2p/client/basicportallocator.cc @@ -186,23 +186,23 @@ BasicPortAllocator::BasicPortAllocator( BasicPortAllocator::BasicPortAllocator( talk_base::NetworkManager* network_manager, talk_base::PacketSocketFactory* socket_factory, - const talk_base::SocketAddress& stun_address) + const ServerAddresses& stun_servers) : network_manager_(network_manager), socket_factory_(socket_factory), - stun_address_(stun_address) { + stun_servers_(stun_servers) { ASSERT(socket_factory_ != NULL); Construct(); } BasicPortAllocator::BasicPortAllocator( talk_base::NetworkManager* network_manager, - const talk_base::SocketAddress& stun_address, + const ServerAddresses& stun_servers, const talk_base::SocketAddress& relay_address_udp, const talk_base::SocketAddress& relay_address_tcp, const talk_base::SocketAddress& relay_address_ssl) : network_manager_(network_manager), socket_factory_(NULL), - stun_address_(stun_address) { + stun_servers_(stun_servers) { RelayServerConfig config(RELAY_GTURN); if (!relay_address_udp.IsNil()) @@ -333,7 +333,7 @@ void BasicPortAllocatorSession::OnMessage(talk_base::Message *message) { } void BasicPortAllocatorSession::GetPortConfigurations() { - PortConfiguration* config = new PortConfiguration(allocator_->stun_address(), + PortConfiguration* config = new PortConfiguration(allocator_->stun_servers(), username(), password()); @@ -422,7 +422,7 @@ void BasicPortAllocatorSession::DoAllocate() { } // Disables phases that are not specified in this config. - if (!config || config->stun_address.IsNil()) { + if (!config || config->StunServers().empty()) { // No STUN ports specified in this config. sequence_flags |= PORTALLOCATOR_DISABLE_STUN; } @@ -753,8 +753,8 @@ void AllocationSequence::DisableEquivalentPhases(talk_base::Network* network, *flags |= PORTALLOCATOR_DISABLE_TCP; if (config_ && config) { - if (config_->stun_address == config->stun_address) { - // Already got this STUN server covered. + if (config_->StunServers() == config->StunServers()) { + // Already got this STUN servers covered. *flags |= PORTALLOCATOR_DISABLE_STUN; } if (!config_->relays.empty()) { @@ -878,15 +878,15 @@ void AllocationSequence::CreateUDPPorts() { // If STUN is not disabled, setting stun server address to port. if (!IsFlagSet(PORTALLOCATOR_DISABLE_STUN)) { - // If config has stun_address, use it to get server reflexive candidate + // If config has stun_servers, use it to get server reflexive candidate // otherwise use first TURN server which supports UDP. - if (config_ && !config_->stun_address.IsNil()) { + if (config_ && !config_->StunServers().empty()) { LOG(LS_INFO) << "AllocationSequence: UDPPort will be handling the " << "STUN candidate generation."; - port->set_server_addr(config_->stun_address); + port->set_server_addresses(config_->StunServers()); } else if (config_ && config_->SupportsProtocol(RELAY_TURN, PROTO_UDP)) { - port->set_server_addr(config_->GetFirstRelayServerAddress( + port->set_server_addresses(config_->GetRelayServerAddresses( RELAY_TURN, PROTO_UDP)); LOG(LS_INFO) << "AllocationSequence: TURN Server address will be " << " used for generating STUN candidate."; @@ -931,8 +931,8 @@ void AllocationSequence::CreateStunPorts() { // If BasicPortAllocatorSession::OnAllocate left STUN ports enabled then we // ought to have an address for them here. - ASSERT(config_ && !config_->stun_address.IsNil()); - if (!(config_ && !config_->stun_address.IsNil())) { + ASSERT(config_ && !config_->StunServers().empty()); + if (!(config_ && !config_->StunServers().empty())) { LOG(LS_WARNING) << "AllocationSequence: No STUN server configured, skipping."; return; @@ -944,7 +944,7 @@ void AllocationSequence::CreateStunPorts() { session_->allocator()->min_port(), session_->allocator()->max_port(), session_->username(), session_->password(), - config_->stun_address); + config_->StunServers()); if (port) { session_->AddAllocatedPort(port, this, true); // Since StunPort is not created using shared socket, |port| will not be @@ -1115,9 +1115,27 @@ PortConfiguration::PortConfiguration( const talk_base::SocketAddress& stun_address, const std::string& username, const std::string& password) - : stun_address(stun_address), + : stun_address(stun_address), username(username), password(password) { + if (!stun_address.IsNil()) + stun_servers.insert(stun_address); +} + +PortConfiguration::PortConfiguration(const ServerAddresses& stun_servers, + const std::string& username, + const std::string& password) + : stun_servers(stun_servers), username(username), password(password) { + if (!stun_servers.empty()) + stun_address = *(stun_servers.begin()); +} + +ServerAddresses PortConfiguration::StunServers() { + if (!stun_address.IsNil() && + stun_servers.find(stun_address) == stun_servers.end()) { + stun_servers.insert(stun_address); + } + return stun_servers; } void PortConfiguration::AddRelay(const RelayServerConfig& config) { @@ -1146,14 +1164,15 @@ bool PortConfiguration::SupportsProtocol(RelayType turn_type, return false; } -talk_base::SocketAddress PortConfiguration::GetFirstRelayServerAddress( +ServerAddresses PortConfiguration::GetRelayServerAddresses( RelayType turn_type, ProtocolType type) const { + ServerAddresses servers; for (size_t i = 0; i < relays.size(); ++i) { if (relays[i].type == turn_type && SupportsProtocol(relays[i], type)) { - return relays[i].ports.front().address; + servers.insert(relays[i].ports.front().address); } } - return talk_base::SocketAddress(); + return servers; } } // namespace cricket diff --git a/talk/p2p/client/basicportallocator.h b/talk/p2p/client/basicportallocator.h index 5c43b42c2..aee6135ef 100644 --- a/talk/p2p/client/basicportallocator.h +++ b/talk/p2p/client/basicportallocator.h @@ -69,9 +69,9 @@ class BasicPortAllocator : public PortAllocator { explicit BasicPortAllocator(talk_base::NetworkManager* network_manager); BasicPortAllocator(talk_base::NetworkManager* network_manager, talk_base::PacketSocketFactory* socket_factory, - const talk_base::SocketAddress& stun_server); + const ServerAddresses& stun_servers); BasicPortAllocator(talk_base::NetworkManager* network_manager, - const talk_base::SocketAddress& stun_server, + const ServerAddresses& stun_servers, const talk_base::SocketAddress& relay_server_udp, const talk_base::SocketAddress& relay_server_tcp, const talk_base::SocketAddress& relay_server_ssl); @@ -83,8 +83,8 @@ class BasicPortAllocator : public PortAllocator { // creates its own socket factory. talk_base::PacketSocketFactory* socket_factory() { return socket_factory_; } - const talk_base::SocketAddress& stun_address() const { - return stun_address_; + const ServerAddresses& stun_servers() const { + return stun_servers_; } const std::vector& relays() const { @@ -105,7 +105,7 @@ class BasicPortAllocator : public PortAllocator { talk_base::NetworkManager* network_manager_; talk_base::PacketSocketFactory* socket_factory_; - const talk_base::SocketAddress stun_address_; + const ServerAddresses stun_servers_; std::vector relays_; bool allow_tcp_listen_; }; @@ -218,17 +218,27 @@ class BasicPortAllocatorSession : public PortAllocatorSession, // Records configuration information useful in creating ports. struct PortConfiguration : public talk_base::MessageData { + // TODO(jiayl): remove |stun_address| when Chrome is updated. talk_base::SocketAddress stun_address; + ServerAddresses stun_servers; std::string username; std::string password; typedef std::vector RelayList; RelayList relays; + // TODO(jiayl): remove this ctor when Chrome is updated. PortConfiguration(const talk_base::SocketAddress& stun_address, const std::string& username, const std::string& password); + PortConfiguration(const ServerAddresses& stun_servers, + const std::string& username, + const std::string& password); + + // TODO(jiayl): remove when |stun_address| is removed. + ServerAddresses StunServers(); + // Adds another relay server, with the given ports and modifier, to the list. void AddRelay(const RelayServerConfig& config); @@ -236,9 +246,9 @@ struct PortConfiguration : public talk_base::MessageData { bool SupportsProtocol(const RelayServerConfig& relay, ProtocolType type) const; bool SupportsProtocol(RelayType turn_type, ProtocolType type) const; - // Helper method returns the first server address for the matching - // RelayType and Protocol type. - talk_base::SocketAddress GetFirstRelayServerAddress( + // Helper method returns the server addresses for the matching RelayType and + // Protocol type. + ServerAddresses GetRelayServerAddresses( RelayType turn_type, ProtocolType type) const; }; diff --git a/talk/p2p/client/connectivitychecker.cc b/talk/p2p/client/connectivitychecker.cc index 1b599430e..facb01e03 100644 --- a/talk/p2p/client/connectivitychecker.cc +++ b/talk/p2p/client/connectivitychecker.cc @@ -288,7 +288,9 @@ void ConnectivityChecker::OnStunPortComplete(Port* port) { uint32 now = talk_base::Time(); NicInfo* nic_info = &i->second; nic_info->external_address = c.address(); - nic_info->stun_server_address = static_cast(port)->server_addr(); + + nic_info->stun_server_addresses = + static_cast(port)->server_addresses(); nic_info->stun.rtt = now - nic_info->stun.start_time_ms; } else { LOG(LS_ERROR) << "Got stun address for non-existing nic"; @@ -303,7 +305,9 @@ void ConnectivityChecker::OnStunPortError(Port* port) { if (i != nics_.end()) { // We have it already, add the new information. NicInfo* nic_info = &i->second; - nic_info->stun_server_address = static_cast(port)->server_addr(); + + nic_info->stun_server_addresses = + static_cast(port)->server_addresses(); } } @@ -335,7 +339,7 @@ StunPort* ConnectivityChecker::CreateStunPort( const PortConfiguration* config, talk_base::Network* network) { return StunPort::Create(worker_, socket_factory_.get(), network, network->ip(), 0, 0, - username, password, config->stun_address); + username, password, config->stun_servers); } RelayPort* ConnectivityChecker::CreateRelayPort( @@ -407,7 +411,9 @@ void ConnectivityChecker::CreateRelayPorts( void ConnectivityChecker::AllocatePorts() { const std::string username = talk_base::CreateRandomString(ICE_UFRAG_LENGTH); const std::string password = talk_base::CreateRandomString(ICE_PWD_LENGTH); - PortConfiguration config(stun_address_, username, password); + ServerAddresses stun_servers; + stun_servers.insert(stun_address_); + PortConfiguration config(stun_servers, username, password); std::vector networks; network_manager_->GetNetworks(&networks); if (networks.empty()) { diff --git a/talk/p2p/client/connectivitychecker.h b/talk/p2p/client/connectivitychecker.h index 95b736d09..3f10c5762 100644 --- a/talk/p2p/client/connectivitychecker.h +++ b/talk/p2p/client/connectivitychecker.h @@ -96,7 +96,7 @@ struct NicInfo { talk_base::IPAddress ip; talk_base::ProxyInfo proxy_info; talk_base::SocketAddress external_address; - talk_base::SocketAddress stun_server_address; + ServerAddresses stun_server_addresses; talk_base::SocketAddress media_server_address; ConnectInfo stun; ConnectInfo http; diff --git a/talk/p2p/client/connectivitychecker_unittest.cc b/talk/p2p/client/connectivitychecker_unittest.cc index c62120bee..8d6fa9df7 100644 --- a/talk/p2p/client/connectivitychecker_unittest.cc +++ b/talk/p2p/client/connectivitychecker_unittest.cc @@ -66,7 +66,7 @@ class FakeStunPort : public StunPort { const talk_base::IPAddress& ip, int min_port, int max_port, const std::string& username, const std::string& password, - const talk_base::SocketAddress& server_addr) + const ServerAddresses& server_addr) : StunPort(thread, factory, network, ip, min_port, max_port, username, password, server_addr) { } @@ -215,7 +215,7 @@ class ConnectivityCheckerForTest : public ConnectivityChecker { network, network->ip(), kMinPort, kMaxPort, username, password, - config->stun_address); + config->stun_servers); } virtual RelayPort* CreateRelayPort( const std::string& username, const std::string& password, @@ -254,7 +254,8 @@ class ConnectivityCheckerTest : public testing::Test { EXPECT_EQ(kExternalAddr, info.external_address); // Verify that the stun server address has been set. - EXPECT_EQ(kStunAddr, info.stun_server_address); + EXPECT_EQ(1U, info.stun_server_addresses.size()); + EXPECT_EQ(kStunAddr, *(info.stun_server_addresses.begin())); // Verify that the media server address has been set. Don't care // about port since it is different for different protocols. diff --git a/talk/p2p/client/httpportallocator.cc b/talk/p2p/client/httpportallocator.cc index b881d439f..1529770c7 100644 --- a/talk/p2p/client/httpportallocator.cc +++ b/talk/p2p/client/httpportallocator.cc @@ -142,7 +142,13 @@ void HttpPortAllocatorSessionBase::GetPortConfigurations() { // but for now is done here and added to the initial config. Note any later // configs will have unresolved stun ips and will be discarded by the // AllocationSequence. - PortConfiguration* config = new PortConfiguration(stun_hosts_[0], + ServerAddresses hosts; + for (std::vector::iterator it = stun_hosts_.begin(); + it != stun_hosts_.end(); ++it) { + hosts.insert(*it); + } + + PortConfiguration* config = new PortConfiguration(hosts, username(), password()); ConfigReady(config); @@ -206,7 +212,13 @@ void HttpPortAllocatorSessionBase::ReceiveSessionResponse( std::string relay_tcp_port = map["relay.tcp_port"]; std::string relay_ssltcp_port = map["relay.ssltcp_port"]; - PortConfiguration* config = new PortConfiguration(stun_hosts_[0], + ServerAddresses hosts; + for (std::vector::iterator it = stun_hosts_.begin(); + it != stun_hosts_.end(); ++it) { + hosts.insert(*it); + } + + PortConfiguration* config = new PortConfiguration(hosts, map["username"], map["password"]); diff --git a/talk/p2p/client/portallocator_unittest.cc b/talk/p2p/client/portallocator_unittest.cc index b9def3041..760d16820 100644 --- a/talk/p2p/client/portallocator_unittest.cc +++ b/talk/p2p/client/portallocator_unittest.cc @@ -48,6 +48,7 @@ #include "talk/p2p/client/basicportallocator.h" #include "talk/p2p/client/httpportallocator.h" +using cricket::ServerAddresses; using talk_base::SocketAddress; using talk_base::Thread; @@ -115,10 +116,13 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { kRelayTcpIntAddr, kRelayTcpExtAddr, kRelaySslTcpIntAddr, kRelaySslTcpExtAddr), turn_server_(Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr), - allocator_(new cricket::BasicPortAllocator( - &network_manager_, kStunAddr, - kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)), candidate_allocation_done_(false) { + cricket::ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); + allocator_.reset(new cricket::BasicPortAllocator( + &network_manager_, + stun_servers, + kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); allocator_->set_step_delay(cricket::kMinimumStepDelay); } @@ -265,7 +269,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { // Tests that we can init the port allocator and create a session. TEST_F(PortAllocatorTest, TestBasic) { EXPECT_EQ(&network_manager_, allocator().network_manager()); - EXPECT_EQ(kStunAddr, allocator().stun_address()); + EXPECT_EQ(kStunAddr, *allocator().stun_servers().begin()); ASSERT_EQ(1u, allocator().relays().size()); EXPECT_EQ(cricket::RELAY_GTURN, allocator().relays()[0].type); // Empty relay credentials are used for GTURN. @@ -696,8 +700,10 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNat) { AddInterface(kClientAddr); talk_base::scoped_ptr nat_server( CreateNatServer(kNatAddr, talk_base::NAT_OPEN_CONE)); + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, &nat_socket_factory_, kStunAddr)); + &network_manager_, &nat_socket_factory_, stun_servers)); allocator_->set_step_delay(cricket::kMinimumStepDelay); allocator_->set_flags(allocator().flags() | cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | @@ -786,8 +792,10 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurn) { AddInterface(kClientAddr); talk_base::scoped_ptr nat_server( CreateNatServer(kNatAddr, talk_base::NAT_OPEN_CONE)); + ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, &nat_socket_factory_, kStunAddr)); + &network_manager_, &nat_socket_factory_, stun_servers)); cricket::RelayServerConfig relay_server(cricket::RELAY_TURN); cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword); relay_server.credentials = credentials; @@ -895,6 +903,7 @@ TEST(HttpPortAllocatorTest, TestHttpPortAllocatorHostLists) { talk_base::SocketAddress("1.unittest.corp.google.com", 0)); stun_servers.push_back( talk_base::SocketAddress("2.unittest.corp.google.com", 0)); + alloc.SetRelayHosts(relay_servers); alloc.SetStunHosts(stun_servers); EXPECT_EQ(2U, alloc.relay_hosts().size());