From f65de8483e90d1d52d5d8f40f646e77bf45b10ea Mon Sep 17 00:00:00 2001 From: Donald Curtis Date: Fri, 22 May 2015 14:55:19 -0700 Subject: [PATCH] Fix sending wrong candidates down to transportchannel. BUG=4665 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/54489004 Cr-Commit-Position: refs/heads/master@{#9266} --- talk/app/webrtc/webrtcsession_unittest.cc | 55 +++++++++++++++++++++++ webrtc/p2p/base/p2ptransportchannel.h | 3 ++ webrtc/p2p/base/session.cc | 7 +++ 3 files changed, 65 insertions(+) diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 2b6f207e2..f21319b15 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -40,10 +40,13 @@ #include "talk/media/base/fakevideorenderer.h" #include "talk/media/base/mediachannel.h" #include "talk/media/devices/fakedevicemanager.h" +#include "webrtc/p2p/base/p2ptransportchannel.h" +#include "webrtc/p2p/base/dtlstransportchannel.h" #include "webrtc/p2p/base/stunserver.h" #include "webrtc/p2p/base/teststunserver.h" #include "webrtc/p2p/base/testturnserver.h" #include "webrtc/p2p/base/transportchannel.h" +#include "webrtc/p2p/base/transportchannelproxy.h" #include "webrtc/p2p/client/basicportallocator.h" #include "talk/session/media/channelmanager.h" #include "talk/session/media/mediasession.h" @@ -2602,6 +2605,58 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) { EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error)); } +// Test that candidates sent to the "video" transport do not get pushed down to +// the "audio" transport channel when bundling using TransportProxy. +TEST_F(WebRtcSessionTest, TestBalancedBundleCandidateMisuse) { + AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); + + InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced); + mediastream_signaling_.SendAudioVideoStream1(); + + PeerConnectionInterface::RTCOfferAnswerOptions options; + options.use_rtp_mux = true; + + SessionDescriptionInterface* offer = CreateRemoteOffer(); + SetRemoteDescriptionWithoutError(offer); + + SessionDescriptionInterface* answer = CreateAnswer(NULL); + SetLocalDescriptionWithoutError(answer); + + EXPECT_EQ(session_->GetTransportProxy("audio")->impl(), + session_->GetTransportProxy("video")->impl()); + + cricket::Candidate candidate0; + candidate0.set_address(rtc::SocketAddress("1.1.1.1", 5000)); + candidate0.set_component(1); + candidate0.set_protocol("udp"); + JsepIceCandidate ice_candidate0(kMediaContentName0, kMediaContentIndex0, + candidate0); + EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate0)); + // Let the candidate get processed completely. + rtc::Thread::Current()->ProcessMessages(100); + + cricket::Transport* t = session_->GetTransport("audio"); + cricket::TransportStats stats; + + t->GetStats(&stats); + EXPECT_EQ(1, stats.channel_stats.size()); + EXPECT_EQ(2, stats.channel_stats[0].connection_infos.size()); + + cricket::Candidate candidate1; + candidate1.set_address(rtc::SocketAddress("1.1.1.1", 5001)); + candidate1.set_component(1); + candidate1.set_protocol("udp"); + JsepIceCandidate ice_candidate1(kMediaContentName1, kMediaContentIndex1, + candidate1); + EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate1)); + // Let the candidate get processed completely. + rtc::Thread::Current()->ProcessMessages(100); + + t->GetStats(&stats); + EXPECT_EQ(1, stats.channel_stats.size()); + EXPECT_EQ(2, stats.channel_stats[0].connection_infos.size()); +} + // kBundlePolicyBalanced bundle policy and answer contains BUNDLE. TEST_F(WebRtcSessionTest, TestBalancedBundleInAnswer) { InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced); diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index a014701f1..ab1d53c86 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -156,6 +156,9 @@ class P2PTransportChannel : public TransportChannelImpl, // Public for unit tests. Connection* FindNextPingableConnection(); + // Public for unit tests only. + size_t RemoteCandidateCount() const { return remote_candidates_.size(); } + private: rtc::Thread* thread() { return worker_thread_; } PortAllocatorSession* allocator_session() { diff --git a/webrtc/p2p/base/session.cc b/webrtc/p2p/base/session.cc index 38c983689..8cbe128a7 100644 --- a/webrtc/p2p/base/session.cc +++ b/webrtc/p2p/base/session.cc @@ -259,6 +259,13 @@ bool TransportProxy::OnRemoteCandidates(const Candidates& candidates, // TODO(juberti): Remove this once everybody calls SetLocalTD. CompleteNegotiation(); + // Ignore candidates for if the proxy content_name doesn't match the content + // name of the actual transport. This stops video candidates from being sent + // down to the audio transport when BUNDLE is enabled. + if (content_name_ != transport_->get()->content_name()) { + return true; + } + // Verify each candidate before passing down to transport layer. for (Candidates::const_iterator cand = candidates.begin(); cand != candidates.end(); ++cand) {