Don't send STUN pings if we don't have a remote ufrag and pwd.

BUG=4495
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/44029004

Cr-Commit-Position: refs/heads/master@{#8926}
This commit is contained in:
Peter Thatcher 2015-04-02 16:39:16 -07:00
parent bc4b93453c
commit 7351f4689c
3 changed files with 39 additions and 2 deletions

View File

@ -1163,6 +1163,14 @@ void P2PTransportChannel::OnPing() {
// Is the connection in a state for us to even consider pinging the other side?
bool P2PTransportChannel::IsPingable(Connection* conn) {
const Candidate& remote = conn->remote_candidate();
// We should never get this far with an empty remote ufrag.
ASSERT(!remote.username().empty());
if (remote.username().empty() || remote.password().empty()) {
// If we don't have an ICE ufrag and pwd, there's no way we can ping.
return false;
}
// An unconnected connection cannot be written to at all, so pinging is out
// of the question.
if (!conn->connected())

View File

@ -154,6 +154,9 @@ class P2PTransportChannel : public TransportChannelImpl,
// Helper method used only in unittest.
rtc::DiffServCodePoint DefaultDscpValue() const;
// Public for unit tests.
Connection* FindNextPingableConnection();
private:
rtc::Thread* thread() { return worker_thread_; }
PortAllocatorSession* allocator_session() {
@ -182,7 +185,6 @@ class P2PTransportChannel : public TransportChannelImpl,
void RememberRemoteCandidate(const Candidate& remote_candidate,
PortInterface* origin_port);
bool IsPingable(Connection* conn);
Connection* FindNextPingableConnection();
void PingConnection(Connection* conn);
void AddAllocatorSession(PortAllocatorSession* session);
void AddConnection(Connection* connection);

View File

@ -1267,7 +1267,11 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
PORTALLOCATOR_ENABLE_SHARED_UFRAG,
kDefaultStepDelay, kDefaultStepDelay,
cricket::ICEPROTO_RFC5245);
// Emulate no remote credentials coming in.
set_clear_remote_candidates_ufrag_pwd(false);
CreateChannels(1);
// Only have remote credentials come in for ep2, not ep1.
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
// Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
// candidate.
@ -1279,11 +1283,20 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
WAIT((best_connection = ep1_ch1()->best_connection()) != NULL, 2000);
EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());
// Because we don't have a remote pwd, we don't ping yet.
EXPECT_EQ(kIceUfrag[1],
ep1_ch1()->best_connection()->remote_candidate().username());
EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
ResumeCandidates(1);
WAIT(ep2_ch1()->best_connection() != NULL, 2000);
EXPECT_EQ(kIcePwd[1],
ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
WAIT(ep2_ch1()->best_connection() != NULL, 2000);
// Verify ep1's best connection is updated to use the 'local' candidate.
EXPECT_EQ_WAIT(
"local",
@ -1301,7 +1314,11 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
PORTALLOCATOR_ENABLE_SHARED_UFRAG,
kDefaultStepDelay, kDefaultStepDelay,
cricket::ICEPROTO_RFC5245);
// Emulate no remote credentials coming in.
set_clear_remote_candidates_ufrag_pwd(false);
CreateChannels(1);
// Only have remote credentials come in for ep2, not ep1.
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
// Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
// candidate.
PauseCandidates(1);
@ -1311,9 +1328,19 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
WAIT(ep1_ch1()->best_connection() != NULL, 2000);
EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());
// Because we don't have a remote pwd, we don't ping yet.
EXPECT_EQ(kIceUfrag[1],
ep1_ch1()->best_connection()->remote_candidate().username());
EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
ResumeCandidates(1);
EXPECT_EQ(kIcePwd[1],
ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
const cricket::Connection* best_connection = NULL;
WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 2000);