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}
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 1965ad9..3f5aa0a 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -58,94 +58,9 @@
} // 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<AllocationSequence*> SignalPortAllocationComplete;
-
- private:
- typedef std::vector<ProtocolType> 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<rtc::AsyncPacketSocket> udp_socket_;
- // There will be only one udp port per AllocationSequence.
- UDPPort* udp_port_;
- std::vector<TurnPort*> turn_ports_;
- int phase_;
-};
+ PORTALLOCATOR_DISABLE_UDP | PORTALLOCATOR_DISABLE_TCP |
+ PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_RELAY;
// BasicPortAllocator
BasicPortAllocator::BasicPortAllocator(
@@ -1059,7 +974,7 @@
// 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 f2d92ef..a27ff4c 100644
--- a/webrtc/p2p/client/basicportallocator.h
+++ b/webrtc/p2p/client/basicportallocator.h
@@ -237,6 +237,93 @@
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<AllocationSequence*> SignalPortAllocationComplete;
+
+ protected:
+ // For testing.
+ void CreateTurnPort(const RelayServerConfig& config);
+
+ private:
+ typedef std::vector<ProtocolType> 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<rtc::AsyncPacketSocket> udp_socket_;
+ // There will be only one udp port per AllocationSequence.
+ UDPPort* udp_port_;
+ std::vector<TurnPort*> 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 6940ea1..7dc2514 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 @@
(reinterpret_cast<cricket::Port*>(*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<cricket::BasicPortAllocatorSession*>(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);
+}
