From f421bdc68dbf8ab456d36909185b6b2a8935bc64 Mon Sep 17 00:00:00 2001 From: honghaiz Date: Fri, 17 Jul 2015 16:21:55 -0700 Subject: [PATCH] Fix an NPE when creating TurnPort with a NULL socket. BUG=4827 Review URL: https://codereview.webrtc.org/1241943002 Cr-Commit-Position: refs/heads/master@{#9601} --- webrtc/p2p/client/basicportallocator.cc | 91 +-------------------- webrtc/p2p/client/basicportallocator.h | 87 ++++++++++++++++++++ webrtc/p2p/client/portallocator_unittest.cc | 32 ++++++++ 3 files changed, 122 insertions(+), 88 deletions(-) diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 1965ad94f7..3f5aa0a1ff 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -58,94 +58,9 @@ int ShakeDelay() { } // namespace namespace cricket { - const uint32 DISABLE_ALL_PHASES = - PORTALLOCATOR_DISABLE_UDP - | PORTALLOCATOR_DISABLE_TCP - | PORTALLOCATOR_DISABLE_STUN - | PORTALLOCATOR_DISABLE_RELAY; - -// Performs the allocation of ports, in a sequenced (timed) manner, for a given -// network and IP address. -class AllocationSequence : public rtc::MessageHandler, - public sigslot::has_slots<> { - public: - enum State { - kInit, // Initial state. - kRunning, // Started allocating ports. - kStopped, // Stopped from running. - kCompleted, // All ports are allocated. - - // kInit --> kRunning --> {kCompleted|kStopped} - }; - - AllocationSequence(BasicPortAllocatorSession* session, - rtc::Network* network, - PortConfiguration* config, - uint32 flags); - ~AllocationSequence(); - bool Init(); - void Clear(); - - State state() const { return state_; } - - // Disables the phases for a new sequence that this one already covers for an - // equivalent network setup. - void DisableEquivalentPhases(rtc::Network* network, - PortConfiguration* config, uint32* flags); - - // Starts and stops the sequence. When started, it will continue allocating - // new ports on its own timed schedule. - void Start(); - void Stop(); - - // MessageHandler - void OnMessage(rtc::Message* msg); - - void EnableProtocol(ProtocolType proto); - bool ProtocolEnabled(ProtocolType proto) const; - - // Signal from AllocationSequence, when it's done with allocating ports. - // This signal is useful, when port allocation fails which doesn't result - // in any candidates. Using this signal BasicPortAllocatorSession can send - // its candidate discovery conclusion signal. Without this signal, - // BasicPortAllocatorSession doesn't have any event to trigger signal. This - // can also be achieved by starting timer in BPAS. - sigslot::signal1 SignalPortAllocationComplete; - - private: - typedef std::vector ProtocolList; - - bool IsFlagSet(uint32 flag) { - return ((flags_ & flag) != 0); - } - void CreateUDPPorts(); - void CreateTCPPorts(); - void CreateStunPorts(); - void CreateRelayPorts(); - void CreateGturnPort(const RelayServerConfig& config); - void CreateTurnPort(const RelayServerConfig& config); - - void OnReadPacket(rtc::AsyncPacketSocket* socket, - const char* data, size_t size, - const rtc::SocketAddress& remote_addr, - const rtc::PacketTime& packet_time); - - void OnPortDestroyed(PortInterface* port); - - BasicPortAllocatorSession* session_; - rtc::Network* network_; - rtc::IPAddress ip_; - PortConfiguration* config_; - State state_; - uint32 flags_; - ProtocolList protocols_; - rtc::scoped_ptr udp_socket_; - // There will be only one udp port per AllocationSequence. - UDPPort* udp_port_; - std::vector turn_ports_; - int phase_; -}; + PORTALLOCATOR_DISABLE_UDP | PORTALLOCATOR_DISABLE_TCP | + PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_RELAY; // BasicPortAllocator BasicPortAllocator::BasicPortAllocator( @@ -1059,7 +974,7 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config) { // TODO(mallinath) - Enable shared socket mode for TURN ports. Disabled // due to webrtc bug https://code.google.com/p/webrtc/issues/detail?id=3537 if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET) && - relay_port->proto == PROTO_UDP) { + relay_port->proto == PROTO_UDP && udp_socket_) { port = TurnPort::Create(session_->network_thread(), session_->socket_factory(), network_, udp_socket_.get(), diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h index f2d92ef64e..a27ff4c627 100644 --- a/webrtc/p2p/client/basicportallocator.h +++ b/webrtc/p2p/client/basicportallocator.h @@ -237,6 +237,93 @@ struct PortConfiguration : public rtc::MessageData { RelayType turn_type, ProtocolType type) const; }; +class UDPPort; +class TurnPort; + +// Performs the allocation of ports, in a sequenced (timed) manner, for a given +// network and IP address. +class AllocationSequence : public rtc::MessageHandler, + public sigslot::has_slots<> { + public: + enum State { + kInit, // Initial state. + kRunning, // Started allocating ports. + kStopped, // Stopped from running. + kCompleted, // All ports are allocated. + + // kInit --> kRunning --> {kCompleted|kStopped} + }; + AllocationSequence(BasicPortAllocatorSession* session, + rtc::Network* network, + PortConfiguration* config, + uint32 flags); + ~AllocationSequence(); + bool Init(); + void Clear(); + + State state() const { return state_; } + + // Disables the phases for a new sequence that this one already covers for an + // equivalent network setup. + void DisableEquivalentPhases(rtc::Network* network, + PortConfiguration* config, + uint32* flags); + + // Starts and stops the sequence. When started, it will continue allocating + // new ports on its own timed schedule. + void Start(); + void Stop(); + + // MessageHandler + void OnMessage(rtc::Message* msg); + + void EnableProtocol(ProtocolType proto); + bool ProtocolEnabled(ProtocolType proto) const; + + // Signal from AllocationSequence, when it's done with allocating ports. + // This signal is useful, when port allocation fails which doesn't result + // in any candidates. Using this signal BasicPortAllocatorSession can send + // its candidate discovery conclusion signal. Without this signal, + // BasicPortAllocatorSession doesn't have any event to trigger signal. This + // can also be achieved by starting timer in BPAS. + sigslot::signal1 SignalPortAllocationComplete; + + protected: + // For testing. + void CreateTurnPort(const RelayServerConfig& config); + + private: + typedef std::vector ProtocolList; + + bool IsFlagSet(uint32 flag) { return ((flags_ & flag) != 0); } + void CreateUDPPorts(); + void CreateTCPPorts(); + void CreateStunPorts(); + void CreateRelayPorts(); + void CreateGturnPort(const RelayServerConfig& config); + + void OnReadPacket(rtc::AsyncPacketSocket* socket, + const char* data, + size_t size, + const rtc::SocketAddress& remote_addr, + const rtc::PacketTime& packet_time); + + void OnPortDestroyed(PortInterface* port); + + BasicPortAllocatorSession* session_; + rtc::Network* network_; + rtc::IPAddress ip_; + PortConfiguration* config_; + State state_; + uint32 flags_; + ProtocolList protocols_; + rtc::scoped_ptr udp_socket_; + // There will be only one udp port per AllocationSequence. + UDPPort* udp_port_; + std::vector turn_ports_; + int phase_; +}; + } // namespace cricket #endif // WEBRTC_P2P_CLIENT_BASICPORTALLOCATOR_H_ diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 6940ea17ad..7dc25140cc 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -20,6 +20,7 @@ #include "webrtc/base/firewallsocketserver.h" #include "webrtc/base/gunit.h" #include "webrtc/base/helpers.h" +#include "webrtc/base/ipaddress.h" #include "webrtc/base/logging.h" #include "webrtc/base/natserver.h" #include "webrtc/base/natsocketfactory.h" @@ -1159,3 +1160,34 @@ TEST_F(PortAllocatorTest, TestDestroyPortsNonSharedSockets) { (reinterpret_cast(*it))->Destroy(); } } + +class AllocationSequenceForTest : public cricket::AllocationSequence { + public: + AllocationSequenceForTest(cricket::BasicPortAllocatorSession* session, + rtc::Network* network, + cricket::PortConfiguration* config, + uint32 flags) + : cricket::AllocationSequence(session, network, config, flags) {} + using cricket::AllocationSequence::CreateTurnPort; +}; + +TEST_F(PortAllocatorTest, TestCreateTurnPortWithNullSocket) { + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + session_->StartGettingPorts(); + + cricket::ServerAddresses stun_servers; + stun_servers.insert(kStunAddr); + cricket::PortConfiguration config(stun_servers, kIceUfrag0, kIcePwd0); + rtc::Network network1("test_eth0", "Test Network Adapter 1", + rtc::IPAddress(0x12345600U), 24); + uint32 flag = cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET; + AllocationSequenceForTest alloc_sequence( + static_cast(session_.get()), + &network1, &config, flag); + // This simply tests it will not crash if udp_socket_ in the + // AllocationSequence is null, which is chosen in the constructor. + cricket::RelayServerConfig relay_server(cricket::RELAY_TURN); + relay_server.ports.push_back( + cricket::ProtocolAddress(kTurnUdpIntAddr, cricket::PROTO_UDP, false)); + alloc_sequence.CreateTurnPort(relay_server); +}