From 2159b89fa2cb55beeef38f72bd45e217f3d33d4e Mon Sep 17 00:00:00 2001 From: Peter Thatcher Date: Fri, 21 Aug 2015 20:46:05 -0700 Subject: [PATCH] Reland "Remove GICE (gone forever!) and PORTALLOCATOR_ENABLE_SHARED_UFRAG (enabled forever)." becauese remoting code is using dead constants and breaks the FYI bots. This reverts commit 5bdafd44c86ee46bd7e040f19828324583418b33. Original CL: https://codereview.webrtc.org/1263663002/ R=guoweis@webrtc.org Review URL: https://codereview.webrtc.org/1303393002 . Cr-Commit-Position: refs/heads/master@{#9761} --- .../webrtc/jsepsessiondescription_unittest.cc | 2 - talk/app/webrtc/peerconnection.cc | 3 +- .../webrtc/peerconnectionendtoend_unittest.cc | 53 --- talk/app/webrtc/webrtcsdp.cc | 7 +- talk/app/webrtc/webrtcsdp_unittest.cc | 24 +- talk/app/webrtc/webrtcsession_unittest.cc | 79 +--- .../webrtc/webrtcsessiondescriptionfactory.cc | 1 - talk/session/media/mediasession.cc | 67 --- talk/session/media/mediasession.h | 19 - talk/session/media/mediasession_unittest.cc | 9 +- .../valgrind-webrtc/memcheck/suppressions.txt | 15 + webrtc/p2p/base/candidate.h | 8 +- webrtc/p2p/base/constants.cc | 15 - webrtc/p2p/base/constants.h | 14 - webrtc/p2p/base/dtlstransportchannel.h | 6 - .../p2p/base/dtlstransportchannel_unittest.cc | 34 +- webrtc/p2p/base/fakesession.h | 10 +- webrtc/p2p/base/p2ptransport.cc | 2 +- webrtc/p2p/base/p2ptransportchannel.cc | 239 +++------- webrtc/p2p/base/p2ptransportchannel.h | 3 - .../p2p/base/p2ptransportchannel_unittest.cc | 414 ++++++---------- webrtc/p2p/base/port.cc | 274 +++-------- webrtc/p2p/base/port.h | 23 +- webrtc/p2p/base/port_unittest.cc | 440 +++--------------- webrtc/p2p/base/portallocator.cc | 7 +- webrtc/p2p/base/portallocator.h | 5 + webrtc/p2p/base/portinterface.h | 3 - webrtc/p2p/base/rawtransport.cc | 45 +- webrtc/p2p/base/rawtransport.h | 48 +- webrtc/p2p/base/rawtransportchannel.cc | 262 +---------- webrtc/p2p/base/rawtransportchannel.h | 202 +------- webrtc/p2p/base/session.cc | 9 +- webrtc/p2p/base/session.h | 3 - webrtc/p2p/base/transport.cc | 85 +--- webrtc/p2p/base/transport.h | 12 +- webrtc/p2p/base/transport_unittest.cc | 42 +- webrtc/p2p/base/transportchannelimpl.h | 12 +- webrtc/p2p/base/transportdescription.h | 27 +- .../p2p/base/transportdescriptionfactory.cc | 43 +- webrtc/p2p/base/transportdescriptionfactory.h | 3 - .../transportdescriptionfactory_unittest.cc | 155 +----- webrtc/p2p/base/turnport_unittest.cc | 14 - webrtc/p2p/client/basicportallocator.cc | 8 - webrtc/p2p/client/connectivitychecker.cc | 3 - webrtc/p2p/client/fakeportallocator.h | 2 +- webrtc/p2p/client/httpportallocator.cc | 10 +- webrtc/p2p/client/portallocator_unittest.cc | 61 +-- webrtc/p2p/p2p.gyp | 4 - 48 files changed, 485 insertions(+), 2341 deletions(-) diff --git a/talk/app/webrtc/jsepsessiondescription_unittest.cc b/talk/app/webrtc/jsepsessiondescription_unittest.cc index 36f9a74caf..a60911494c 100644 --- a/talk/app/webrtc/jsepsessiondescription_unittest.cc +++ b/talk/app/webrtc/jsepsessiondescription_unittest.cc @@ -77,7 +77,6 @@ static cricket::SessionDescription* CreateCricketSessionDescription() { cricket::TransportInfo( cricket::CN_AUDIO, cricket::TransportDescription( - cricket::NS_GINGLE_P2P, std::vector(), kCandidateUfragVoice, kCandidatePwdVoice, cricket::ICEMODE_FULL, @@ -86,7 +85,6 @@ static cricket::SessionDescription* CreateCricketSessionDescription() { EXPECT_TRUE(desc->AddTransportInfo( cricket::TransportInfo(cricket::CN_VIDEO, cricket::TransportDescription( - cricket::NS_GINGLE_P2P, std::vector(), kCandidateUfragVideo, kCandidatePwdVideo, cricket::ICEMODE_FULL, diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index a53e2cb510..ef5836bbdd 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -367,8 +367,7 @@ bool PeerConnection::Initialize( // To handle both internal and externally created port allocator, we will // enable BUNDLE here. int portallocator_flags = port_allocator_->flags(); - portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | + portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_ENABLE_IPV6; bool value; // If IPv6 flag was specified, we'll not override it by experiment. diff --git a/talk/app/webrtc/peerconnectionendtoend_unittest.cc b/talk/app/webrtc/peerconnectionendtoend_unittest.cc index 7800a6724a..ceabf04cf0 100644 --- a/talk/app/webrtc/peerconnectionendtoend_unittest.cc +++ b/talk/app/webrtc/peerconnectionendtoend_unittest.cc @@ -48,8 +48,6 @@ using webrtc::PeerConnectionInterface; namespace { -const char kExternalGiceUfrag[] = "1234567890123456"; -const char kExternalGicePwd[] = "123456789012345678901234"; const size_t kMaxWait = 10000; void RemoveLinesFromSdp(const std::string& line_start, @@ -98,24 +96,6 @@ void UseExternalSdes(std::string* sdp) { InjectAfter("a=mid:data\r\n", kDataSdes, sdp); } -void UseGice(std::string* sdp) { - InjectAfter("t=0 0\r\n", "a=ice-options:google-ice\r\n", sdp); - - std::string ufragline = "a=ice-ufrag:"; - std::string pwdline = "a=ice-pwd:"; - RemoveLinesFromSdp(ufragline, sdp); - RemoveLinesFromSdp(pwdline, sdp); - ufragline.append(kExternalGiceUfrag); - ufragline.append("\r\n"); - pwdline.append(kExternalGicePwd); - pwdline.append("\r\n"); - const std::string ufrag_pwd = ufragline + pwdline; - - InjectAfter("a=mid:audio\r\n", ufrag_pwd, sdp); - InjectAfter("a=mid:video\r\n", ufrag_pwd, sdp); - InjectAfter("a=mid:data\r\n", ufrag_pwd, sdp); -} - void RemoveBundle(std::string* sdp) { RemoveLinesFromSdp("a=group:BUNDLE", sdp); } @@ -179,37 +159,6 @@ class PeerConnectionEndToEndTest callee_->WaitForConnection(); } - void SetupLegacySdpConverter() { - caller_->SignalOnSdpCreated.connect( - this, &PeerConnectionEndToEndTest::ConvertToLegacySdp); - callee_->SignalOnSdpCreated.connect( - this, &PeerConnectionEndToEndTest::ConvertToLegacySdp); - } - - void ConvertToLegacySdp(std::string* sdp) { - UseExternalSdes(sdp); - UseGice(sdp); - RemoveBundle(sdp); - LOG(LS_INFO) << "ConvertToLegacySdp: " << *sdp; - } - - void SetupGiceConverter() { - caller_->SignalOnIceCandidateCreated.connect( - this, &PeerConnectionEndToEndTest::AddGiceCredsToCandidate); - callee_->SignalOnIceCandidateCreated.connect( - this, &PeerConnectionEndToEndTest::AddGiceCredsToCandidate); - } - - void AddGiceCredsToCandidate(std::string* sdp) { - std::string gice_creds = " username "; - gice_creds.append(kExternalGiceUfrag); - gice_creds.append(" password "); - gice_creds.append(kExternalGicePwd); - gice_creds.append("\r\n"); - Replace("\r\n", gice_creds, sdp); - LOG(LS_INFO) << "AddGiceCredsToCandidate: " << *sdp; - } - void OnCallerAddedDataChanel(DataChannelInterface* dc) { caller_signaled_data_channels_.push_back(dc); } @@ -281,8 +230,6 @@ TEST_F(PeerConnectionEndToEndTest, DISABLED_CallWithLegacySdp) { pc_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp, false); CreatePcs(&pc_constraints); - SetupLegacySdpConverter(); - SetupGiceConverter(); GetAndAddUserMedia(); Negotiate(); WaitForCallEstablished(); diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index 5518233f2f..28d4e9e8b1 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -77,7 +77,6 @@ using cricket::kCodecParamMaxPlaybackRate; using cricket::kCodecParamAssociatedPayloadType; using cricket::MediaContentDescription; using cricket::MediaType; -using cricket::NS_JINGLE_ICE_UDP; using cricket::RtpHeaderExtension; using cricket::SsrcGroup; using cricket::StreamParams; @@ -892,8 +891,7 @@ bool SdpDeserialize(const std::string& message, SdpParseError* error) { std::string session_id; std::string session_version; - TransportDescription session_td(NS_JINGLE_ICE_UDP, - std::string(), std::string()); + TransportDescription session_td("", ""); RtpHeaderExtensions session_extmaps; cricket::SessionDescription* desc = new cricket::SessionDescription(); std::vector candidates; @@ -2195,8 +2193,7 @@ bool ParseMediaDescription(const std::string& message, // Make a temporary TransportDescription based on |session_td|. // Some of this gets overwritten by ParseContent. - TransportDescription transport(NS_JINGLE_ICE_UDP, - session_td.transport_options, + TransportDescription transport(session_td.transport_options, session_td.ice_ufrag, session_td.ice_pwd, session_td.ice_mode, diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index 869c3ecabd..a534c23920 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -55,7 +55,6 @@ using cricket::ICE_CANDIDATE_COMPONENT_RTP; using cricket::kFecSsrcGroupSemantics; using cricket::LOCAL_PORT_TYPE; using cricket::NS_JINGLE_DRAFT_SCTP; -using cricket::NS_JINGLE_ICE_UDP; using cricket::NS_JINGLE_RTP; using cricket::RtpHeaderExtension; using cricket::RELAY_PORT_TYPE; @@ -580,13 +579,11 @@ class WebRtcSdpTest : public testing::Test { // TransportInfo EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kAudioContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - kCandidateUfragVoice, + TransportDescription(kCandidateUfragVoice, kCandidatePwdVoice)))); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kVideoContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - kCandidateUfragVideo, + TransportDescription(kCandidateUfragVideo, kCandidatePwdVideo)))); // v4 host @@ -863,8 +860,6 @@ class WebRtcSdpTest : public testing::Test { const cricket::TransportInfo transport1 = transports1.at(i); const cricket::TransportInfo transport2 = transports2.at(i); EXPECT_EQ(transport1.content_name, transport2.content_name); - EXPECT_EQ(transport1.description.transport_type, - transport2.description.transport_type); EXPECT_EQ(transport1.description.ice_ufrag, transport2.description.ice_ufrag); EXPECT_EQ(transport1.description.ice_pwd, @@ -945,8 +940,7 @@ class WebRtcSdpTest : public testing::Test { ASSERT(false); } TransportInfo transport_info( - content_name, TransportDescription(NS_JINGLE_ICE_UDP, - ufrag, pwd)); + content_name, TransportDescription(ufrag, pwd)); SessionDescription* desc = const_cast(jdesc->description()); desc->RemoveTransportInfoByName(content_name); @@ -983,8 +977,7 @@ class WebRtcSdpTest : public testing::Test { sizeof(kIdentityDigest)); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kAudioContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), + TransportDescription(std::vector(), kCandidateUfragVoice, kCandidatePwdVoice, cricket::ICEMODE_FULL, @@ -992,8 +985,7 @@ class WebRtcSdpTest : public testing::Test { &fingerprint, Candidates())))); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kVideoContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), + TransportDescription(std::vector(), kCandidateUfragVideo, kCandidatePwdVideo, cricket::ICEMODE_FULL, @@ -1073,8 +1065,7 @@ class WebRtcSdpTest : public testing::Test { desc_.AddContent(kDataContentName, NS_JINGLE_DRAFT_SCTP, data.release()); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kDataContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - kCandidateUfragData, + TransportDescription(kCandidateUfragData, kCandidatePwdData)))); } @@ -1097,8 +1088,7 @@ class WebRtcSdpTest : public testing::Test { desc_.AddContent(kDataContentName, NS_JINGLE_RTP, data.release()); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kDataContentName, - TransportDescription(NS_JINGLE_ICE_UDP, - kCandidateUfragData, + TransportDescription(kCandidateUfragData, kCandidatePwdData)))); } diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index e442cf158b..1e1785fe57 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -71,8 +71,6 @@ using cricket::BaseSession; using cricket::DF_PLAY; using cricket::DF_SEND; using cricket::FakeVoiceMediaChannel; -using cricket::NS_GINGLE_P2P; -using cricket::NS_JINGLE_ICE_UDP; using cricket::TransportInfo; using rtc::SocketAddress; using rtc::scoped_ptr; @@ -337,8 +335,6 @@ class WebRtcSessionTest : public testing::Test { turn_server_(Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr), mediastream_signaling_(channel_manager_.get()), metrics_observer_(new rtc::RefCountedObject()) { - tdesc_factory_->set_protocol(cricket::ICEPROTO_HYBRID); - cricket::ServerAddresses stun_servers; stun_servers.insert(stun_socket_addr_); allocator_.reset(new cricket::BasicPortAllocator( @@ -346,8 +342,7 @@ class WebRtcSessionTest : public testing::Test { stun_servers, SocketAddress(), SocketAddress(), SocketAddress())); allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_DISABLE_RELAY | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); + cricket::PORTALLOCATOR_DISABLE_RELAY); EXPECT_TRUE(channel_manager_->Init()); desc_factory_->set_add_legacy_streams(false); allocator_->set_step_delay(cricket::kMinimumStepDelay); @@ -1151,13 +1146,6 @@ class WebRtcSessionTest : public testing::Test { TestLoopbackCall(config); } - void VerifyTransportType(const std::string& content_name, - cricket::TransportProtocol protocol) { - const cricket::Transport* transport = session_->GetTransport(content_name); - ASSERT_TRUE(transport != NULL); - EXPECT_EQ(protocol, transport->protocol()); - } - // Adds CN codecs to FakeMediaEngine and MediaDescriptionFactory. void AddCNCodecs() { const cricket::AudioCodec kCNCodec1(102, "CN", 8000, 0, 1, 0); @@ -1239,8 +1227,7 @@ class WebRtcSessionTest : public testing::Test { kTurnUdpIntAddr, cricket::PROTO_UDP, false)); allocator_->AddRelay(relay_server); allocator_->set_step_delay(cricket::kMinimumStepDelay); - allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); + allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP); } cricket::FakeMediaEngine* media_engine_; @@ -2546,7 +2533,6 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithoutIce) { // too short ice ufrag and pwd strings. TEST_F(WebRtcSessionTest, TestSetLocalDescriptionInvalidIceCredentials) { Init(); - tdesc_factory_->set_protocol(cricket::ICEPROTO_RFC5245); mediastream_signaling_.SendAudioVideoStream1(); rtc::scoped_ptr offer(CreateOffer()); @@ -2572,7 +2558,6 @@ TEST_F(WebRtcSessionTest, TestSetLocalDescriptionInvalidIceCredentials) { // too short ice ufrag and pwd strings. TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) { Init(); - tdesc_factory_->set_protocol(cricket::ICEPROTO_RFC5245); rtc::scoped_ptr offer(CreateRemoteOffer()); std::string sdp; // Modifying ice ufrag and pwd in remote offer with strings smaller than the @@ -3146,55 +3131,6 @@ TEST_F(WebRtcSessionTest, TestInitiatorFlagAsReceiver) { EXPECT_FALSE(session_->initiator()); } -// This test verifies the ice protocol type at initiator of the call -// if |a=ice-options:google-ice| is present in answer. -TEST_F(WebRtcSessionTest, TestInitiatorGIceInAnswer) { - Init(); - mediastream_signaling_.SendAudioVideoStream1(); - SessionDescriptionInterface* offer = CreateOffer(); - rtc::scoped_ptr answer( - CreateRemoteAnswer(offer)); - SetLocalDescriptionWithoutError(offer); - std::string sdp; - EXPECT_TRUE(answer->ToString(&sdp)); - // Adding ice-options to the session level. - InjectAfter("t=0 0\r\n", - "a=ice-options:google-ice\r\n", - &sdp); - SessionDescriptionInterface* answer_with_gice = - CreateSessionDescription(JsepSessionDescription::kAnswer, sdp, NULL); - // Default offer is ICEPROTO_RFC5245, so we expect responder with - // only gice to fail. - SetRemoteDescriptionAnswerExpectError(kPushDownTDFailed, answer_with_gice); -} - -// This test verifies the ice protocol type at initiator of the call -// if ICE RFC5245 is supported in answer. -TEST_F(WebRtcSessionTest, TestInitiatorIceInAnswer) { - Init(); - mediastream_signaling_.SendAudioVideoStream1(); - SessionDescriptionInterface* offer = CreateOffer(); - SessionDescriptionInterface* answer = CreateRemoteAnswer(offer); - SetLocalDescriptionWithoutError(offer); - - SetRemoteDescriptionWithoutError(answer); - VerifyTransportType("audio", cricket::ICEPROTO_RFC5245); - VerifyTransportType("video", cricket::ICEPROTO_RFC5245); -} - -// This test verifies the ice protocol type at receiver side of the call if -// receiver decides to use ice RFC 5245. -TEST_F(WebRtcSessionTest, TestReceiverIceInOffer) { - Init(); - mediastream_signaling_.SendAudioVideoStream1(); - SessionDescriptionInterface* offer = CreateOffer(); - SetRemoteDescriptionWithoutError(offer); - SessionDescriptionInterface* answer = CreateAnswer(NULL); - SetLocalDescriptionWithoutError(answer); - VerifyTransportType("audio", cricket::ICEPROTO_RFC5245); - VerifyTransportType("video", cricket::ICEPROTO_RFC5245); -} - // Verifing local offer and remote answer have matching m-lines as per RFC 3264. TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) { Init(); @@ -3415,16 +3351,14 @@ TEST_F(WebRtcSessionTest, TestSessionContentError) { // Runs the loopback call test with BUNDLE and STUN disabled. TEST_F(WebRtcSessionTest, TestIceStatesBasic) { // Lets try with only UDP ports. - allocator_->set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_DISABLE_TCP | + allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_STUN | cricket::PORTALLOCATOR_DISABLE_RELAY); TestLoopbackCall(); } TEST_F(WebRtcSessionTest, TestIceStatesBasicIPv6) { - allocator_->set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_DISABLE_TCP | + allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_STUN | cricket::PORTALLOCATOR_ENABLE_IPV6 | cricket::PORTALLOCATOR_DISABLE_RELAY); @@ -3440,9 +3374,8 @@ TEST_F(WebRtcSessionTest, TestIceStatesBasicIPv6) { // Runs the loopback call test with BUNDLE and STUN enabled. TEST_F(WebRtcSessionTest, TestIceStatesBundle) { - allocator_->set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_DISABLE_RELAY); + allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | + cricket::PORTALLOCATOR_DISABLE_RELAY); TestLoopbackCall(); } diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc index 3dcc0a1a3b..6c6981ce5a 100644 --- a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc +++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc @@ -148,7 +148,6 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( session_id_(session_id), data_channel_type_(dct), identity_request_state_(IDENTITY_NOT_NEEDED) { - transport_desc_factory_.set_protocol(cricket::ICEPROTO_RFC5245); session_desc_factory_.set_add_legacy_streams(false); // SRTP-SDES is disabled if DTLS is on. SetSdesPolicy(dtls_enabled ? cricket::SEC_DISABLED : cricket::SEC_REQUIRED); diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index a1426c34ab..66d0caf9d7 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -1908,71 +1908,4 @@ const DataContentDescription* GetFirstDataContentDescription( GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA)); } -bool GetMediaChannelNameFromComponent( - int component, MediaType media_type, std::string* channel_name) { - if (media_type == MEDIA_TYPE_AUDIO) { - if (component == ICE_CANDIDATE_COMPONENT_RTP) { - *channel_name = GICE_CHANNEL_NAME_RTP; - return true; - } else if (component == ICE_CANDIDATE_COMPONENT_RTCP) { - *channel_name = GICE_CHANNEL_NAME_RTCP; - return true; - } - } else if (media_type == MEDIA_TYPE_VIDEO) { - if (component == ICE_CANDIDATE_COMPONENT_RTP) { - *channel_name = GICE_CHANNEL_NAME_VIDEO_RTP; - return true; - } else if (component == ICE_CANDIDATE_COMPONENT_RTCP) { - *channel_name = GICE_CHANNEL_NAME_VIDEO_RTCP; - return true; - } - } else if (media_type == MEDIA_TYPE_DATA) { - if (component == ICE_CANDIDATE_COMPONENT_RTP) { - *channel_name = GICE_CHANNEL_NAME_DATA_RTP; - return true; - } else if (component == ICE_CANDIDATE_COMPONENT_RTCP) { - *channel_name = GICE_CHANNEL_NAME_DATA_RTCP; - return true; - } - } - - return false; -} - -bool GetMediaComponentFromChannelName( - const std::string& channel_name, int* component) { - if (channel_name == GICE_CHANNEL_NAME_RTP || - channel_name == GICE_CHANNEL_NAME_VIDEO_RTP || - channel_name == GICE_CHANNEL_NAME_DATA_RTP) { - *component = ICE_CANDIDATE_COMPONENT_RTP; - return true; - } else if (channel_name == GICE_CHANNEL_NAME_RTCP || - channel_name == GICE_CHANNEL_NAME_VIDEO_RTCP || - channel_name == GICE_CHANNEL_NAME_DATA_RTP) { - *component = ICE_CANDIDATE_COMPONENT_RTCP; - return true; - } - - return false; -} - -bool GetMediaTypeFromChannelName( - const std::string& channel_name, MediaType* media_type) { - if (channel_name == GICE_CHANNEL_NAME_RTP || - channel_name == GICE_CHANNEL_NAME_RTCP) { - *media_type = MEDIA_TYPE_AUDIO; - return true; - } else if (channel_name == GICE_CHANNEL_NAME_VIDEO_RTP || - channel_name == GICE_CHANNEL_NAME_VIDEO_RTCP) { - *media_type = MEDIA_TYPE_VIDEO; - return true; - } else if (channel_name == GICE_CHANNEL_NAME_DATA_RTP || - channel_name == GICE_CHANNEL_NAME_DATA_RTCP) { - *media_type = MEDIA_TYPE_DATA; - return true; - } - - return false; -} - } // namespace cricket diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h index 5bcc8cc55b..d329dcdf27 100644 --- a/talk/session/media/mediasession.h +++ b/talk/session/media/mediasession.h @@ -547,25 +547,6 @@ const VideoContentDescription* GetFirstVideoContentDescription( const DataContentDescription* GetFirstDataContentDescription( const SessionDescription* sdesc); -// Functions for translating media candidate names. - -// For converting between media ICE component and G-ICE channel -// names. For example: -// "rtp" <=> 1 -// "rtcp" <=> 2 -// "video_rtp" <=> 1 -// "video_rtcp" <=> 2 -// Will not convert in the general case of arbitrary channel names, -// but is useful for cases where we have candidates for media -// channels. -// returns false if there is no mapping. -bool GetMediaChannelNameFromComponent( - int component, cricket::MediaType media_type, std::string* channel_name); -bool GetMediaComponentFromChannelName( - const std::string& channel_name, int* component); -bool GetMediaTypeFromChannelName( - const std::string& channel_name, cricket::MediaType* media_type); - void GetSupportedAudioCryptoSuites(std::vector* crypto_suites); void GetSupportedVideoCryptoSuites(std::vector* crypto_suites); void GetSupportedDataCryptoSuites(std::vector* crypto_suites); diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc index f8b4798f3a..ededa8a680 100644 --- a/talk/session/media/mediasession_unittest.cc +++ b/talk/session/media/mediasession_unittest.cc @@ -290,18 +290,15 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { current_desc.reset(new SessionDescription()); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("audio", - TransportDescription("", - current_audio_ufrag, + TransportDescription(current_audio_ufrag, current_audio_pwd)))); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("video", - TransportDescription("", - current_video_ufrag, + TransportDescription(current_video_ufrag, current_video_pwd)))); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("data", - TransportDescription("", - current_data_ufrag, + TransportDescription(current_data_ufrag, current_data_pwd)))); } if (offer) { diff --git a/tools/valgrind-webrtc/memcheck/suppressions.txt b/tools/valgrind-webrtc/memcheck/suppressions.txt index 97f3240b8f..6291df20dd 100644 --- a/tools/valgrind-webrtc/memcheck/suppressions.txt +++ b/tools/valgrind-webrtc/memcheck/suppressions.txt @@ -125,6 +125,21 @@ fun:_ZN3rtc10HttpServer10Connection12BeginProcessEPNS_15StreamInterfaceE ... } +{ + SignalsCloseAfterForcedCloseAll2 + Memcheck:Leak + fun:_Znw* + fun:_ZN9__gnu_cxx13new_allocatorISt13_Rb_tree_nodeISt4pairIKSsSsEEE8allocateEmPKv + fun:_ZNSt8_Rb_treeISsSt4pairIKSsSsESt10_Select1stIS2_EN3rtc5ilessESaIS2_EE11_M_get_nodeEv + fun:_ZNSt8_Rb_treeISsSt4pairIKSsSsESt10_Select1stIS2_EN3rtc5ilessESaIS2_EE14_M_create_nodeIJS2_EEEPSt13_Rb_tree_nodeIS2_EDpOT_ + fun:_ZNSt8_Rb_treeISsSt4pairIKSsSsESt10_Select1stIS2_EN3rtc5ilessESaIS2_EE10_M_insert_IS2_EESt17_Rb_tree_iteratorIS2_EPKSt18_Rb_tree_node_baseSE_OT_ + fun:_ZNSt8_Rb_treeISsSt4pairIKSsSsESt10_Select1stIS2_EN3rtc5ilessESaIS2_EE15_M_insert_equalIS2_EESt17_Rb_tree_iteratorIS2_EOT_ + fun:_ZNSt8multimapISsSsN3rtc5ilessESaISt4pairIKSsSsEEE6insertIS4_vEESt17_Rb_tree_iteratorIS4_EOT_ + fun:_ZN3rtc8HttpData12changeHeaderERKSsS2_NS0_13HeaderCombineE + fun:_ZN3rtc8HttpData9setHeaderERKSsS2_b + fun:_ZN3rtc8HttpData9setHeaderENS_10HttpHeaderERKSsb + ... +} { DoNotDeleteTask2 Memcheck:Leak diff --git a/webrtc/p2p/base/candidate.h b/webrtc/p2p/base/candidate.h index 66b0ab04b3..2655c1b26c 100644 --- a/webrtc/p2p/base/candidate.h +++ b/webrtc/p2p/base/candidate.h @@ -84,10 +84,8 @@ class Candidate { uint32 priority() const { return priority_; } void set_priority(const uint32 priority) { priority_ = priority; } -// void set_type_preference(uint32 type_preference) { -// priority_ = GetPriority(type_preference); -// } - + // TODO(pthatcher): Remove once Chromium's jingle/glue/utils.cc + // doesn't use it. // Maps old preference (which was 0.0-1.0) to match priority (which // is 0-2^32-1) to to match RFC 5245, section 4.1.2.1. Also see // https://docs.google.com/a/google.com/document/d/ @@ -97,6 +95,8 @@ class Candidate { return static_cast(((priority_ >> 24) * 100 / 127) / 100.0); } + // TODO(pthatcher): Remove once Chromium's jingle/glue/utils.cc + // doesn't use it. void set_preference(float preference) { // Limiting priority to UINT_MAX when value exceeds uint32 max. // This can happen for e.g. when preference = 3. diff --git a/webrtc/p2p/base/constants.cc b/webrtc/p2p/base/constants.cc index 614cbc845a..2a258718f4 100644 --- a/webrtc/p2p/base/constants.cc +++ b/webrtc/p2p/base/constants.cc @@ -21,12 +21,6 @@ const char CN_OTHER[] = "main"; const char GROUP_TYPE_BUNDLE[] = "BUNDLE"; -const char NS_JINGLE_ICE_UDP[] = "urn:xmpp:jingle:transports:ice-udp:1"; -const char NS_GINGLE_P2P[] = "http://www.google.com/transport/p2p"; -const char NS_GINGLE_RAW[] = "http://www.google.com/transport/raw-udp"; - -const char ICE_OPTION_GICE[] = "google-ice"; - // Minimum ufrag length is 4 characters as per RFC5245. We chose 16 because // some internal systems expect username to be 16 bytes. const int ICE_UFRAG_LENGTH = 16; @@ -37,7 +31,6 @@ const size_t ICE_UFRAG_MIN_LENGTH = 4; const size_t ICE_PWD_MIN_LENGTH = 22; const size_t ICE_UFRAG_MAX_LENGTH = 255; const size_t ICE_PWD_MAX_LENGTH = 256; -const size_t GICE_UFRAG_MAX_LENGTH = 16; // TODO: This is media-specific, so might belong // somewhere like media/base/constants.h @@ -48,14 +41,6 @@ const int ICE_CANDIDATE_COMPONENT_DEFAULT = 1; const char NS_JINGLE_RTP[] = "urn:xmpp:jingle:apps:rtp:1"; const char NS_JINGLE_DRAFT_SCTP[] = "google:jingle:sctp"; -const char GICE_CHANNEL_NAME_RTP[] = "rtp"; -const char GICE_CHANNEL_NAME_RTCP[] = "rtcp"; -const char GICE_CHANNEL_NAME_VIDEO_RTP[] = "video_rtp"; -const char GICE_CHANNEL_NAME_VIDEO_RTCP[] = "video_rtcp"; -const char GICE_CHANNEL_NAME_DATA_RTP[] = "data_rtp"; -const char GICE_CHANNEL_NAME_DATA_RTCP[] = "data_rtcp"; - - // From RFC 4145, SDP setup attribute values. const char CONNECTIONROLE_ACTIVE_STR[] = "active"; const char CONNECTIONROLE_PASSIVE_STR[] = "passive"; diff --git a/webrtc/p2p/base/constants.h b/webrtc/p2p/base/constants.h index 90a7816239..c3e1b781dc 100644 --- a/webrtc/p2p/base/constants.h +++ b/webrtc/p2p/base/constants.h @@ -28,19 +28,12 @@ extern const char CN_OTHER[]; // GN stands for group name extern const char GROUP_TYPE_BUNDLE[]; -extern const char NS_JINGLE_ICE_UDP[]; -extern const char NS_GINGLE_P2P[]; -extern const char NS_GINGLE_RAW[]; - -extern const char ICE_OPTION_GICE[]; - extern const int ICE_UFRAG_LENGTH; extern const int ICE_PWD_LENGTH; extern const size_t ICE_UFRAG_MIN_LENGTH; extern const size_t ICE_PWD_MIN_LENGTH; extern const size_t ICE_UFRAG_MAX_LENGTH; extern const size_t ICE_PWD_MAX_LENGTH; -extern const size_t GICE_UFRAG_MAX_LENGTH; extern const int ICE_CANDIDATE_COMPONENT_RTP; extern const int ICE_CANDIDATE_COMPONENT_RTCP; @@ -49,13 +42,6 @@ extern const int ICE_CANDIDATE_COMPONENT_DEFAULT; extern const char NS_JINGLE_RTP[]; extern const char NS_JINGLE_DRAFT_SCTP[]; -extern const char GICE_CHANNEL_NAME_RTP[]; -extern const char GICE_CHANNEL_NAME_RTCP[]; -extern const char GICE_CHANNEL_NAME_VIDEO_RTP[]; -extern const char GICE_CHANNEL_NAME_VIDEO_RTCP[]; -extern const char GICE_CHANNEL_NAME_DATA_RTP[]; -extern const char GICE_CHANNEL_NAME_DATA_RTCP[]; - // RFC 4145, SDP setup attribute values. extern const char CONNECTIONROLE_ACTIVE_STR[]; extern const char CONNECTIONROLE_PASSIVE_STR[]; diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index 1bf77b6566..9c14314135 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -177,12 +177,6 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { virtual void SetIceTiebreaker(uint64 tiebreaker) { channel_->SetIceTiebreaker(tiebreaker); } - virtual bool GetIceProtocolType(IceProtocolType* type) const { - return channel_->GetIceProtocolType(type); - } - virtual void SetIceProtocolType(IceProtocolType type) { - channel_->SetIceProtocolType(type); - } virtual void SetIceCredentials(const std::string& ice_ufrag, const std::string& ice_pwd) { channel_->SetIceCredentials(ice_ufrag, ice_pwd); diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index dfae0c8844..26f6578d7b 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -51,7 +51,6 @@ class DtlsTestClient : public sigslot::has_slots<> { name_(name), signaling_thread_(signaling_thread), worker_thread_(worker_thread), - protocol_(cricket::ICEPROTO_GOOGLE), packet_size_(0), use_dtls_srtp_(false), ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_10), @@ -59,9 +58,6 @@ class DtlsTestClient : public sigslot::has_slots<> { received_dtls_client_hello_(false), received_dtls_server_hello_(false) { } - void SetIceProtocol(cricket::TransportProtocol proto) { - protocol_ = proto; - } void CreateIdentity(rtc::KeyType key_type) { identity_.reset(rtc::SSLIdentity::Generate(name_, key_type)); } @@ -162,10 +158,8 @@ class DtlsTestClient : public sigslot::has_slots<> { } } - std::string transport_type = (protocol_ == cricket::ICEPROTO_GOOGLE) ? - cricket::NS_GINGLE_P2P : cricket::NS_JINGLE_ICE_UDP; cricket::TransportDescription local_desc( - transport_type, std::vector(), kIceUfrag1, kIcePwd1, + std::vector(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, local_role, // If remote if the offerer and has no DTLS support, answer will be // without any fingerprint. @@ -174,7 +168,7 @@ class DtlsTestClient : public sigslot::has_slots<> { cricket::Candidates()); cricket::TransportDescription remote_desc( - transport_type, std::vector(), kIceUfrag1, kIcePwd1, + std::vector(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, remote_role, remote_fingerprint.get(), cricket::Candidates()); @@ -376,7 +370,6 @@ class DtlsTestClient : public sigslot::has_slots<> { std::string name_; rtc::Thread* signaling_thread_; rtc::Thread* worker_thread_; - cricket::TransportProtocol protocol_; rtc::scoped_ptr identity_; rtc::scoped_ptr transport_; std::vector channels_; @@ -549,8 +542,6 @@ class DtlsTransportChannelTest : public testing::Test { // Test that transport negotiation of ICE, no DTLS works properly. TEST_F(DtlsTransportChannelTest, TestChannelSetupIce) { - client1_.SetIceProtocol(cricket::ICEPROTO_RFC5245); - client2_.SetIceProtocol(cricket::ICEPROTO_RFC5245); Negotiate(); cricket::FakeTransportChannel* channel1 = client1_.GetFakeChannel(0); cricket::FakeTransportChannel* channel2 = client2_.GetFakeChannel(0); @@ -558,31 +549,10 @@ TEST_F(DtlsTransportChannelTest, TestChannelSetupIce) { ASSERT_TRUE(channel2 != NULL); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel1->GetIceRole()); EXPECT_EQ(1U, channel1->IceTiebreaker()); - EXPECT_EQ(cricket::ICEPROTO_RFC5245, channel1->protocol()); EXPECT_EQ(kIceUfrag1, channel1->ice_ufrag()); EXPECT_EQ(kIcePwd1, channel1->ice_pwd()); EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel2->GetIceRole()); EXPECT_EQ(2U, channel2->IceTiebreaker()); - EXPECT_EQ(cricket::ICEPROTO_RFC5245, channel2->protocol()); -} - -// Test that transport negotiation of GICE, no DTLS works properly. -TEST_F(DtlsTransportChannelTest, TestChannelSetupGice) { - client1_.SetIceProtocol(cricket::ICEPROTO_GOOGLE); - client2_.SetIceProtocol(cricket::ICEPROTO_GOOGLE); - Negotiate(); - cricket::FakeTransportChannel* channel1 = client1_.GetFakeChannel(0); - cricket::FakeTransportChannel* channel2 = client2_.GetFakeChannel(0); - ASSERT_TRUE(channel1 != NULL); - ASSERT_TRUE(channel2 != NULL); - EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel1->GetIceRole()); - EXPECT_EQ(1U, channel1->IceTiebreaker()); - EXPECT_EQ(cricket::ICEPROTO_GOOGLE, channel1->protocol()); - EXPECT_EQ(kIceUfrag1, channel1->ice_ufrag()); - EXPECT_EQ(kIcePwd1, channel1->ice_pwd()); - EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel2->GetIceRole()); - EXPECT_EQ(2U, channel2->IceTiebreaker()); - EXPECT_EQ(cricket::ICEPROTO_GOOGLE, channel2->protocol()); } // Connect without DTLS, and transfer some data. diff --git a/webrtc/p2p/base/fakesession.h b/webrtc/p2p/base/fakesession.h index 67b770ae5c..b63958e4d3 100644 --- a/webrtc/p2p/base/fakesession.h +++ b/webrtc/p2p/base/fakesession.h @@ -53,7 +53,6 @@ class FakeTransportChannel : public TransportChannelImpl, do_dtls_(false), role_(ICEROLE_UNKNOWN), tiebreaker_(0), - ice_proto_(ICEPROTO_HYBRID), remote_ice_mode_(ICEMODE_FULL), dtls_fingerprint_("", NULL, 0), ssl_role_(rtc::SSL_CLIENT), @@ -64,7 +63,6 @@ class FakeTransportChannel : public TransportChannelImpl, } uint64 IceTiebreaker() const { return tiebreaker_; } - TransportProtocol protocol() const { return ice_proto_; } IceMode remote_ice_mode() const { return remote_ice_mode_; } const std::string& ice_ufrag() const { return ice_ufrag_; } const std::string& ice_pwd() const { return ice_pwd_; } @@ -97,11 +95,6 @@ class FakeTransportChannel : public TransportChannelImpl, virtual void SetIceRole(IceRole role) { role_ = role; } virtual IceRole GetIceRole() const { return role_; } virtual void SetIceTiebreaker(uint64 tiebreaker) { tiebreaker_ = tiebreaker; } - virtual bool GetIceProtocolType(IceProtocolType* type) const { - *type = ice_proto_; - return true; - } - virtual void SetIceProtocolType(IceProtocolType type) { ice_proto_ = type; } virtual void SetIceCredentials(const std::string& ice_ufrag, const std::string& ice_pwd) { ice_ufrag_ = ice_ufrag; @@ -320,7 +313,6 @@ class FakeTransportChannel : public TransportChannelImpl, std::string chosen_srtp_cipher_; IceRole role_; uint64 tiebreaker_; - IceProtocolType ice_proto_; std::string ice_ufrag_; std::string ice_pwd_; std::string remote_ice_ufrag_; @@ -342,7 +334,7 @@ class FakeTransport : public Transport { const std::string& content_name, PortAllocator* alllocator = NULL) : Transport(signaling_thread, worker_thread, - content_name, "test_type", NULL), + content_name, NULL), dest_(NULL), async_(false), identity_(NULL) { diff --git a/webrtc/p2p/base/p2ptransport.cc b/webrtc/p2p/base/p2ptransport.cc index 89586f9f3d..b919fde31a 100644 --- a/webrtc/p2p/base/p2ptransport.cc +++ b/webrtc/p2p/base/p2ptransport.cc @@ -25,7 +25,7 @@ P2PTransport::P2PTransport(rtc::Thread* signaling_thread, const std::string& content_name, PortAllocator* allocator) : Transport(signaling_thread, worker_thread, - content_name, NS_GINGLE_P2P, allocator) { + content_name, allocator) { } P2PTransport::~P2PTransport() { diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 51f4736e24..094a8dcc8f 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -124,14 +124,6 @@ class ConnectionCompare { cricket::Connection* a = const_cast(ca); cricket::Connection* b = const_cast(cb); - // The IceProtocol is initialized to ICEPROTO_HYBRID and can be updated to - // GICE or RFC5245 when an answer SDP is set, or when a STUN message is - // received. So the port receiving the STUN message may have a different - // IceProtocol if the answer SDP is not set yet. - ASSERT(a->port()->IceProtocol() == b->port()->IceProtocol() || - a->port()->IceProtocol() == cricket::ICEPROTO_HYBRID || - b->port()->IceProtocol() == cricket::ICEPROTO_HYBRID); - // Compare first on writability and static preferences. int cmp = CompareConnections(a, b); if (cmp > 0) @@ -192,7 +184,6 @@ P2PTransportChannel::P2PTransportChannel(const std::string& content_name, pending_best_connection_(NULL), sort_dirty_(false), was_writable_(false), - protocol_type_(ICEPROTO_HYBRID), remote_ice_mode_(ICEMODE_FULL), ice_role_(ICEROLE_UNKNOWN), tiebreaker_(0), @@ -293,21 +284,6 @@ TransportChannelState P2PTransportChannel::GetState() const { return TransportChannelState::STATE_COMPLETED; } -bool P2PTransportChannel::GetIceProtocolType(IceProtocolType* type) const { - *type = protocol_type_; - return true; -} - -void P2PTransportChannel::SetIceProtocolType(IceProtocolType type) { - ASSERT(worker_thread_ == rtc::Thread::Current()); - - protocol_type_ = type; - for (std::vector::iterator it = ports_.begin(); - it != ports_.end(); ++it) { - (*it)->SetIceProtocolType(protocol_type_); - } -} - void P2PTransportChannel::SetIceCredentials(const std::string& ice_ufrag, const std::string& ice_pwd) { ASSERT(worker_thread_ == rtc::Thread::Current()); @@ -347,9 +323,8 @@ void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag, } if (ice_restart) { - // |candidate.generation()| is not signaled in ICEPROTO_RFC5245. - // Therefore we need to keep track of the remote ice restart so - // newer connections are prioritized over the older. + // We need to keep track of the remote ice restart so newer + // connections are prioritized over the older. ++remote_candidate_generation_; } } @@ -410,7 +385,6 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession *session, // The session will handle this, and send an initiate/accept/modify message // if one is pending. - port->SetIceProtocolType(protocol_type_); port->SetIceRole(ice_role_); port->SetIceTiebreaker(tiebreaker_); ports_.push_back(port); @@ -505,46 +479,30 @@ void P2PTransportChannel::OnUnknownAddress( } } else { // Create a new candidate with this address. - std::string type; int remote_candidate_priority; - if (port->IceProtocol() == ICEPROTO_RFC5245) { - // RFC 5245 - // If the source transport address of the request does not match any - // existing remote candidates, it represents a new peer reflexive remote - // candidate. - type = PRFLX_PORT_TYPE; - // The priority of the candidate is set to the PRIORITY attribute - // from the request. - const StunUInt32Attribute* priority_attr = - stun_msg->GetUInt32(STUN_ATTR_PRIORITY); - if (!priority_attr) { - LOG(LS_WARNING) << "P2PTransportChannel::OnUnknownAddress - " - << "No STUN_ATTR_PRIORITY found in the " - << "stun request message"; - port->SendBindingErrorResponse(stun_msg, address, - STUN_ERROR_BAD_REQUEST, - STUN_ERROR_REASON_BAD_REQUEST); - return; - } - remote_candidate_priority = priority_attr->value(); - } else { - // G-ICE doesn't support prflx candidate. - // We set candidate type to STUN_PORT_TYPE if the binding request comes - // from a relay port or the shared socket is used. Otherwise we use the - // port's type as the candidate type. - if (port->Type() == RELAY_PORT_TYPE || port->SharedSocket()) { - type = STUN_PORT_TYPE; - } else { - type = port->Type(); - } - remote_candidate_priority = remote_candidate.GetPriority( - ICE_TYPE_PREFERENCE_PRFLX, port->Network()->preference(), 0); + // The priority of the candidate is set to the PRIORITY attribute + // from the request. + const StunUInt32Attribute* priority_attr = + stun_msg->GetUInt32(STUN_ATTR_PRIORITY); + if (!priority_attr) { + LOG(LS_WARNING) << "P2PTransportChannel::OnUnknownAddress - " + << "No STUN_ATTR_PRIORITY found in the " + << "stun request message"; + port->SendBindingErrorResponse(stun_msg, address, + STUN_ERROR_BAD_REQUEST, + STUN_ERROR_REASON_BAD_REQUEST); + return; } + remote_candidate_priority = priority_attr->value(); + // RFC 5245 + // If the source transport address of the request does not match any + // existing remote candidates, it represents a new peer reflexive remote + // candidate. remote_candidate = Candidate(component(), ProtoToString(proto), address, 0, - remote_username, remote_password, type, 0U, ""); + remote_username, remote_password, PRFLX_PORT_TYPE, 0U, ""); // From RFC 5245, section-7.2.1.3: // The foundation of the candidate is set to an arbitrary value, different @@ -555,81 +513,56 @@ void P2PTransportChannel::OnUnknownAddress( remote_candidate.set_priority(remote_candidate_priority); } - if (port->IceProtocol() == ICEPROTO_RFC5245) { - // RFC5245, the agent constructs a pair whose local candidate is equal to - // the transport address on which the STUN request was received, and a - // remote candidate equal to the source transport address where the - // request came from. + // RFC5245, the agent constructs a pair whose local candidate is equal to + // the transport address on which the STUN request was received, and a + // remote candidate equal to the source transport address where the + // request came from. - // There shouldn't be an existing connection with this remote address. - // When ports are muxed, this channel might get multiple unknown address - // signals. In that case if the connection is already exists, we should - // simply ignore the signal otherwise send server error. - if (port->GetConnection(remote_candidate.address())) { - if (port_muxed) { - LOG(LS_INFO) << "Connection already exists for peer reflexive " - << "candidate: " << remote_candidate.ToString(); - return; - } else { - ASSERT(false); - port->SendBindingErrorResponse(stun_msg, address, - STUN_ERROR_SERVER_ERROR, - STUN_ERROR_REASON_SERVER_ERROR); - return; - } - } - - Connection* connection = port->CreateConnection( - remote_candidate, cricket::PortInterface::ORIGIN_THIS_PORT); - if (!connection) { + // There shouldn't be an existing connection with this remote address. + // When ports are muxed, this channel might get multiple unknown address + // signals. In that case if the connection is already exists, we should + // simply ignore the signal otherwise send server error. + if (port->GetConnection(remote_candidate.address())) { + if (port_muxed) { + LOG(LS_INFO) << "Connection already exists for peer reflexive " + << "candidate: " << remote_candidate.ToString(); + return; + } else { ASSERT(false); port->SendBindingErrorResponse(stun_msg, address, STUN_ERROR_SERVER_ERROR, STUN_ERROR_REASON_SERVER_ERROR); return; } - - LOG(LS_INFO) << "Adding connection from " - << (remote_candidate_is_new ? "peer reflexive" : "resurrected") - << " candidate: " << remote_candidate.ToString(); - AddConnection(connection); - connection->ReceivedPing(); - - // Send the pinger a successful stun response. - port->SendBindingResponse(stun_msg, address); - - bool received_use_candidate = - stun_msg->GetByteString(STUN_ATTR_USE_CANDIDATE) != nullptr; - if (received_use_candidate && ice_role_ == ICEROLE_CONTROLLED) { - connection->set_nominated(true); - OnNominated(connection); - } - - // Update the list of connections since we just added another. We do this - // after sending the response since it could (in principle) delete the - // connection in question. - SortConnections(); - } else { - // Check for connectivity to this address. Create connections - // to this address across all local ports. First, add this as a new remote - // address - if (!CreateConnections(remote_candidate, port, true)) { - // Hopefully this won't occur, because changing a destination address - // shouldn't cause a new connection to fail - ASSERT(false); - port->SendBindingErrorResponse(stun_msg, address, STUN_ERROR_SERVER_ERROR, - STUN_ERROR_REASON_SERVER_ERROR); - return; - } - - // Send the pinger a successful stun response. - port->SendBindingResponse(stun_msg, address); - - // Update the list of connections since we just added another. We do this - // after sending the response since it could (in principle) delete the - // connection in question. - SortConnections(); } + + Connection* connection = port->CreateConnection( + remote_candidate, cricket::PortInterface::ORIGIN_THIS_PORT); + if (!connection) { + ASSERT(false); + port->SendBindingErrorResponse(stun_msg, address, + STUN_ERROR_SERVER_ERROR, + STUN_ERROR_REASON_SERVER_ERROR); + return; + } + + LOG(LS_INFO) << "Adding connection from " + << (remote_candidate_is_new ? "peer reflexive" : "resurrected") + << " candidate: " << remote_candidate.ToString(); + AddConnection(connection); + connection->ReceivedPing(); + + bool received_use_candidate = + stun_msg->GetByteString(STUN_ATTR_USE_CANDIDATE) != nullptr; + if (received_use_candidate && ice_role_ == ICEROLE_CONTROLLED) { + connection->set_nominated(true); + OnNominated(connection); + } + + // Update the list of connections since we just added another. We do this + // after sending the response since it could (in principle) delete the + // connection in question. + SortConnections(); } void P2PTransportChannel::OnRoleConflict(PortInterface* port) { @@ -650,7 +583,6 @@ void P2PTransportChannel::OnSignalingReady() { void P2PTransportChannel::OnNominated(Connection* conn) { ASSERT(worker_thread_ == rtc::Thread::Current()); ASSERT(ice_role_ == ICEROLE_CONTROLLED); - ASSERT(protocol_type_ == ICEPROTO_RFC5245); if (conn->write_state() == Connection::STATE_WRITABLE) { if (best_connection_ != conn) { @@ -809,13 +741,8 @@ bool P2PTransportChannel::FindConnection( uint32 P2PTransportChannel::GetRemoteCandidateGeneration( const Candidate& candidate) { - if (protocol_type_ == ICEPROTO_GOOGLE) { - // The Candidate.generation() can be trusted. Nothing needs to be done. - return candidate.generation(); - } - // |candidate.generation()| is not signaled in ICEPROTO_RFC5245. - // Therefore we need to keep track of the remote ice restart so - // newer connections are prioritized over the older. + // We need to keep track of the remote ice restart so newer + // connections are prioritized over the older. ASSERT(candidate.generation() == 0 || candidate.generation() == remote_candidate_generation_); return remote_candidate_generation_; @@ -993,15 +920,6 @@ void P2PTransportChannel::SortConnections() { // will be sorted. UpdateConnectionStates(); - if (protocol_type_ == ICEPROTO_HYBRID) { - // If we are in hybrid mode, we are not sending any ping requests, so there - // is no point in sorting the connections. In hybrid state, ports can have - // different protocol than hybrid and protocol may differ from one another. - // Instead just update the state of this channel - UpdateChannelState(); - return; - } - // Any changes after this point will require a re-sort. sort_dirty_ = false; @@ -1024,10 +942,7 @@ void P2PTransportChannel::SortConnections() { // connection although it will have higher priority if it is writable. // The controlled side can switch the best connection only if the current // |best connection_| has not been nominated by the controlling side yet. - - // We don't want to pick the best connections if channel is using RFC5245. - if ((protocol_type_ != ICEPROTO_RFC5245 || ice_role_ == ICEROLE_CONTROLLING || - !best_nominated_connection()) && + if ((ice_role_ == ICEROLE_CONTROLLING || !best_nominated_connection()) && ShouldSwitch(best_connection_, top_connection)) { LOG(LS_INFO) << "Switching best connection: " << top_connection->ToString(); SwitchBestConnectionTo(top_connection); @@ -1036,8 +951,7 @@ void P2PTransportChannel::SortConnections() { // Controlled side can prune only if the best connection has been nominated. // because otherwise it may delete the connection that will be selected by // the controlling side. - if (protocol_type_ != ICEPROTO_RFC5245 || ice_role_ == ICEROLE_CONTROLLING || - best_nominated_connection()) { + if (ice_role_ == ICEROLE_CONTROLLING || best_nominated_connection()) { PruneConnections(); } @@ -1302,8 +1216,7 @@ Connection* P2PTransportChannel::FindNextPingableConnection() { continue; } bool needs_triggered_check = - (protocol_type_ == ICEPROTO_RFC5245 && - !conn->writable() && + (!conn->writable() && conn->last_ping_received() > conn->last_ping_sent()); if (needs_triggered_check && (!oldest_needing_triggered_check || @@ -1338,15 +1251,13 @@ Connection* P2PTransportChannel::FindNextPingableConnection() { // b.2) |conn| is writable. void P2PTransportChannel::PingConnection(Connection* conn) { bool use_candidate = false; - if (protocol_type_ == ICEPROTO_RFC5245) { - if (remote_ice_mode_ == ICEMODE_FULL && ice_role_ == ICEROLE_CONTROLLING) { - use_candidate = (conn == best_connection_) || - (best_connection_ == NULL) || - (!best_connection_->writable()) || - (conn->priority() > best_connection_->priority()); - } else if (remote_ice_mode_ == ICEMODE_LITE && conn == best_connection_) { - use_candidate = best_connection_->writable(); - } + if (remote_ice_mode_ == ICEMODE_FULL && ice_role_ == ICEROLE_CONTROLLING) { + use_candidate = (conn == best_connection_) || + (best_connection_ == NULL) || + (!best_connection_->writable()) || + (conn->priority() > best_connection_->priority()); + } else if (remote_ice_mode_ == ICEMODE_LITE && conn == best_connection_) { + use_candidate = best_connection_->writable(); } conn->set_use_candidate_attr(use_candidate); conn->Ping(rtc::Time()); @@ -1359,7 +1270,7 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) { // Update the best connection if the state change is from pending best // connection and role is controlled. - if (protocol_type_ == ICEPROTO_RFC5245 && ice_role_ == ICEROLE_CONTROLLED) { + if (ice_role_ == ICEROLE_CONTROLLED) { if (connection == pending_best_connection_ && connection->writable()) { pending_best_connection_ = NULL; LOG(LS_INFO) << "Switching best connection on controlled side" diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 2f75b055a5..335ee7d166 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -63,8 +63,6 @@ class P2PTransportChannel : public TransportChannelImpl, virtual void SetIceRole(IceRole role); virtual IceRole GetIceRole() const { return ice_role_; } virtual void SetIceTiebreaker(uint64 tiebreaker); - virtual bool GetIceProtocolType(IceProtocolType* type) const; - virtual void SetIceProtocolType(IceProtocolType type); virtual void SetIceCredentials(const std::string& ice_ufrag, const std::string& ice_pwd); virtual void SetRemoteIceCredentials(const std::string& ice_ufrag, @@ -246,7 +244,6 @@ class P2PTransportChannel : public TransportChannelImpl, std::string ice_pwd_; std::string remote_ice_ufrag_; std::string remote_ice_pwd_; - IceProtocolType protocol_type_; IceMode remote_ice_mode_; IceRole ice_role_; uint64 tiebreaker_; diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 9065d1f5b7..3bb3be5ae8 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -32,7 +32,6 @@ using cricket::kDefaultPortAllocatorFlags; using cricket::kMinimumStepDelay; using cricket::kDefaultStepDelay; -using cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; using cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET; using cricket::ServerAddresses; using rtc::SocketAddress; @@ -177,6 +176,7 @@ class P2PTransportChannelTestBase : public testing::Test, local_type2(lt2), local_proto2(lp2), remote_type2(rt2), remote_proto2(rp2), connect_wait(wait) { } + std::string local_type; std::string local_proto; std::string remote_type; @@ -217,8 +217,7 @@ class P2PTransportChannelTestBase : public testing::Test, : role_(cricket::ICEROLE_UNKNOWN), tiebreaker_(0), role_conflict_(false), - save_candidates_(false), - protocol_type_(cricket::ICEPROTO_GOOGLE) {} + save_candidates_(false) {} bool HasChannel(cricket::TransportChannel* ch) { return (ch == cd1_.ch_.get() || ch == cd2_.ch_.get()); } @@ -232,10 +231,6 @@ class P2PTransportChannelTestBase : public testing::Test, void SetIceRole(cricket::IceRole role) { role_ = role; } cricket::IceRole ice_role() { return role_; } - void SetIceProtocolType(cricket::IceProtocolType type) { - protocol_type_ = type; - } - cricket::IceProtocolType protocol_type() { return protocol_type_; } void SetIceTiebreaker(uint64 tiebreaker) { tiebreaker_ = tiebreaker; } uint64 GetIceTiebreaker() { return tiebreaker_; } void OnRoleConflict(bool role_conflict) { role_conflict_ = role_conflict; } @@ -255,7 +250,6 @@ class P2PTransportChannelTestBase : public testing::Test, uint64 tiebreaker_; bool role_conflict_; bool save_candidates_; - cricket::IceProtocolType protocol_type_; std::vector saved_candidates_; }; @@ -311,7 +305,6 @@ class P2PTransportChannelTestBase : public testing::Test, this, &P2PTransportChannelTestBase::OnReadPacket); channel->SignalRoleConflict.connect( this, &P2PTransportChannelTestBase::OnRoleConflict); - channel->SetIceProtocolType(GetEndpoint(endpoint)->protocol_type()); channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd); if (clear_remote_candidates_ufrag_pwd_) { // This only needs to be set if we're clearing them from the @@ -379,9 +372,6 @@ class P2PTransportChannelTestBase : public testing::Test, void SetAllocatorFlags(int endpoint, int flags) { GetAllocator(endpoint)->set_flags(flags); } - void SetIceProtocol(int endpoint, cricket::IceProtocolType type) { - GetEndpoint(endpoint)->SetIceProtocolType(type); - } void SetIceRole(int endpoint, cricket::IceRole role) { GetEndpoint(endpoint)->SetIceRole(role); } @@ -398,6 +388,103 @@ class P2PTransportChannelTestBase : public testing::Test, return GetEndpoint(endpoint)->SetAllowTcpListen(allow_tcp_listen); } + bool IsLocalToPrflxOrTheReverse(const Result& expected) { + return ((expected.local_type == "local" && + expected.remote_type == "prflx") || + (expected.local_type == "prflx" && + expected.remote_type == "local")); + } + + // Return true if the approprite parts of the expected Result, based + // on the local and remote candidate of ep1_ch1, match. This can be + // used in an EXPECT_TRUE_WAIT. + bool CheckCandidate1(const Result& expected) { + const std::string& local_type = LocalCandidate(ep1_ch1())->type(); + const std::string& local_proto = LocalCandidate(ep1_ch1())->protocol(); + const std::string& remote_type = RemoteCandidate(ep1_ch1())->type(); + const std::string& remote_proto = RemoteCandidate(ep1_ch1())->protocol(); + return ((local_proto == expected.local_proto && + remote_proto == expected.remote_proto) && + ((local_type == expected.local_type && + remote_type == expected.remote_type) || + // Sometimes we expect local -> prflx or prflx -> local + // and instead get prflx -> local or local -> prflx, and + // that's OK. + (IsLocalToPrflxOrTheReverse(expected) && + local_type == expected.remote_type && + remote_type == expected.local_type))); + } + + // EXPECT_EQ on the approprite parts of the expected Result, based + // on the local and remote candidate of ep1_ch1. This is like + // CheckCandidate1, except that it will provide more detail about + // what didn't match. + void ExpectCandidate1(const Result& expected) { + if (CheckCandidate1(expected)) { + return; + } + + const std::string& local_type = LocalCandidate(ep1_ch1())->type(); + const std::string& local_proto = LocalCandidate(ep1_ch1())->protocol(); + const std::string& remote_type = RemoteCandidate(ep1_ch1())->type(); + const std::string& remote_proto = RemoteCandidate(ep1_ch1())->protocol(); + EXPECT_EQ(expected.local_type, local_type); + EXPECT_EQ(expected.remote_type, remote_type); + EXPECT_EQ(expected.local_proto, local_proto); + EXPECT_EQ(expected.remote_proto, remote_proto); + } + + // Return true if the approprite parts of the expected Result, based + // on the local and remote candidate of ep2_ch1, match. This can be + // used in an EXPECT_TRUE_WAIT. + bool CheckCandidate2(const Result& expected) { + const std::string& local_type = LocalCandidate(ep2_ch1())->type(); + // const std::string& remote_type = RemoteCandidate(ep2_ch1())->type(); + const std::string& local_proto = LocalCandidate(ep2_ch1())->protocol(); + const std::string& remote_proto = RemoteCandidate(ep2_ch1())->protocol(); + // Removed remote_type comparision aginst best connection remote + // candidate. This is done to handle remote type discrepancy from + // local to stun based on the test type. + // For example in case of Open -> NAT, ep2 channels will have LULU + // and in other cases like NAT -> NAT it will be LUSU. To avoid these + // mismatches and we are doing comparision in different way. + // i.e. when don't match its remote type is either local or stun. + // TODO(ronghuawu): Refine the test criteria. + // https://code.google.com/p/webrtc/issues/detail?id=1953 + return ((local_proto == expected.local_proto2 && + remote_proto == expected.remote_proto2) && + (local_type == expected.local_type2 || + // Sometimes we expect local -> prflx or prflx -> local + // and instead get prflx -> local or local -> prflx, and + // that's OK. + (IsLocalToPrflxOrTheReverse(expected) && + local_type == expected.remote_type2))); + } + + // EXPECT_EQ on the approprite parts of the expected Result, based + // on the local and remote candidate of ep2_ch1. This is like + // CheckCandidate2, except that it will provide more detail about + // what didn't match. + void ExpectCandidate2(const Result& expected) { + if (CheckCandidate2(expected)) { + return; + } + + const std::string& local_type = LocalCandidate(ep2_ch1())->type(); + const std::string& local_proto = LocalCandidate(ep2_ch1())->protocol(); + const std::string& remote_type = RemoteCandidate(ep2_ch1())->type(); + EXPECT_EQ(expected.local_proto2, local_proto); + EXPECT_EQ(expected.remote_proto2, remote_type); + EXPECT_EQ(expected.local_type2, local_type); + if (remote_type != expected.remote_type2) { + EXPECT_TRUE(expected.remote_type2 == cricket::LOCAL_PORT_TYPE || + expected.remote_type2 == cricket::STUN_PORT_TYPE); + EXPECT_TRUE(remote_type == cricket::LOCAL_PORT_TYPE || + remote_type == cricket::STUN_PORT_TYPE || + remote_type == cricket::PRFLX_PORT_TYPE); + } + } + void Test(const Result& expected) { int32 connect_start = rtc::Time(), connect_time; @@ -425,55 +512,20 @@ class P2PTransportChannelTestBase : public testing::Test, ep2_ch1()->best_connection()) { int32 converge_start = rtc::Time(), converge_time; int converge_wait = 2000; - EXPECT_TRUE_WAIT_MARGIN( - LocalCandidate(ep1_ch1())->type() == expected.local_type && - LocalCandidate(ep1_ch1())->protocol() == expected.local_proto && - RemoteCandidate(ep1_ch1())->type() == expected.remote_type && - RemoteCandidate(ep1_ch1())->protocol() == expected.remote_proto, - converge_wait, - converge_wait); - + EXPECT_TRUE_WAIT_MARGIN(CheckCandidate1(expected), + converge_wait, converge_wait); // Also do EXPECT_EQ on each part so that failures are more verbose. - EXPECT_EQ(expected.local_type, LocalCandidate(ep1_ch1())->type()); - EXPECT_EQ(expected.local_proto, LocalCandidate(ep1_ch1())->protocol()); - EXPECT_EQ(expected.remote_type, RemoteCandidate(ep1_ch1())->type()); - EXPECT_EQ(expected.remote_proto, RemoteCandidate(ep1_ch1())->protocol()); + ExpectCandidate1(expected); // Verifying remote channel best connection information. This is done // only for the RFC 5245 as controlled agent will use USE-CANDIDATE // from controlling (ep1) agent. We can easily predict from EP1 result // matrix. - if (ep2_.protocol_type_ == cricket::ICEPROTO_RFC5245) { - // Checking for best connection candidates information at remote. - EXPECT_TRUE_WAIT( - LocalCandidate(ep2_ch1())->type() == expected.local_type2 && - LocalCandidate(ep2_ch1())->protocol() == expected.local_proto2 && - RemoteCandidate(ep2_ch1())->protocol() == expected.remote_proto2, - kDefaultTimeout); - // For verbose - EXPECT_EQ(expected.local_type2, LocalCandidate(ep2_ch1())->type()); - EXPECT_EQ(expected.local_proto2, LocalCandidate(ep2_ch1())->protocol()); - EXPECT_EQ(expected.remote_proto2, - RemoteCandidate(ep2_ch1())->protocol()); - // Removed remote_type comparision aginst best connection remote - // candidate. This is done to handle remote type discrepancy from - // local to stun based on the test type. - // For example in case of Open -> NAT, ep2 channels will have LULU - // and in other cases like NAT -> NAT it will be LUSU. To avoid these - // mismatches and we are doing comparision in different way. - // i.e. when don't match its remote type is either local or stun. - // TODO(ronghuawu): Refine the test criteria. - // https://code.google.com/p/webrtc/issues/detail?id=1953 - if (expected.remote_type2 != RemoteCandidate(ep2_ch1())->type()) { - EXPECT_TRUE(expected.remote_type2 == cricket::LOCAL_PORT_TYPE || - expected.remote_type2 == cricket::STUN_PORT_TYPE); - EXPECT_TRUE( - RemoteCandidate(ep2_ch1())->type() == cricket::LOCAL_PORT_TYPE || - RemoteCandidate(ep2_ch1())->type() == cricket::STUN_PORT_TYPE || - RemoteCandidate(ep2_ch1())->type() == cricket::PRFLX_PORT_TYPE); - } - } + // Checking for best connection candidates information at remote. + EXPECT_TRUE_WAIT(CheckCandidate2(expected), kDefaultTimeout); + // For verbose + ExpectCandidate2(expected); converge_time = rtc::TimeSince(converge_start); if (converge_time < converge_wait) { @@ -550,10 +602,8 @@ class P2PTransportChannelTestBase : public testing::Test, } void TestSignalRoleConflict() { - SetIceProtocol(0, cricket::ICEPROTO_RFC5245); SetIceTiebreaker(0, kTiebreaker1); // Default EP1 is in controlling state. - SetIceProtocol(1, cricket::ICEPROTO_RFC5245); SetIceRole(1, cricket::ICEROLE_CONTROLLING); SetIceTiebreaker(1, kTiebreaker2); @@ -576,46 +626,6 @@ class P2PTransportChannelTestBase : public testing::Test, TestSendRecv(1); } - void TestHybridConnectivity(cricket::IceProtocolType proto) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - - SetAllocationStepDelay(0, kMinimumStepDelay); - SetAllocationStepDelay(1, kMinimumStepDelay); - - SetIceRole(0, cricket::ICEROLE_CONTROLLING); - SetIceProtocol(0, cricket::ICEPROTO_HYBRID); - SetIceTiebreaker(0, kTiebreaker1); - SetIceRole(1, cricket::ICEROLE_CONTROLLED); - SetIceProtocol(1, proto); - SetIceTiebreaker(1, kTiebreaker2); - - CreateChannels(1); - // When channel is in hybrid and it's controlling agent, channel will - // receive ping request from the remote. Hence connection is readable. - // Since channel is in hybrid, it will not send any pings, so no writable - // connection. Since channel2 is in controlled state, it will not have - // any connections which are readable or writable, as it didn't received - // pings (or none) with USE-CANDIDATE attribute. - EXPECT_TRUE_WAIT(ep1_ch1()->readable(), 1000); - - // Set real protocol type. - ep1_ch1()->SetIceProtocolType(proto); - - // Channel should able to send ping requests and connections become writable - // in both directions. - EXPECT_TRUE_WAIT(ep1_ch1()->readable() && ep1_ch1()->writable() && - ep2_ch1()->readable() && ep2_ch1()->writable(), - 1000); - EXPECT_TRUE( - ep1_ch1()->best_connection() && ep2_ch1()->best_connection() && - LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && - RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); - - TestSendRecv(1); - DestroyChannels(); - } - void OnChannelRequestSignaling(cricket::TransportChannelImpl* channel) { channel->OnSignalingReady(); } @@ -795,12 +805,7 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { static const Result* kMatrixSharedSocketAsGice[NUM_CONFIGS][NUM_CONFIGS]; static const Result* kMatrixSharedSocketAsIce[NUM_CONFIGS][NUM_CONFIGS]; void ConfigureEndpoints(Config config1, Config config2, - int allocator_flags1, int allocator_flags2, - int delay1, int delay2, - cricket::IceProtocolType type) { - // 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. + int allocator_flags1, int allocator_flags2) { ServerAddresses stun_servers; stun_servers.insert(kStunAddr); GetEndpoint(0)->allocator_.reset( @@ -814,35 +819,22 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { rtc::SocketAddress(), rtc::SocketAddress(), rtc::SocketAddress())); - cricket::RelayServerConfig relay_server(cricket::RELAY_GTURN); - if (type == cricket::ICEPROTO_RFC5245) { - relay_server.type = cricket::RELAY_TURN; - relay_server.credentials = kRelayCredentials; - relay_server.ports.push_back(cricket::ProtocolAddress( - kTurnUdpIntAddr, cricket::PROTO_UDP, false)); - } else { - relay_server.ports.push_back(cricket::ProtocolAddress( - kRelayUdpIntAddr, cricket::PROTO_UDP, false)); - relay_server.ports.push_back(cricket::ProtocolAddress( - kRelayTcpIntAddr, cricket::PROTO_TCP, false)); - relay_server.ports.push_back(cricket::ProtocolAddress( - kRelaySslTcpIntAddr, cricket::PROTO_SSLTCP, false)); - } + cricket::RelayServerConfig relay_server(cricket::RELAY_TURN); + relay_server.credentials = kRelayCredentials; + relay_server.ports.push_back(cricket::ProtocolAddress( + kTurnUdpIntAddr, cricket::PROTO_UDP, false)); GetEndpoint(0)->allocator_->AddRelay(relay_server); GetEndpoint(1)->allocator_->AddRelay(relay_server); + int delay = kMinimumStepDelay; ConfigureEndpoint(0, config1); - SetIceProtocol(0, type); SetAllocatorFlags(0, allocator_flags1); - SetAllocationStepDelay(0, delay1); + SetAllocationStepDelay(0, delay); ConfigureEndpoint(1, config2); - SetIceProtocol(1, type); SetAllocatorFlags(1, allocator_flags2); - SetAllocationStepDelay(1, delay2); + SetAllocationStepDelay(1, delay); - if (type == cricket::ICEPROTO_RFC5245) { - set_clear_remote_candidates_ufrag_pwd(true); - } + set_clear_remote_candidates_ufrag_pwd(true); } void ConfigureEndpoint(int endpoint, Config config) { switch (config) { @@ -1034,85 +1026,11 @@ const P2PTransportChannelTest::Result* // The actual tests that exercise all the various configurations. // Test names are of the form P2PTransportChannelTest_TestOPENToNAT_FULL_CONE -// Same test case is run in both GICE and ICE mode. -// kDefaultStepDelay - is used for all Gice cases. -// kMinimumStepDelay - is used when both end points have -// PORTALLOCATOR_ENABLE_SHARED_UFRAG flag enabled. -// Technically we should be able to use kMinimumStepDelay irrespective of -// protocol type. But which might need modifications to current result matrices -// for tests in this file. #define P2P_TEST_DECLARATION(x, y, z) \ - TEST_F(P2PTransportChannelTest, z##Test##x##To##y##AsGiceNoneSharedUfrag) { \ - ConfigureEndpoints(x, y, kDefaultPortAllocatorFlags, \ - kDefaultPortAllocatorFlags, \ - kDefaultStepDelay, kDefaultStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrix[x][y] != NULL) \ - Test(*kMatrix[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, z##Test##x##To##y##AsGiceP0SharedUfrag) { \ - ConfigureEndpoints(x, y, PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - kDefaultPortAllocatorFlags, \ - kDefaultStepDelay, kDefaultStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrix[x][y] != NULL) \ - Test(*kMatrix[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, z##Test##x##To##y##AsGiceP1SharedUfrag) { \ - ConfigureEndpoints(x, y, kDefaultPortAllocatorFlags, \ - PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - kDefaultStepDelay, kDefaultStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrixSharedUfrag[x][y] != NULL) \ - Test(*kMatrixSharedUfrag[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, z##Test##x##To##y##AsGiceBothSharedUfrag) { \ - ConfigureEndpoints(x, y, PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - kDefaultStepDelay, kDefaultStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrixSharedUfrag[x][y] != NULL) \ - Test(*kMatrixSharedUfrag[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, \ - z##Test##x##To##y##AsGiceBothSharedUfragWithMinimumStepDelay) { \ - ConfigureEndpoints(x, y, PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - PORTALLOCATOR_ENABLE_SHARED_UFRAG, \ - kMinimumStepDelay, kMinimumStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrixSharedUfrag[x][y] != NULL) \ - Test(*kMatrixSharedUfrag[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, \ - z##Test##x##To##y##AsGiceBothSharedUfragSocket) { \ - ConfigureEndpoints(x, y, PORTALLOCATOR_ENABLE_SHARED_UFRAG | \ + TEST_F(P2PTransportChannelTest, z##Test##x##To##y) { \ + ConfigureEndpoints(x, y, \ PORTALLOCATOR_ENABLE_SHARED_SOCKET, \ - PORTALLOCATOR_ENABLE_SHARED_UFRAG | \ - PORTALLOCATOR_ENABLE_SHARED_SOCKET, \ - kMinimumStepDelay, kMinimumStepDelay, \ - cricket::ICEPROTO_GOOGLE); \ - if (kMatrixSharedSocketAsGice[x][y] != NULL) \ - Test(*kMatrixSharedSocketAsGice[x][y]); \ - else \ - LOG(LS_WARNING) << "Not yet implemented"; \ - } \ - TEST_F(P2PTransportChannelTest, z##Test##x##To##y##AsIce) { \ - ConfigureEndpoints(x, y, PORTALLOCATOR_ENABLE_SHARED_UFRAG | \ - PORTALLOCATOR_ENABLE_SHARED_SOCKET, \ - PORTALLOCATOR_ENABLE_SHARED_UFRAG | \ - PORTALLOCATOR_ENABLE_SHARED_SOCKET, \ - kMinimumStepDelay, kMinimumStepDelay, \ - cricket::ICEPROTO_RFC5245); \ + PORTALLOCATOR_ENABLE_SHARED_SOCKET); \ if (kMatrixSharedSocketAsIce[x][y] != NULL) \ Test(*kMatrixSharedSocketAsIce[x][y]); \ else \ @@ -1170,25 +1088,10 @@ P2P_TEST_SET(PROXY_SOCKS) // Test that we restart candidate allocation when local ufrag&pwd changed. // Standard Ice protocol is used. -TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeAsIce) { +TEST_F(P2PTransportChannelTest, HandleUfragPwdChange) { ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kMinimumStepDelay, kMinimumStepDelay, - cricket::ICEPROTO_RFC5245); - CreateChannels(1); - TestHandleIceUfragPasswordChanged(); - DestroyChannels(); -} - -// Test that we restart candidate allocation when local ufrag&pwd changed. -// Google Ice protocol is used. -TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeAsGice) { - ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); + kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); CreateChannels(1); TestHandleIceUfragPasswordChanged(); DestroyChannels(); @@ -1198,9 +1101,7 @@ TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeAsGice) { TEST_F(P2PTransportChannelTest, GetStats) { ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, - kDefaultPortAllocatorFlags, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); + kDefaultPortAllocatorFlags); CreateChannels(1); EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->readable() && ep1_ch1()->writable() && ep2_ch1()->readable() && ep2_ch1()->writable(), @@ -1226,10 +1127,8 @@ TEST_F(P2PTransportChannelTest, GetStats) { // when the signaling is slow. TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_RFC5245); + kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); // Emulate no remote credentials coming in. set_clear_remote_candidates_ufrag_pwd(false); CreateChannels(1); @@ -1273,10 +1172,8 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { // when the signaling is slow and the end points are behind NAT. TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { ConfigureEndpoints(OPEN, NAT_SYMMETRIC, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_RFC5245); + kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); // Emulate no remote credentials coming in. set_clear_remote_candidates_ufrag_pwd(false); CreateChannels(1); @@ -1318,10 +1215,8 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) { set_clear_remote_candidates_ufrag_pwd(true); ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kMinimumStepDelay, kMinimumStepDelay, - cricket::ICEPROTO_GOOGLE); + kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); CreateChannels(1); const cricket::Connection* best_connection = NULL; // Wait until the callee's connections are created. @@ -1337,9 +1232,7 @@ TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) { TEST_F(P2PTransportChannelTest, IncomingOnlyBlocked) { ConfigureEndpoints(NAT_FULL_CONE, OPEN, kDefaultPortAllocatorFlags, - kDefaultPortAllocatorFlags, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); + kDefaultPortAllocatorFlags); SetAllocatorFlags(0, kOnlyLocalPorts); CreateChannels(1); @@ -1361,9 +1254,7 @@ TEST_F(P2PTransportChannelTest, IncomingOnlyBlocked) { TEST_F(P2PTransportChannelTest, IncomingOnlyOpen) { ConfigureEndpoints(OPEN, NAT_FULL_CONE, kDefaultPortAllocatorFlags, - kDefaultPortAllocatorFlags, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); + kDefaultPortAllocatorFlags); SetAllocatorFlags(0, kOnlyLocalPorts); CreateChannels(1); @@ -1386,8 +1277,7 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { int kOnlyLocalTcpPorts = cricket::PORTALLOCATOR_DISABLE_UDP | cricket::PORTALLOCATOR_DISABLE_STUN | - cricket::PORTALLOCATOR_DISABLE_RELAY | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; + cricket::PORTALLOCATOR_DISABLE_RELAY; // Disable all protocols except TCP. SetAllocatorFlags(0, kOnlyLocalTcpPorts); SetAllocatorFlags(1, kOnlyLocalTcpPorts); @@ -1428,10 +1318,8 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { AddAddress(1, kPublicAddrs[1]); SetIceRole(0, cricket::ICEROLE_CONTROLLING); - SetIceProtocol(0, cricket::ICEPROTO_GOOGLE); SetIceTiebreaker(0, kTiebreaker1); SetIceRole(1, cricket::ICEROLE_CONTROLLING); - SetIceProtocol(1, cricket::ICEPROTO_RFC5245); SetIceTiebreaker(1, kTiebreaker2); CreateChannels(1); @@ -1441,18 +1329,15 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { const std::vector ports_before = ep1_ch1()->ports(); for (size_t i = 0; i < ports_before.size(); ++i) { EXPECT_EQ(cricket::ICEROLE_CONTROLLING, ports_before[i]->GetIceRole()); - EXPECT_EQ(cricket::ICEPROTO_GOOGLE, ports_before[i]->IceProtocol()); EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); } ep1_ch1()->SetIceRole(cricket::ICEROLE_CONTROLLED); - ep1_ch1()->SetIceProtocolType(cricket::ICEPROTO_RFC5245); ep1_ch1()->SetIceTiebreaker(kTiebreaker2); const std::vector ports_after = ep1_ch1()->ports(); for (size_t i = 0; i < ports_after.size(); ++i) { EXPECT_EQ(cricket::ICEROLE_CONTROLLED, ports_before[i]->GetIceRole()); - EXPECT_EQ(cricket::ICEPROTO_RFC5245, ports_before[i]->IceProtocol()); // SetIceTiebreaker after Connect() has been called will fail. So expect the // original value. EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); @@ -1471,18 +1356,6 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { DestroyChannels(); } -// This test verifies channel can handle ice messages when channel is in -// hybrid mode. -TEST_F(P2PTransportChannelTest, TestConnectivityBetweenHybridandIce) { - TestHybridConnectivity(cricket::ICEPROTO_RFC5245); -} - -// This test verifies channel can handle Gice messages when channel is in -// hybrid mode. -TEST_F(P2PTransportChannelTest, TestConnectivityBetweenHybridandGice) { - TestHybridConnectivity(cricket::ICEPROTO_GOOGLE); -} - // Verify that we can set DSCP value and retrieve properly from P2PTC. TEST_F(P2PTransportChannelTest, TestDefaultDscpValue) { AddAddress(0, kPublicAddrs[0]); @@ -1543,13 +1416,9 @@ TEST_F(P2PTransportChannelTest, TestIPv6Connections) { TEST_F(P2PTransportChannelTest, TestForceTurn) { ConfigureEndpoints(NAT_PORT_RESTRICTED, NAT_SYMMETRIC, kDefaultPortAllocatorFlags | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG, + cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET, kDefaultPortAllocatorFlags | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_RFC5245); + cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); set_force_relay(true); SetAllocationStepDelay(0, kMinimumStepDelay); @@ -1561,7 +1430,7 @@ TEST_F(P2PTransportChannelTest, TestForceTurn) { ep1_ch1()->writable() && ep2_ch1()->readable() && ep2_ch1()->writable(), - 1000); + 2000); EXPECT_TRUE(ep1_ch1()->best_connection() && ep2_ch1()->best_connection()); @@ -1605,7 +1474,9 @@ class P2PTransportChannelSameNatTest : public P2PTransportChannelTestBase { TEST_F(P2PTransportChannelSameNatTest, TestConesBehindSameCone) { ConfigureEndpoints(NAT_FULL_CONE, NAT_FULL_CONE, NAT_FULL_CONE); - Test(kLocalUdpToStunUdp); + Test(P2PTransportChannelTestBase::Result( + "prflx", "udp", "stun", "udp", + "stun", "udp", "prflx", "udp", 1000)); } // Test what happens when we have multiple available pathways. @@ -1664,6 +1535,13 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailover) { DestroyChannels(); } +/* + +TODO(pthatcher): Once have a way to handle network interfaces changes +without signalling an ICE restart, put a test like this back. In the +mean time, this test only worked for GICE. With ICE, it's currently +not possible without an ICE restart. + // Test that we can switch links in a coordinated fashion. TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { AddAddress(0, kPublicAddrs[0]); @@ -1682,6 +1560,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); + // Remove the public interface, add the alternate interface, and allocate // a new generation of candidates for the new interface (via Connect()). LOG(LS_INFO) << "Draining..."; @@ -1700,6 +1579,8 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { DestroyChannels(); } +*/ + // A collection of tests which tests a single P2PTransportChannel by sending // pings. class P2PTransportChannelPingTest : public testing::Test, @@ -1714,7 +1595,6 @@ class P2PTransportChannelPingTest : public testing::Test, void PrepareChannel(cricket::P2PTransportChannel* ch) { ch->SignalRequestSignaling.connect( this, &P2PTransportChannelPingTest::OnChannelRequestSignaling); - ch->SetIceProtocolType(cricket::ICEPROTO_RFC5245); ch->SetIceRole(cricket::ICEROLE_CONTROLLING); ch->SetIceCredentials(kIceUfrag[0], kIcePwd[0]); ch->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index eb63b02c86..8048201754 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -58,27 +58,6 @@ inline bool TooLongWithoutResponse( return now > (first.sent_time + maximum_time); } -// GICE(ICEPROTO_GOOGLE) requires different username for RTP and RTCP. -// This function generates a different username by +1 on the last character of -// the given username (|rtp_ufrag|). -std::string GetRtcpUfragFromRtpUfrag(const std::string& rtp_ufrag) { - ASSERT(!rtp_ufrag.empty()); - if (rtp_ufrag.empty()) { - return rtp_ufrag; - } - // Change the last character to the one next to it in the base64 table. - char new_last_char; - if (!rtc::Base64::GetNextBase64Char(rtp_ufrag[rtp_ufrag.size() - 1], - &new_last_char)) { - // Should not be here. - ASSERT(false); - } - std::string rtcp_ufrag = rtp_ufrag; - rtcp_ufrag[rtcp_ufrag.size() - 1] = new_last_char; - ASSERT(rtcp_ufrag != rtp_ufrag); - return rtcp_ufrag; -} - // We will restrict RTT estimates (when used for determining state) to be // within a reasonable range. const uint32 MINIMUM_RTT = 100; // 0.1 seconds @@ -171,7 +150,6 @@ Port::Port(rtc::Thread* thread, password_(password), timeout_delay_(kPortTimeoutDelay), enable_port_packets_(false), - ice_protocol_(ICEPROTO_HYBRID), ice_role_(ICEROLE_UNKNOWN), tiebreaker_(0), shared_socket_(true), @@ -202,7 +180,6 @@ Port::Port(rtc::Thread* thread, password_(password), timeout_delay_(kPortTimeoutDelay), enable_port_packets_(false), - ice_protocol_(ICEPROTO_HYBRID), ice_role_(ICEROLE_UNKNOWN), tiebreaker_(0), shared_socket_(false), @@ -212,7 +189,9 @@ Port::Port(rtc::Thread* thread, } void Port::Construct() { - // If the username_fragment and password are empty, we should just create one. + // TODO(pthatcher): Remove this old behavior once we're sure no one + // relies on it. If the username_fragment and password are empty, + // we should just create one. if (ice_username_fragment_.empty()) { ASSERT(password_.empty()); ice_username_fragment_ = rtc::CreateRandomString(ICE_UFRAG_LENGTH); @@ -314,8 +293,7 @@ void Port::OnReadPacket( << " from unknown address " << addr.ToSensitiveString(); // Check for role conflicts. - if (IsStandardIce() && - !MaybeIceRoleConflict(addr, msg.get(), remote_username)) { + if (!MaybeIceRoleConflict(addr, msg.get(), remote_username)) { LOG(LS_INFO) << "Received conflicting role from the peer."; return; } @@ -346,18 +324,6 @@ size_t Port::AddPrflxCandidate(const Candidate& local) { return (candidates_.size() - 1); } -bool Port::IsStandardIce() const { - return (ice_protocol_ == ICEPROTO_RFC5245); -} - -bool Port::IsGoogleIce() const { - return (ice_protocol_ == ICEPROTO_GOOGLE); -} - -bool Port::IsHybridIce() const { - return (ice_protocol_ == ICEPROTO_HYBRID); -} - bool Port::GetStunMessage(const char* data, size_t size, const rtc::SocketAddress& addr, IceMessage** out_msg, std::string* out_username) { @@ -371,7 +337,7 @@ bool Port::GetStunMessage(const char* data, size_t size, // Don't bother parsing the packet if we can tell it's not STUN. // In ICE mode, all STUN packets will have a valid fingerprint. - if (IsStandardIce() && !StunMessage::ValidateFingerprint(data, size)) { + if (!StunMessage::ValidateFingerprint(data, size)) { return false; } @@ -387,8 +353,7 @@ bool Port::GetStunMessage(const char* data, size_t size, // Check for the presence of USERNAME and MESSAGE-INTEGRITY (if ICE) first. // If not present, fail with a 400 Bad Request. if (!stun_msg->GetByteString(STUN_ATTR_USERNAME) || - (IsStandardIce() && - !stun_msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY))) { + !stun_msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY)) { LOG_J(LS_ERROR, this) << "Received STUN request without username/M-I " << "from " << addr.ToSensitiveString(); SendBindingErrorResponse(stun_msg.get(), addr, STUN_ERROR_BAD_REQUEST, @@ -399,9 +364,7 @@ bool Port::GetStunMessage(const char* data, size_t size, // If the username is bad or unknown, fail with a 401 Unauthorized. std::string local_ufrag; std::string remote_ufrag; - IceProtocolType remote_protocol_type; - if (!ParseStunUsername(stun_msg.get(), &local_ufrag, &remote_ufrag, - &remote_protocol_type) || + if (!ParseStunUsername(stun_msg.get(), &local_ufrag, &remote_ufrag) || local_ufrag != username_fragment()) { LOG_J(LS_ERROR, this) << "Received STUN request with bad local username " << local_ufrag << " from " @@ -411,18 +374,8 @@ bool Port::GetStunMessage(const char* data, size_t size, return true; } - // Port is initialized to GOOGLE-ICE protocol type. If pings from remote - // are received before the signal message, protocol type may be different. - // Based on the STUN username, we can determine what's the remote protocol. - // This also enables us to send the response back using the same protocol - // as the request. - if (IsHybridIce()) { - SetIceProtocolType(remote_protocol_type); - } - // If ICE, and the MESSAGE-INTEGRITY is bad, fail with a 401 Unauthorized - if (IsStandardIce() && - !stun_msg->ValidateMessageIntegrity(data, size, password_)) { + if (!stun_msg->ValidateMessageIntegrity(data, size, password_)) { LOG_J(LS_ERROR, this) << "Received STUN request with bad M-I " << "from " << addr.ToSensitiveString() << ", password_=" << password_; @@ -483,8 +436,7 @@ bool Port::IsCompatibleAddress(const rtc::SocketAddress& addr) { bool Port::ParseStunUsername(const StunMessage* stun_msg, std::string* local_ufrag, - std::string* remote_ufrag, - IceProtocolType* remote_protocol_type) const { + std::string* remote_ufrag) const { // The packet must include a username that either begins or ends with our // fragment. It should begin with our fragment if it is a request and it // should end with our fragment if it is a response. @@ -495,34 +447,15 @@ bool Port::ParseStunUsername(const StunMessage* stun_msg, if (username_attr == NULL) return false; - const std::string username_attr_str = username_attr->GetString(); - size_t colon_pos = username_attr_str.find(":"); - // If we are in hybrid mode set the appropriate ice protocol type based on - // the username argument style. - if (IsHybridIce()) { - *remote_protocol_type = (colon_pos != std::string::npos) ? - ICEPROTO_RFC5245 : ICEPROTO_GOOGLE; - } else { - *remote_protocol_type = ice_protocol_; + // RFRAG:LFRAG + const std::string username = username_attr->GetString(); + size_t colon_pos = username.find(":"); + if (colon_pos == std::string::npos) { + return false; } - if (*remote_protocol_type == ICEPROTO_RFC5245) { - if (colon_pos != std::string::npos) { // RFRAG:LFRAG - *local_ufrag = username_attr_str.substr(0, colon_pos); - *remote_ufrag = username_attr_str.substr( - colon_pos + 1, username_attr_str.size()); - } else { - return false; - } - } else if (*remote_protocol_type == ICEPROTO_GOOGLE) { - int remote_frag_len = static_cast(username_attr_str.size()); - remote_frag_len -= static_cast(username_fragment().size()); - if (remote_frag_len < 0) - return false; - *local_ufrag = username_attr_str.substr(0, username_fragment().size()); - *remote_ufrag = username_attr_str.substr( - username_fragment().size(), username_attr_str.size()); - } + *local_ufrag = username.substr(0, colon_pos); + *remote_ufrag = username.substr(colon_pos + 1, username.size()); return true; } @@ -591,10 +524,7 @@ void Port::CreateStunUsername(const std::string& remote_username, std::string* stun_username_attr_str) const { stun_username_attr_str->clear(); *stun_username_attr_str = remote_username; - if (IsStandardIce()) { - // Connectivity checks from L->R will have username RFRAG:LFRAG. - stun_username_attr_str->append(":"); - } + stun_username_attr_str->append(":"); stun_username_attr_str->append(username_fragment()); } @@ -630,19 +560,10 @@ void Port::SendBindingResponse(StunMessage* request, } } - // Only GICE messages have USERNAME and MAPPED-ADDRESS in the response. - // ICE messages use XOR-MAPPED-ADDRESS, and add MESSAGE-INTEGRITY. - if (IsStandardIce()) { - response.AddAttribute( - new StunXorAddressAttribute(STUN_ATTR_XOR_MAPPED_ADDRESS, addr)); - response.AddMessageIntegrity(password_); - response.AddFingerprint(); - } else if (IsGoogleIce()) { - response.AddAttribute( - new StunAddressAttribute(STUN_ATTR_MAPPED_ADDRESS, addr)); - response.AddAttribute(new StunByteStringAttribute( - STUN_ATTR_USERNAME, username_attr->GetString())); - } + response.AddAttribute( + new StunXorAddressAttribute(STUN_ATTR_XOR_MAPPED_ADDRESS, addr)); + response.AddMessageIntegrity(password_); + response.AddFingerprint(); // The fact that we received a successful request means that this connection // (if one exists) should now be readable. @@ -688,30 +609,16 @@ void Port::SendBindingErrorResponse(StunMessage* request, // When doing GICE, we need to write out the error code incorrectly to // maintain backwards compatiblility. StunErrorCodeAttribute* error_attr = StunAttribute::CreateErrorCode(); - if (IsStandardIce()) { - error_attr->SetCode(error_code); - } else if (IsGoogleIce()) { - error_attr->SetClass(error_code / 256); - error_attr->SetNumber(error_code % 256); - } + error_attr->SetCode(error_code); error_attr->SetReason(reason); response.AddAttribute(error_attr); - if (IsStandardIce()) { - // Per Section 10.1.2, certain error cases don't get a MESSAGE-INTEGRITY, - // because we don't have enough information to determine the shared secret. - if (error_code != STUN_ERROR_BAD_REQUEST && - error_code != STUN_ERROR_UNAUTHORIZED) - response.AddMessageIntegrity(password_); - response.AddFingerprint(); - } else if (IsGoogleIce()) { - // GICE responses include a username, if one exists. - const StunByteStringAttribute* username_attr = - request->GetByteString(STUN_ATTR_USERNAME); - if (username_attr) - response.AddAttribute(new StunByteStringAttribute( - STUN_ATTR_USERNAME, username_attr->GetString())); - } + // Per Section 10.1.2, certain error cases don't get a MESSAGE-INTEGRITY, + // because we don't have enough information to determine the shared secret. + if (error_code != STUN_ERROR_BAD_REQUEST && + error_code != STUN_ERROR_UNAUTHORIZED) + response.AddMessageIntegrity(password_); + response.AddFingerprint(); // Send the response message. rtc::ByteBuffer buf; @@ -771,13 +678,7 @@ void Port::CheckTimeout() { } const std::string Port::username_fragment() const { - if (!IsStandardIce() && - component_ == ICE_CANDIDATE_COMPONENT_RTCP) { - // In GICE mode, we should adjust username fragment for rtcp component. - return GetRtcpUfragFromRtpUfrag(ice_username_fragment_); - } else { - return ice_username_fragment_; - } + return ice_username_fragment_; } // A ConnectionRequest is a simple STUN ping used to determine writability. @@ -807,44 +708,41 @@ class ConnectionRequest : public StunRequest { connection_->pings_since_last_response_.size() - 1))); } - // Adding ICE-specific attributes to the STUN request message. - if (connection_->port()->IsStandardIce()) { - // Adding ICE_CONTROLLED or ICE_CONTROLLING attribute based on the role. - if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLING) { - request->AddAttribute(new StunUInt64Attribute( - STUN_ATTR_ICE_CONTROLLING, connection_->port()->IceTiebreaker())); - // Since we are trying aggressive nomination, sending USE-CANDIDATE - // attribute in every ping. - // If we are dealing with a ice-lite end point, nomination flag - // in Connection will be set to false by default. Once the connection - // becomes "best connection", nomination flag will be turned on. - if (connection_->use_candidate_attr()) { - request->AddAttribute(new StunByteStringAttribute( - STUN_ATTR_USE_CANDIDATE)); - } - } else if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLED) { - request->AddAttribute(new StunUInt64Attribute( - STUN_ATTR_ICE_CONTROLLED, connection_->port()->IceTiebreaker())); - } else { - ASSERT(false); + // Adding ICE_CONTROLLED or ICE_CONTROLLING attribute based on the role. + if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLING) { + request->AddAttribute(new StunUInt64Attribute( + STUN_ATTR_ICE_CONTROLLING, connection_->port()->IceTiebreaker())); + // Since we are trying aggressive nomination, sending USE-CANDIDATE + // attribute in every ping. + // If we are dealing with a ice-lite end point, nomination flag + // in Connection will be set to false by default. Once the connection + // becomes "best connection", nomination flag will be turned on. + if (connection_->use_candidate_attr()) { + request->AddAttribute(new StunByteStringAttribute( + STUN_ATTR_USE_CANDIDATE)); } - - // Adding PRIORITY Attribute. - // Changing the type preference to Peer Reflexive and local preference - // and component id information is unchanged from the original priority. - // priority = (2^24)*(type preference) + - // (2^8)*(local preference) + - // (2^0)*(256 - component ID) - uint32 prflx_priority = ICE_TYPE_PREFERENCE_PRFLX << 24 | - (connection_->local_candidate().priority() & 0x00FFFFFF); - request->AddAttribute( - new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority)); - - // Adding Message Integrity attribute. - request->AddMessageIntegrity(connection_->remote_candidate().password()); - // Adding Fingerprint. - request->AddFingerprint(); + } else if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLED) { + request->AddAttribute(new StunUInt64Attribute( + STUN_ATTR_ICE_CONTROLLED, connection_->port()->IceTiebreaker())); + } else { + ASSERT(false); } + + // Adding PRIORITY Attribute. + // Changing the type preference to Peer Reflexive and local preference + // and component id information is unchanged from the original priority. + // priority = (2^24)*(type preference) + + // (2^8)*(local preference) + + // (2^0)*(256 - component ID) + uint32 prflx_priority = ICE_TYPE_PREFERENCE_PRFLX << 24 | + (connection_->local_candidate().priority() & 0x00FFFFFF); + request->AddAttribute( + new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority)); + + // Adding Message Integrity attribute. + request->AddMessageIntegrity(connection_->remote_candidate().password()); + // Adding Fingerprint. + request->AddFingerprint(); } void OnResponse(StunMessage* response) override { @@ -1040,8 +938,7 @@ void Connection::OnReadPacket( if (remote_ufrag == remote_candidate_.username()) { // Check for role conflicts. - if (port_->IsStandardIce() && - !port_->MaybeIceRoleConflict(addr, msg.get(), remote_ufrag)) { + if (!port_->MaybeIceRoleConflict(addr, msg.get(), remote_ufrag)) { // Received conflicting role from the peer. LOG(LS_INFO) << "Received conflicting role from the peer."; return; @@ -1055,8 +952,7 @@ void Connection::OnReadPacket( if (!pruned_ && (write_state_ == STATE_WRITE_TIMEOUT)) set_write_state(STATE_WRITE_INIT); - if ((port_->IsStandardIce()) && - (port_->GetIceRole() == ICEROLE_CONTROLLED)) { + if (port_->GetIceRole() == ICEROLE_CONTROLLED) { const StunByteStringAttribute* use_candidate_attr = msg->GetByteString(STUN_ATTR_USE_CANDIDATE); if (use_candidate_attr) { @@ -1082,8 +978,7 @@ void Connection::OnReadPacket( // id's match. case STUN_BINDING_RESPONSE: case STUN_BINDING_ERROR_RESPONSE: - if (port_->IsGoogleIce() || - msg->ValidateMessageIntegrity( + if (msg->ValidateMessageIntegrity( data, size, remote_candidate().password())) { requests_.CheckResponse(msg.get()); } @@ -1095,7 +990,7 @@ void Connection::OnReadPacket( // Otherwise we can mark connection to read timeout. No response will be // sent in this scenario. case STUN_BINDING_INDICATION: - if (port_->IsStandardIce() && read_state_ == STATE_READABLE) { + if (read_state_ == STATE_READABLE) { ReceivedPing(); } else { LOG_J(LS_WARNING, this) << "Received STUN binding indication " @@ -1163,30 +1058,6 @@ void Connection::UpdateState(uint32 now) { << ", pings_since_last_response=" << pings; } - // Check the readable state. - // - // Since we don't know how many pings the other side has attempted, the best - // test we can do is a simple window. - // If other side has not sent ping after connection has become readable, use - // |last_data_received_| as the indication. - // If remote endpoint is doing RFC 5245, it's not required to send ping - // after connection is established. If this connection is serving a data - // channel, it may not be in a position to send media continuously. Do not - // mark connection timeout if it's in RFC5245 mode. - // Below check will be performed with end point if it's doing google-ice. - if (port_->IsGoogleIce() && (read_state_ == STATE_READABLE) && - (last_ping_received_ + CONNECTION_READ_TIMEOUT <= now) && - (last_data_received_ + CONNECTION_READ_TIMEOUT <= now)) { - LOG_J(LS_INFO, this) << "Unreadable after " << now - last_ping_received_ - << " ms without a ping," - << " ms since last received response=" - << now - last_ping_response_received_ - << " ms since last received data=" - << now - last_data_received_ - << " rtt=" << rtt; - set_read_state(STATE_READ_TIMEOUT); - } - // Check the writable state. (The order of these checks is important.) // // Before becoming unwritable, we allow for a fixed number of pings to fail @@ -1345,10 +1216,7 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request, rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1); - // Peer reflexive candidate is only for RFC 5245 ICE. - if (port_->IsStandardIce()) { - MaybeAddPrflxCandidate(request, response); - } + MaybeAddPrflxCandidate(request, response); } void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request, @@ -1356,13 +1224,7 @@ void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request, const StunErrorCodeAttribute* error_attr = response->GetErrorCode(); int error_code = STUN_ERROR_GLOBAL_FAILURE; if (error_attr) { - if (port_->IsGoogleIce()) { - // When doing GICE, the error code is written out incorrectly, so we need - // to unmunge it here. - error_code = error_attr->eclass() * 256 + error_attr->number(); - } else { - error_code = error_attr->code(); - } + error_code = error_attr->code(); } LOG_J(LS_INFO, this) << "Received STUN error response" diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index bb054e5341..fbda9cea01 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -127,16 +127,6 @@ class Port : public PortInterface, public rtc::MessageHandler, virtual const std::string& Type() const { return type_; } virtual rtc::Network* Network() const { return network_; } - // This method will set the flag which enables standard ICE/STUN procedures - // in STUN connectivity checks. Currently this method does - // 1. Add / Verify MI attribute in STUN binding requests. - // 2. Username attribute in STUN binding request will be RFRAF:LFRAG, - // as opposed to RFRAGLFRAG. - virtual void SetIceProtocolType(IceProtocolType protocol) { - ice_protocol_ = protocol; - } - virtual IceProtocolType IceProtocol() const { return ice_protocol_; } - // Methods to set/get ICE role and tiebreaker values. IceRole GetIceRole() const { return ice_role_; } void SetIceRole(IceRole role) { ice_role_ = role; } @@ -273,8 +263,7 @@ class Port : public PortInterface, public rtc::MessageHandler, // stun username attribute if present. bool ParseStunUsername(const StunMessage* stun_msg, std::string* local_username, - std::string* remote_username, - IceProtocolType* remote_protocol_type) const; + std::string* remote_username) const; void CreateStunUsername(const std::string& remote_username, std::string* stun_username_attr_str) const; @@ -289,15 +278,6 @@ class Port : public PortInterface, public rtc::MessageHandler, // Returns the index of the new local candidate. size_t AddPrflxCandidate(const Candidate& local); - // Returns if RFC 5245 ICE protocol is used. - bool IsStandardIce() const; - - // Returns if Google ICE protocol is used. - bool IsGoogleIce() const; - - // Returns if Hybrid ICE protocol is used. - bool IsHybridIce() const; - void set_candidate_filter(uint32 candidate_filter) { candidate_filter_ = candidate_filter; } @@ -384,7 +364,6 @@ class Port : public PortInterface, public rtc::MessageHandler, AddressMap connections_; int timeout_delay_; bool enable_port_packets_; - IceProtocolType ice_protocol_; IceRole ice_role_; uint64 tiebreaker_; bool shared_socket_; diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 76c7f2b988..2c122da6fc 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -64,12 +64,6 @@ static const RelayCredentials kRelayCredentials("test", "test"); // Magic value of 30 is from RFC3484, for IPv4 addresses. static const uint32 kDefaultPrflxPriority = ICE_TYPE_PREFERENCE_PRFLX << 24 | 30 << 8 | (256 - ICE_CANDIDATE_COMPONENT_DEFAULT); -static const int STUN_ERROR_BAD_REQUEST_AS_GICE = - STUN_ERROR_BAD_REQUEST / 256 * 100 + STUN_ERROR_BAD_REQUEST % 256; -static const int STUN_ERROR_UNAUTHORIZED_AS_GICE = - STUN_ERROR_UNAUTHORIZED / 256 * 100 + STUN_ERROR_UNAUTHORIZED % 256; -static const int STUN_ERROR_SERVER_ERROR_AS_GICE = - STUN_ERROR_SERVER_ERROR / 256 * 100 + STUN_ERROR_SERVER_ERROR % 256; static const int kTiebreaker1 = 11111; static const int kTiebreaker2 = 22222; @@ -77,7 +71,7 @@ static const int kTiebreaker2 = 22222; static const char* data = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; static Candidate GetCandidate(Port* port) { - assert(port->Candidates().size() == 1); + assert(port->Candidates().size() >= 1); return port->Candidates()[0]; } @@ -280,23 +274,15 @@ class TestChannel : public sigslot::has_slots<> { if (!remote_address_.IsNil()) { ASSERT_EQ(remote_address_, addr); } - // MI and PRIORITY attribute should be present in ping requests when port - // is in ICEPROTO_RFC5245 mode. const cricket::StunUInt32Attribute* priority_attr = msg->GetUInt32(STUN_ATTR_PRIORITY); const cricket::StunByteStringAttribute* mi_attr = msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY); const cricket::StunUInt32Attribute* fingerprint_attr = msg->GetUInt32(STUN_ATTR_FINGERPRINT); - if (src_->IceProtocol() == cricket::ICEPROTO_RFC5245) { - EXPECT_TRUE(priority_attr != NULL); - EXPECT_TRUE(mi_attr != NULL); - EXPECT_TRUE(fingerprint_attr != NULL); - } else { - EXPECT_TRUE(priority_attr == NULL); - EXPECT_TRUE(mi_attr == NULL); - EXPECT_TRUE(fingerprint_attr == NULL); - } + EXPECT_TRUE(priority_attr != NULL); + EXPECT_TRUE(mi_attr != NULL); + EXPECT_TRUE(fingerprint_attr != NULL); remote_address_ = addr; remote_request_.reset(CopyStunMessage(msg)); remote_frag_ = rf; @@ -358,7 +344,6 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { kRelaySslTcpExtAddr), username_(rtc::CreateRandomString(ICE_UFRAG_LENGTH)), password_(rtc::CreateRandomString(ICE_PWD_LENGTH)), - ice_protocol_(cricket::ICEPROTO_GOOGLE), role_conflict_(false), destroyed_(false) { network_.AddIP(rtc::IPAddress(INADDR_ANY)); @@ -367,35 +352,45 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { protected: void TestLocalToLocal() { Port* port1 = CreateUdpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateUdpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("udp", port1, "udp", port2, true, true, true, true); } void TestLocalToStun(NATType ntype) { Port* port1 = CreateUdpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); nat_server2_.reset(CreateNatServer(kNatAddr2, ntype)); Port* port2 = CreateStunPort(kLocalAddr2, &nat_socket_factory2_); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("udp", port1, StunName(ntype), port2, ntype == NAT_OPEN_CONE, true, ntype != NAT_SYMMETRIC, true); } void TestLocalToRelay(RelayType rtype, ProtocolType proto) { Port* port1 = CreateUdpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateRelayPort(kLocalAddr2, rtype, proto, PROTO_UDP); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("udp", port1, RelayName(rtype, proto), port2, rtype == RELAY_GTURN, true, true, true); } void TestStunToLocal(NATType ntype) { nat_server1_.reset(CreateNatServer(kNatAddr1, ntype)); Port* port1 = CreateStunPort(kLocalAddr1, &nat_socket_factory1_); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateUdpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity(StunName(ntype), port1, "udp", port2, true, ntype != NAT_SYMMETRIC, true, true); } void TestStunToStun(NATType ntype1, NATType ntype2) { nat_server1_.reset(CreateNatServer(kNatAddr1, ntype1)); Port* port1 = CreateStunPort(kLocalAddr1, &nat_socket_factory1_); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); nat_server2_.reset(CreateNatServer(kNatAddr2, ntype2)); Port* port2 = CreateStunPort(kLocalAddr2, &nat_socket_factory2_); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity(StunName(ntype1), port1, StunName(ntype2), port2, ntype2 == NAT_OPEN_CONE, ntype1 != NAT_SYMMETRIC, ntype2 != NAT_SYMMETRIC, @@ -404,24 +399,32 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { void TestStunToRelay(NATType ntype, RelayType rtype, ProtocolType proto) { nat_server1_.reset(CreateNatServer(kNatAddr1, ntype)); Port* port1 = CreateStunPort(kLocalAddr1, &nat_socket_factory1_); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateRelayPort(kLocalAddr2, rtype, proto, PROTO_UDP); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity(StunName(ntype), port1, RelayName(rtype, proto), port2, rtype == RELAY_GTURN, ntype != NAT_SYMMETRIC, true, true); } void TestTcpToTcp() { Port* port1 = CreateTcpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateTcpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("tcp", port1, "tcp", port2, true, false, true, true); } void TestTcpToRelay(RelayType rtype, ProtocolType proto) { Port* port1 = CreateTcpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateRelayPort(kLocalAddr2, rtype, proto, PROTO_TCP); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("tcp", port1, RelayName(rtype, proto), port2, rtype == RELAY_GTURN, false, true, true); } void TestSslTcpToRelay(RelayType rtype, ProtocolType proto) { Port* port1 = CreateTcpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateRelayPort(kLocalAddr2, rtype, proto, PROTO_SSLTCP); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); TestConnectivity("ssltcp", port1, RelayName(rtype, proto), port2, rtype == RELAY_GTURN, false, true, true); } @@ -431,35 +434,27 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { } UDPPort* CreateUdpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - UDPPort* port = UDPPort::Create(main_, socket_factory, &network_, - addr.ipaddr(), 0, 0, username_, password_, - std::string(), false); - port->SetIceProtocolType(ice_protocol_); - return port; + return UDPPort::Create(main_, socket_factory, &network_, + addr.ipaddr(), 0, 0, username_, password_, + std::string(), false); } TCPPort* CreateTcpPort(const SocketAddress& addr) { - TCPPort* port = CreateTcpPort(addr, &socket_factory_); - port->SetIceProtocolType(ice_protocol_); - return port; + return CreateTcpPort(addr, &socket_factory_); } TCPPort* CreateTcpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - TCPPort* port = TCPPort::Create(main_, socket_factory, &network_, - addr.ipaddr(), 0, 0, username_, password_, - true); - port->SetIceProtocolType(ice_protocol_); - return port; + return TCPPort::Create(main_, socket_factory, &network_, + addr.ipaddr(), 0, 0, username_, password_, + true); } StunPort* CreateStunPort(const SocketAddress& addr, rtc::PacketSocketFactory* factory) { ServerAddresses stun_servers; stun_servers.insert(kStunAddr); - StunPort* port = StunPort::Create(main_, factory, &network_, - addr.ipaddr(), 0, 0, - username_, password_, stun_servers, - std::string()); - port->SetIceProtocolType(ice_protocol_); - return port; + return StunPort::Create(main_, factory, &network_, + addr.ipaddr(), 0, 0, + username_, password_, stun_servers, + std::string()); } Port* CreateRelayPort(const SocketAddress& addr, RelayType rtype, ProtocolType int_proto, ProtocolType ext_proto) { @@ -479,14 +474,12 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { PacketSocketFactory* socket_factory, ProtocolType int_proto, ProtocolType ext_proto, const rtc::SocketAddress& server_addr) { - TurnPort* port = TurnPort::Create(main_, socket_factory, &network_, - addr.ipaddr(), 0, 0, - username_, password_, ProtocolAddress( - server_addr, PROTO_UDP), - kRelayCredentials, 0, - std::string()); - port->SetIceProtocolType(ice_protocol_); - return port; + return TurnPort::Create(main_, socket_factory, &network_, + addr.ipaddr(), 0, 0, + username_, password_, ProtocolAddress( + server_addr, PROTO_UDP), + kRelayCredentials, 0, + std::string()); } RelayPort* CreateGturnPort(const SocketAddress& addr, ProtocolType int_proto, ProtocolType ext_proto) { @@ -497,13 +490,12 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { return port; } RelayPort* CreateGturnPort(const SocketAddress& addr) { - RelayPort* port = RelayPort::Create(main_, &socket_factory_, &network_, - addr.ipaddr(), 0, 0, - username_, password_); + // TODO(pthatcher): Remove GTURN. + return RelayPort::Create(main_, &socket_factory_, &network_, + addr.ipaddr(), 0, 0, + username_, password_); // TODO: Add an external address for ext_proto, so that the // other side can connect to this port using a non-UDP protocol. - port->SetIceProtocolType(ice_protocol_); - return port; } rtc::NATServer* CreateNatServer(const SocketAddress& addr, rtc::NATType type) { @@ -597,7 +589,9 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { void TestTcpReconnect(bool ping_after_disconnected, bool send_after_disconnected) { Port* port1 = CreateTcpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); Port* port2 = CreateTcpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); port1->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT); port2->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT); @@ -658,10 +652,6 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE_WAIT(ch2.conn() == NULL, kTimeout); } - void SetIceProtocolType(cricket::IceProtocolType protocol) { - ice_protocol_ = protocol; - } - IceMessage* CreateStunMessage(int type) { IceMessage* msg = new IceMessage(); msg->SetType(type); @@ -686,11 +676,9 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { TestPort* CreateTestPort(const rtc::SocketAddress& addr, const std::string& username, const std::string& password, - cricket::IceProtocolType type, cricket::IceRole role, int tiebreaker) { TestPort* port = CreateTestPort(addr, username, password); - port->SetIceProtocolType(type); port->SetIceRole(role); port->SetIceTiebreaker(tiebreaker); return port; @@ -732,7 +720,6 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { TestRelayServer relay_server_; std::string username_; std::string password_; - cricket::IceProtocolType ice_protocol_; bool role_conflict_; bool destroyed_; }; @@ -1180,16 +1167,13 @@ TEST_F(PortTest, TestSslTcpToSslTcpRelay) { // This test case verifies standard ICE features in STUN messages. Currently it // verifies Message Integrity attribute in STUN messages and username in STUN // binding request will have colon (":") between remote and local username. -TEST_F(PortTest, TestLocalToLocalAsIce) { - SetIceProtocolType(cricket::ICEPROTO_RFC5245); +TEST_F(PortTest, TestLocalToLocalStandard) { UDPPort* port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); port1->SetIceTiebreaker(kTiebreaker1); - ASSERT_EQ(cricket::ICEPROTO_RFC5245, port1->IceProtocol()); UDPPort* port2 = CreateUdpPort(kLocalAddr2); port2->SetIceRole(cricket::ICEROLE_CONTROLLED); port2->SetIceTiebreaker(kTiebreaker2); - ASSERT_EQ(cricket::ICEPROTO_RFC5245, port2->IceProtocol()); // Same parameters as TestLocalToLocal above. TestConnectivity("udp", port1, "udp", port2, true, true, true, true); } @@ -1198,10 +1182,9 @@ TEST_F(PortTest, TestLocalToLocalAsIce) { // loopback test when protocol is RFC5245. For success IceTiebreaker, username // should remain equal to the request generated by the port and role of port // must be in controlling. -TEST_F(PortTest, TestLoopbackCallAsIce) { +TEST_F(PortTest, TestLoopbackCal) { rtc::scoped_ptr lport( CreateTestPort(kLocalAddr1, "lfrag", "lpass")); - lport->SetIceProtocolType(ICEPROTO_RFC5245); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); lport->SetIceTiebreaker(kTiebreaker1); lport->PrepareAddress(); @@ -1262,12 +1245,10 @@ TEST_F(PortTest, TestLoopbackCallAsIce) { TEST_F(PortTest, TestIceRoleConflict) { rtc::scoped_ptr lport( CreateTestPort(kLocalAddr1, "lfrag", "lpass")); - lport->SetIceProtocolType(ICEPROTO_RFC5245); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); lport->SetIceTiebreaker(kTiebreaker1); rtc::scoped_ptr rport( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - rport->SetIceProtocolType(ICEPROTO_RFC5245); rport->SetIceRole(cricket::ICEROLE_CONTROLLING); rport->SetIceTiebreaker(kTiebreaker2); @@ -1296,6 +1277,7 @@ TEST_F(PortTest, TestIceRoleConflict) { TEST_F(PortTest, TestTcpNoDelay) { TCPPort* port1 = CreateTcpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); int option_value = -1; int success = port1->GetOption(rtc::Socket::OPT_NODELAY, &option_value); @@ -1475,93 +1457,14 @@ TEST_F(PortTest, TestDefaultDscpValue) { EXPECT_EQ(rtc::DSCP_CS6, dscp); } -// Test sending STUN messages in GICE format. -TEST_F(PortTest, TestSendStunMessageAsGice) { +// Test sending STUN messages. +TEST_F(PortTest, TestSendStunMessage) { rtc::scoped_ptr lport( CreateTestPort(kLocalAddr1, "lfrag", "lpass")); rtc::scoped_ptr rport( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - lport->SetIceProtocolType(ICEPROTO_GOOGLE); - rport->SetIceProtocolType(ICEPROTO_GOOGLE); - - // Send a fake ping from lport to rport. - lport->PrepareAddress(); - rport->PrepareAddress(); - ASSERT_FALSE(rport->Candidates().empty()); - Connection* conn = lport->CreateConnection(rport->Candidates()[0], - Port::ORIGIN_MESSAGE); - rport->CreateConnection(lport->Candidates()[0], Port::ORIGIN_MESSAGE); - conn->Ping(0); - - // Check that it's a proper BINDING-REQUEST. - ASSERT_TRUE_WAIT(lport->last_stun_msg() != NULL, 1000); - IceMessage* msg = lport->last_stun_msg(); - EXPECT_EQ(STUN_BINDING_REQUEST, msg->type()); - EXPECT_FALSE(msg->IsLegacy()); - const StunByteStringAttribute* username_attr = msg->GetByteString( - STUN_ATTR_USERNAME); - ASSERT_TRUE(username_attr != NULL); - EXPECT_EQ("rfraglfrag", username_attr->GetString()); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_PRIORITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_FINGERPRINT) == NULL); - - // Save a copy of the BINDING-REQUEST for use below. - rtc::scoped_ptr request(CopyStunMessage(msg)); - - // Respond with a BINDING-RESPONSE. - rport->SendBindingResponse(request.get(), lport->Candidates()[0].address()); - msg = rport->last_stun_msg(); - ASSERT_TRUE(msg != NULL); - EXPECT_EQ(STUN_BINDING_RESPONSE, msg->type()); - EXPECT_FALSE(msg->IsLegacy()); - username_attr = msg->GetByteString(STUN_ATTR_USERNAME); - ASSERT_TRUE(username_attr != NULL); // GICE has a username in the response. - EXPECT_EQ("rfraglfrag", username_attr->GetString()); - const StunAddressAttribute* addr_attr = msg->GetAddress( - STUN_ATTR_MAPPED_ADDRESS); - ASSERT_TRUE(addr_attr != NULL); - EXPECT_EQ(lport->Candidates()[0].address(), addr_attr->GetAddress()); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_XOR_MAPPED_ADDRESS) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_PRIORITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_FINGERPRINT) == NULL); - - // Respond with a BINDING-ERROR-RESPONSE. This wouldn't happen in real life, - // but we can do it here. - rport->SendBindingErrorResponse(request.get(), - rport->Candidates()[0].address(), - STUN_ERROR_SERVER_ERROR, - STUN_ERROR_REASON_SERVER_ERROR); - msg = rport->last_stun_msg(); - ASSERT_TRUE(msg != NULL); - EXPECT_EQ(STUN_BINDING_ERROR_RESPONSE, msg->type()); - EXPECT_FALSE(msg->IsLegacy()); - username_attr = msg->GetByteString(STUN_ATTR_USERNAME); - ASSERT_TRUE(username_attr != NULL); // GICE has a username in the response. - EXPECT_EQ("rfraglfrag", username_attr->GetString()); - const StunErrorCodeAttribute* error_attr = msg->GetErrorCode(); - ASSERT_TRUE(error_attr != NULL); - // The GICE wire format for error codes is incorrect. - EXPECT_EQ(STUN_ERROR_SERVER_ERROR_AS_GICE, error_attr->code()); - EXPECT_EQ(STUN_ERROR_SERVER_ERROR / 256, error_attr->eclass()); - EXPECT_EQ(STUN_ERROR_SERVER_ERROR % 256, error_attr->number()); - EXPECT_EQ(std::string(STUN_ERROR_REASON_SERVER_ERROR), error_attr->reason()); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_PRIORITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) == NULL); - EXPECT_TRUE(msg->GetByteString(STUN_ATTR_FINGERPRINT) == NULL); -} - -// Test sending STUN messages in ICE format. -TEST_F(PortTest, TestSendStunMessageAsIce) { - rtc::scoped_ptr lport( - CreateTestPort(kLocalAddr1, "lfrag", "lpass")); - rtc::scoped_ptr rport( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - lport->SetIceProtocolType(ICEPROTO_RFC5245); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); lport->SetIceTiebreaker(kTiebreaker1); - rport->SetIceProtocolType(ICEPROTO_RFC5245); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); rport->SetIceTiebreaker(kTiebreaker2); @@ -1700,10 +1603,8 @@ TEST_F(PortTest, TestUseCandidateAttribute) { CreateTestPort(kLocalAddr1, "lfrag", "lpass")); rtc::scoped_ptr rport( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - lport->SetIceProtocolType(ICEPROTO_RFC5245); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); lport->SetIceTiebreaker(kTiebreaker1); - rport->SetIceProtocolType(ICEPROTO_RFC5245); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); rport->SetIceTiebreaker(kTiebreaker2); @@ -1724,79 +1625,11 @@ TEST_F(PortTest, TestUseCandidateAttribute) { ASSERT_TRUE(use_candidate_attr != NULL); } -// Test handling STUN messages in GICE format. -TEST_F(PortTest, TestHandleStunMessageAsGice) { +// Test handling STUN messages. +TEST_F(PortTest, TestHandleStunMessage) { // Our port will act as the "remote" port. rtc::scoped_ptr port( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_GOOGLE); - - rtc::scoped_ptr in_msg, out_msg; - rtc::scoped_ptr buf(new ByteBuffer()); - rtc::SocketAddress addr(kLocalAddr1); - std::string username; - - // BINDING-REQUEST from local to remote with valid GICE username and no M-I. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "rfraglfrag")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); // Succeeds, since this is GICE. - EXPECT_EQ("lfrag", username); - - // Add M-I; should be ignored and rest of message parsed normally. - in_msg->AddMessageIntegrity("password"); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); - EXPECT_EQ("lfrag", username); - - // BINDING-RESPONSE with username, as done in GICE. Should succeed. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_RESPONSE, - "rfraglfrag")); - in_msg->AddAttribute( - new StunAddressAttribute(STUN_ATTR_MAPPED_ADDRESS, kLocalAddr2)); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); - EXPECT_EQ("", username); - - // BINDING-RESPONSE without username. Should be tolerated as well. - in_msg.reset(CreateStunMessage(STUN_BINDING_RESPONSE)); - in_msg->AddAttribute( - new StunAddressAttribute(STUN_ATTR_MAPPED_ADDRESS, kLocalAddr2)); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); - EXPECT_EQ("", username); - - // BINDING-ERROR-RESPONSE with username and error code. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_ERROR_RESPONSE, - "rfraglfrag")); - in_msg->AddAttribute(new StunErrorCodeAttribute(STUN_ATTR_ERROR_CODE, - STUN_ERROR_SERVER_ERROR_AS_GICE, STUN_ERROR_REASON_SERVER_ERROR)); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - ASSERT_TRUE(out_msg.get() != NULL); - EXPECT_EQ("", username); - ASSERT_TRUE(out_msg->GetErrorCode() != NULL); - // GetStunMessage doesn't unmunge the GICE error code (happens downstream). - EXPECT_EQ(STUN_ERROR_SERVER_ERROR_AS_GICE, out_msg->GetErrorCode()->code()); - EXPECT_EQ(std::string(STUN_ERROR_REASON_SERVER_ERROR), - out_msg->GetErrorCode()->reason()); -} - -// Test handling STUN messages in ICE format. -TEST_F(PortTest, TestHandleStunMessageAsIce) { - // Our port will act as the "remote" port. - rtc::scoped_ptr port( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_RFC5245); rtc::scoped_ptr in_msg, out_msg; rtc::scoped_ptr buf(new ByteBuffer()); @@ -1843,145 +1676,10 @@ TEST_F(PortTest, TestHandleStunMessageAsIce) { out_msg->GetErrorCode()->reason()); } -// This test verifies port can handle ICE messages in Hybrid mode and switches -// ICEPROTO_RFC5245 mode after successfully handling the message. -TEST_F(PortTest, TestHandleStunMessageAsIceInHybridMode) { - // Our port will act as the "remote" port. - rtc::scoped_ptr port( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_HYBRID); - - rtc::scoped_ptr in_msg, out_msg; - rtc::scoped_ptr buf(new ByteBuffer()); - rtc::SocketAddress addr(kLocalAddr1); - std::string username; - - // BINDING-REQUEST from local to remote with valid ICE username, - // MESSAGE-INTEGRITY, and FINGERPRINT. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "rfrag:lfrag")); - in_msg->AddMessageIntegrity("rpass"); - in_msg->AddFingerprint(); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); - EXPECT_EQ("lfrag", username); - EXPECT_EQ(ICEPROTO_RFC5245, port->IceProtocol()); -} - -// This test verifies port can handle GICE messages in Hybrid mode and switches -// ICEPROTO_GOOGLE mode after successfully handling the message. -TEST_F(PortTest, TestHandleStunMessageAsGiceInHybridMode) { - // Our port will act as the "remote" port. - rtc::scoped_ptr port( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_HYBRID); - - rtc::scoped_ptr in_msg, out_msg; - rtc::scoped_ptr buf(new ByteBuffer()); - rtc::SocketAddress addr(kLocalAddr1); - std::string username; - - // BINDING-REQUEST from local to remote with valid GICE username and no M-I. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "rfraglfrag")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() != NULL); // Succeeds, since this is GICE. - EXPECT_EQ("lfrag", username); - EXPECT_EQ(ICEPROTO_GOOGLE, port->IceProtocol()); -} - -// Verify port is not switched out of RFC5245 mode if GICE message is received -// in that mode. -TEST_F(PortTest, TestHandleStunMessageAsGiceInIceMode) { - // Our port will act as the "remote" port. - rtc::scoped_ptr port( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_RFC5245); - - rtc::scoped_ptr in_msg, out_msg; - rtc::scoped_ptr buf(new ByteBuffer()); - rtc::SocketAddress addr(kLocalAddr1); - std::string username; - - // BINDING-REQUEST from local to remote with valid GICE username and no M-I. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "rfraglfrag")); - WriteStunMessage(in_msg.get(), buf.get()); - // Should fail as there is no MI and fingerprint. - EXPECT_FALSE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_EQ(ICEPROTO_RFC5245, port->IceProtocol()); -} - - -// Tests handling of GICE binding requests with missing or incorrect usernames. -TEST_F(PortTest, TestHandleStunMessageAsGiceBadUsername) { - rtc::scoped_ptr port( - CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_GOOGLE); - - rtc::scoped_ptr in_msg, out_msg; - rtc::scoped_ptr buf(new ByteBuffer()); - rtc::SocketAddress addr(kLocalAddr1); - std::string username; - - // BINDING-REQUEST with no username. - in_msg.reset(CreateStunMessage(STUN_BINDING_REQUEST)); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() == NULL); - EXPECT_EQ("", username); - EXPECT_EQ(STUN_ERROR_BAD_REQUEST_AS_GICE, port->last_stun_error_code()); - - // BINDING-REQUEST with empty username. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, "")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() == NULL); - EXPECT_EQ("", username); - EXPECT_EQ(STUN_ERROR_UNAUTHORIZED_AS_GICE, port->last_stun_error_code()); - - // BINDING-REQUEST with too-short username. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, "lfra")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() == NULL); - EXPECT_EQ("", username); - EXPECT_EQ(STUN_ERROR_UNAUTHORIZED_AS_GICE, port->last_stun_error_code()); - - // BINDING-REQUEST with reversed username. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "lfragrfrag")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() == NULL); - EXPECT_EQ("", username); - EXPECT_EQ(STUN_ERROR_UNAUTHORIZED_AS_GICE, port->last_stun_error_code()); - - // BINDING-REQUEST with garbage username. - in_msg.reset(CreateStunMessageWithUsername(STUN_BINDING_REQUEST, - "abcdefgh")); - WriteStunMessage(in_msg.get(), buf.get()); - EXPECT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, - out_msg.accept(), &username)); - EXPECT_TRUE(out_msg.get() == NULL); - EXPECT_EQ("", username); - EXPECT_EQ(STUN_ERROR_UNAUTHORIZED_AS_GICE, port->last_stun_error_code()); -} - // Tests handling of ICE binding requests with missing or incorrect usernames. -TEST_F(PortTest, TestHandleStunMessageAsIceBadUsername) { +TEST_F(PortTest, TestHandleStunMessageBadUsername) { rtc::scoped_ptr port( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_RFC5245); rtc::scoped_ptr in_msg, out_msg; rtc::scoped_ptr buf(new ByteBuffer()); @@ -2046,12 +1744,11 @@ TEST_F(PortTest, TestHandleStunMessageAsIceBadUsername) { EXPECT_EQ(STUN_ERROR_UNAUTHORIZED, port->last_stun_error_code()); } -// Test handling STUN messages (as ICE) with missing or malformed M-I. -TEST_F(PortTest, TestHandleStunMessageAsIceBadMessageIntegrity) { +// Test handling STUN messages with missing or malformed M-I. +TEST_F(PortTest, TestHandleStunMessageBadMessageIntegrity) { // Our port will act as the "remote" port. rtc::scoped_ptr port( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_RFC5245); rtc::scoped_ptr in_msg, out_msg; rtc::scoped_ptr buf(new ByteBuffer()); @@ -2088,12 +1785,11 @@ TEST_F(PortTest, TestHandleStunMessageAsIceBadMessageIntegrity) { // Change this test to pass in data via Connection::OnReadPacket instead. } -// Test handling STUN messages (as ICE) with missing or malformed FINGERPRINT. -TEST_F(PortTest, TestHandleStunMessageAsIceBadFingerprint) { +// Test handling STUN messages with missing or malformed FINGERPRINT. +TEST_F(PortTest, TestHandleStunMessageBadFingerprint) { // Our port will act as the "remote" port. rtc::scoped_ptr port( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - port->SetIceProtocolType(ICEPROTO_RFC5245); rtc::scoped_ptr in_msg, out_msg; rtc::scoped_ptr buf(new ByteBuffer()); @@ -2155,12 +1851,11 @@ TEST_F(PortTest, TestHandleStunMessageAsIceBadFingerprint) { EXPECT_EQ(0, port->last_stun_error_code()); } -// Test handling of STUN binding indication messages (as ICE). STUN binding +// Test handling of STUN binding indication messages . STUN binding // indications are allowed only to the connection which is in read mode. TEST_F(PortTest, TestHandleStunBindingIndication) { rtc::scoped_ptr lport( CreateTestPort(kLocalAddr2, "lfrag", "lpass")); - lport->SetIceProtocolType(ICEPROTO_RFC5245); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); lport->SetIceTiebreaker(kTiebreaker1); @@ -2183,7 +1878,6 @@ TEST_F(PortTest, TestHandleStunBindingIndication) { // last_ping_received. rtc::scoped_ptr rport( CreateTestPort(kLocalAddr2, "rfrag", "rpass")); - rport->SetIceProtocolType(ICEPROTO_RFC5245); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); rport->SetIceTiebreaker(kTiebreaker2); @@ -2386,12 +2080,12 @@ TEST_F(PortTest, TestCandidateRelatedAddress) { } // Test priority value overflow handling when preference is set to 3. -TEST_F(PortTest, TestCandidatePreference) { +TEST_F(PortTest, TestCandidatePriority) { cricket::Candidate cand1; - cand1.set_preference(3); + cand1.set_priority(3); cricket::Candidate cand2; - cand2.set_preference(1); - EXPECT_TRUE(cand1.preference() > cand2.preference()); + cand2.set_priority(1); + EXPECT_TRUE(cand1.priority() > cand2.priority()); } // Test the Connection priority is calculated correctly. @@ -2435,7 +2129,9 @@ TEST_F(PortTest, TestConnectionPriority) { TEST_F(PortTest, TestWritableState) { UDPPort* port1 = CreateUdpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); UDPPort* port2 = CreateUdpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); // Set up channels. TestChannel ch1(port1, port2); @@ -2504,7 +2200,9 @@ TEST_F(PortTest, TestWritableState) { TEST_F(PortTest, TestTimeoutForNeverWritable) { UDPPort* port1 = CreateUdpPort(kLocalAddr1); + port1->SetIceRole(cricket::ICEROLE_CONTROLLING); UDPPort* port2 = CreateUdpPort(kLocalAddr2); + port2->SetIceRole(cricket::ICEROLE_CONTROLLED); // Set up channels. TestChannel ch1(port1, port2); @@ -2532,11 +2230,11 @@ TEST_F(PortTest, TestTimeoutForNeverWritable) { // port which responds to the ping message just like LITE client. TEST_F(PortTest, TestIceLiteConnectivity) { TestPort* ice_full_port = CreateTestPort( - kLocalAddr1, "lfrag", "lpass", cricket::ICEPROTO_RFC5245, + kLocalAddr1, "lfrag", "lpass", cricket::ICEROLE_CONTROLLING, kTiebreaker1); rtc::scoped_ptr ice_lite_port(CreateTestPort( - kLocalAddr2, "rfrag", "rpass", cricket::ICEPROTO_RFC5245, + kLocalAddr2, "rfrag", "rpass", cricket::ICEROLE_CONTROLLED, kTiebreaker2)); // Setup TestChannel. This behaves like FULL mode client. TestChannel ch1(ice_full_port, ice_lite_port.get()); @@ -2595,7 +2293,6 @@ TEST_F(PortTest, TestIceLiteConnectivity) { // This test case verifies that the CONTROLLING port does not time out. TEST_F(PortTest, TestControllingNoTimeout) { - SetIceProtocolType(cricket::ICEPROTO_RFC5245); UDPPort* port1 = CreateUdpPort(kLocalAddr1); ConnectToSignalDestroyed(port1); port1->set_timeout_delay(10); // milliseconds @@ -2621,7 +2318,6 @@ TEST_F(PortTest, TestControllingNoTimeout) { // This test case verifies that the CONTROLLED port does time out, but only // after connectivity is lost. TEST_F(PortTest, TestControlledTimeout) { - SetIceProtocolType(cricket::ICEPROTO_RFC5245); UDPPort* port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); port1->SetIceTiebreaker(kTiebreaker1); diff --git a/webrtc/p2p/base/portallocator.cc b/webrtc/p2p/base/portallocator.cc index 35edade13c..76455b506e 100644 --- a/webrtc/p2p/base/portallocator.cc +++ b/webrtc/p2p/base/portallocator.cc @@ -21,11 +21,8 @@ PortAllocatorSession::PortAllocatorSession(const std::string& content_name, component_(component), flags_(flags), generation_(0), - // If PORTALLOCATOR_ENABLE_SHARED_UFRAG flag is not enabled, ignore the - // incoming ufrag and pwd, which will cause each Port to generate one - // by itself. - username_(flags_ & PORTALLOCATOR_ENABLE_SHARED_UFRAG ? ice_ufrag : ""), - password_(flags_ & PORTALLOCATOR_ENABLE_SHARED_UFRAG ? ice_pwd : "") { + username_(ice_ufrag), + password_(ice_pwd) { } PortAllocatorSession* PortAllocator::CreateSession( diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 51b3e98550..c89684727b 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -34,6 +34,11 @@ enum { PORTALLOCATOR_DISABLE_TCP = 0x08, PORTALLOCATOR_ENABLE_SHAKER = 0x10, PORTALLOCATOR_ENABLE_IPV6 = 0x40, + // TODO(pthatcher): Remove this once it's no longer used in: + // remoting/client/plugin/pepper_port_allocator.cc + // remoting/protocol/chromium_port_allocator.cc + // remoting/test/fake_port_allocator.cc + // It's a no-op and is no longer needed. PORTALLOCATOR_ENABLE_SHARED_UFRAG = 0x80, PORTALLOCATOR_ENABLE_SHARED_SOCKET = 0x100, PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE = 0x200, diff --git a/webrtc/p2p/base/portinterface.h b/webrtc/p2p/base/portinterface.h index ee6835ebf3..3c246da58a 100644 --- a/webrtc/p2p/base/portinterface.h +++ b/webrtc/p2p/base/portinterface.h @@ -43,9 +43,6 @@ class PortInterface { virtual const std::string& Type() const = 0; virtual rtc::Network* Network() const = 0; - virtual void SetIceProtocolType(IceProtocolType protocol) = 0; - virtual IceProtocolType IceProtocol() const = 0; - // Methods to set/get ICE role and tiebreaker values. virtual void SetIceRole(IceRole role) = 0; virtual IceRole GetIceRole() const = 0; diff --git a/webrtc/p2p/base/rawtransport.cc b/webrtc/p2p/base/rawtransport.cc index 8ff00cd3e0..cb700ae4a0 100644 --- a/webrtc/p2p/base/rawtransport.cc +++ b/webrtc/p2p/base/rawtransport.cc @@ -1,43 +1,2 @@ -/* - * Copyright 2004 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include - -#include "webrtc/p2p/base/rawtransport.h" -#include "webrtc/p2p/base/rawtransportchannel.h" -#include "webrtc/base/common.h" - -#if defined(FEATURE_ENABLE_PSTN) -namespace cricket { - -RawTransport::RawTransport(rtc::Thread* signaling_thread, - rtc::Thread* worker_thread, - const std::string& content_name, - PortAllocator* allocator) - : Transport(signaling_thread, worker_thread, - content_name, NS_GINGLE_RAW, allocator) { -} - -RawTransport::~RawTransport() { - DestroyAllChannels(); -} - -TransportChannelImpl* RawTransport::CreateTransportChannel(int component) { - return new RawTransportChannel(content_name(), component, this, - worker_thread(), - port_allocator()); -} - -void RawTransport::DestroyTransportChannel(TransportChannelImpl* channel) { - delete channel; -} - -} // namespace cricket -#endif // defined(FEATURE_ENABLE_PSTN) +// TODO(pthatcher): Remove this file once Chrome's build files no +// longer refer to it. diff --git a/webrtc/p2p/base/rawtransport.h b/webrtc/p2p/base/rawtransport.h index 065dc0e257..cb700ae4a0 100644 --- a/webrtc/p2p/base/rawtransport.h +++ b/webrtc/p2p/base/rawtransport.h @@ -1,46 +1,2 @@ -/* - * Copyright 2004 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_P2P_BASE_RAWTRANSPORT_H_ -#define WEBRTC_P2P_BASE_RAWTRANSPORT_H_ - -#include -#include "webrtc/p2p/base/transport.h" - -#if defined(FEATURE_ENABLE_PSTN) -namespace cricket { - -// Implements a transport that only sends raw packets, no STUN. As a result, -// it cannot do pings to determine connectivity, so it only uses a single port -// that it thinks will work. -class RawTransport : public Transport { - public: - RawTransport(rtc::Thread* signaling_thread, - rtc::Thread* worker_thread, - const std::string& content_name, - PortAllocator* allocator); - virtual ~RawTransport(); - - protected: - // Creates and destroys raw channels. - virtual TransportChannelImpl* CreateTransportChannel(int component); - virtual void DestroyTransportChannel(TransportChannelImpl* channel); - - private: - friend class RawTransportChannel; // For ParseAddress. - - DISALLOW_COPY_AND_ASSIGN(RawTransport); -}; - -} // namespace cricket - -#endif // defined(FEATURE_ENABLE_PSTN) - -#endif // WEBRTC_P2P_BASE_RAWTRANSPORT_H_ +// TODO(pthatcher): Remove this file once Chrome's build files no +// longer refer to it. diff --git a/webrtc/p2p/base/rawtransportchannel.cc b/webrtc/p2p/base/rawtransportchannel.cc index b032e63cda..cb700ae4a0 100644 --- a/webrtc/p2p/base/rawtransportchannel.cc +++ b/webrtc/p2p/base/rawtransportchannel.cc @@ -1,260 +1,2 @@ -/* - * Copyright 2004 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/p2p/base/rawtransportchannel.h" - -#include -#include -#include "webrtc/p2p/base/constants.h" -#include "webrtc/p2p/base/portallocator.h" -#include "webrtc/p2p/base/portinterface.h" -#include "webrtc/p2p/base/rawtransport.h" -#include "webrtc/p2p/base/relayport.h" -#include "webrtc/p2p/base/stunport.h" -#include "webrtc/base/common.h" - -#if defined(FEATURE_ENABLE_PSTN) - -namespace { - -const uint32 MSG_DESTROY_RTC_UNUSED_PORTS = 1; - -} // namespace - -namespace cricket { - -RawTransportChannel::RawTransportChannel(const std::string& content_name, - int component, - RawTransport* transport, - rtc::Thread *worker_thread, - PortAllocator *allocator) - : TransportChannelImpl(content_name, component), - raw_transport_(transport), - allocator_(allocator), - allocator_session_(NULL), - stun_port_(NULL), - relay_port_(NULL), - port_(NULL), - use_relay_(false) { - if (worker_thread == NULL) - worker_thread_ = raw_transport_->worker_thread(); - else - worker_thread_ = worker_thread; -} - -RawTransportChannel::~RawTransportChannel() { - delete allocator_session_; -} - -int RawTransportChannel::SendPacket(const char *data, size_t size, - const rtc::PacketOptions& options, - int flags) { - if (port_ == NULL) - return -1; - if (remote_address_.IsNil()) - return -1; - if (flags != 0) - return -1; - return port_->SendTo(data, size, remote_address_, options, true); -} - -int RawTransportChannel::SetOption(rtc::Socket::Option opt, int value) { - // TODO: allow these to be set before we have a port - if (port_ == NULL) - return -1; - return port_->SetOption(opt, value); -} - -bool RawTransportChannel::GetOption(rtc::Socket::Option opt, int* value) { - return false; -} - -int RawTransportChannel::GetError() { - return (port_ != NULL) ? port_->GetError() : 0; -} - -void RawTransportChannel::Connect() { - // Create an allocator that only returns stun and relay ports. - // Use empty string for ufrag and pwd here. There won't be any STUN or relay - // interactions when using RawTC. - // TODO: Change raw to only use local udp ports. - allocator_session_ = allocator_->CreateSession( - SessionId(), content_name(), component(), "", ""); - - uint32 flags = PORTALLOCATOR_DISABLE_UDP | PORTALLOCATOR_DISABLE_TCP; - -#if !defined(FEATURE_ENABLE_STUN_CLASSIFICATION) - flags |= PORTALLOCATOR_DISABLE_RELAY; -#endif - allocator_session_->set_flags(flags); - allocator_session_->SignalPortReady.connect( - this, &RawTransportChannel::OnPortReady); - allocator_session_->SignalCandidatesReady.connect( - this, &RawTransportChannel::OnCandidatesReady); - - // The initial ports will include stun. - allocator_session_->StartGettingPorts(); -} - -void RawTransportChannel::Reset() { - set_readable(false); - set_writable(false); - - delete allocator_session_; - - allocator_session_ = NULL; - stun_port_ = NULL; - relay_port_ = NULL; - port_ = NULL; - remote_address_ = rtc::SocketAddress(); -} - -void RawTransportChannel::OnCandidate(const Candidate& candidate) { - remote_address_ = candidate.address(); - ASSERT(!remote_address_.IsNil()); - set_readable(true); - - // We can write once we have a port and a remote address. - if (port_ != NULL) - SetWritable(); -} - -void RawTransportChannel::OnRemoteAddress( - const rtc::SocketAddress& remote_address) { - remote_address_ = remote_address; - set_readable(true); - - if (port_ != NULL) - SetWritable(); -} - -// Note about stun classification -// Code to classify our NAT type and use the relay port if we are behind an -// asymmetric NAT is under a FEATURE_ENABLE_STUN_CLASSIFICATION #define. -// To turn this one we will have to enable a second stun address and make sure -// that the relay server works for raw UDP. -// -// Another option is to classify the NAT type early and not offer the raw -// transport type at all if we can't support it. - -void RawTransportChannel::OnPortReady( - PortAllocatorSession* session, PortInterface* port) { - ASSERT(session == allocator_session_); - - if (port->Type() == STUN_PORT_TYPE) { - stun_port_ = static_cast(port); - } else if (port->Type() == RELAY_PORT_TYPE) { - relay_port_ = static_cast(port); - } else { - ASSERT(false); - } -} - -void RawTransportChannel::OnCandidatesReady( - PortAllocatorSession *session, const std::vector& candidates) { - ASSERT(session == allocator_session_); - ASSERT(candidates.size() >= 1); - - // The most recent candidate is the one we haven't seen yet. - Candidate c = candidates[candidates.size() - 1]; - - if (c.type() == STUN_PORT_TYPE) { - ASSERT(stun_port_ != NULL); - -#if defined(FEATURE_ENABLE_STUN_CLASSIFICATION) - // We need to wait until we have two addresses. - if (stun_port_->candidates().size() < 2) - return; - - // This is the second address. If these addresses are the same, then we - // are not behind a symmetric NAT. Hence, a stun port should be sufficient. - if (stun_port_->candidates()[0].address() == - stun_port_->candidates()[1].address()) { - SetPort(stun_port_); - return; - } - - // We will need to use relay. - use_relay_ = true; - - // If we already have a relay address, we're good. Otherwise, we will need - // to wait until one arrives. - if (relay_port_->candidates().size() > 0) - SetPort(relay_port_); -#else // defined(FEATURE_ENABLE_STUN_CLASSIFICATION) - // Always use the stun port. We don't classify right now so just assume it - // will work fine. - SetPort(stun_port_); -#endif - } else if (c.type() == RELAY_PORT_TYPE) { - if (use_relay_) - SetPort(relay_port_); - } else { - ASSERT(false); - } -} - -void RawTransportChannel::SetPort(PortInterface* port) { - ASSERT(port_ == NULL); - port_ = port; - - // We don't need any ports other than the one we picked. - allocator_session_->StopGettingPorts(); - worker_thread_->Post( - this, MSG_DESTROY_RTC_UNUSED_PORTS, NULL); - - // Send a message to the other client containing our address. - - ASSERT(port_->Candidates().size() >= 1); - ASSERT(port_->Candidates()[0].protocol() == "udp"); - SignalCandidateReady(this, port_->Candidates()[0]); - - // Read all packets from this port. - port_->EnablePortPackets(); - port_->SignalReadPacket.connect(this, &RawTransportChannel::OnReadPacket); - - // We can write once we have a port and a remote address. - if (!remote_address_.IsAny()) - SetWritable(); -} - -void RawTransportChannel::SetWritable() { - ASSERT(port_ != NULL); - ASSERT(!remote_address_.IsAny()); - - set_writable(true); - - Candidate remote_candidate; - remote_candidate.set_address(remote_address_); - SignalRouteChange(this, remote_candidate); -} - -void RawTransportChannel::OnReadPacket( - PortInterface* port, const char* data, size_t size, - const rtc::SocketAddress& addr) { - ASSERT(port_ == port); - SignalReadPacket(this, data, size, rtc::CreatePacketTime(0), 0); -} - -void RawTransportChannel::OnMessage(rtc::Message* msg) { - ASSERT(msg->message_id == MSG_DESTROY_RTC_UNUSED_PORTS); - ASSERT(port_ != NULL); - if (port_ != stun_port_) { - stun_port_->Destroy(); - stun_port_ = NULL; - } - if (port_ != relay_port_ && relay_port_ != NULL) { - relay_port_->Destroy(); - relay_port_ = NULL; - } -} - -} // namespace cricket -#endif // defined(FEATURE_ENABLE_PSTN) +// TODO(pthatcher): Remove this file once Chrome's build files no +// longer refer to it. diff --git a/webrtc/p2p/base/rawtransportchannel.h b/webrtc/p2p/base/rawtransportchannel.h index 75b494e65a..cb700ae4a0 100644 --- a/webrtc/p2p/base/rawtransportchannel.h +++ b/webrtc/p2p/base/rawtransportchannel.h @@ -1,200 +1,2 @@ -/* - * Copyright 2004 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_P2P_BASE_RAWTRANSPORTCHANNEL_H_ -#define WEBRTC_P2P_BASE_RAWTRANSPORTCHANNEL_H_ - -#include -#include -#include "webrtc/p2p/base/candidate.h" -#include "webrtc/p2p/base/rawtransport.h" -#include "webrtc/p2p/base/transportchannelimpl.h" -#include "webrtc/base/messagequeue.h" - -#if defined(FEATURE_ENABLE_PSTN) - -namespace rtc { -class Thread; -} - -namespace cricket { - -class Connection; -class PortAllocator; -class PortAllocatorSession; -class PortInterface; -class RelayPort; -class StunPort; - -// Implements a channel that just sends bare packets once we have received the -// address of the other side. We pick a single address to send them based on -// a simple investigation of NAT type. -class RawTransportChannel : public TransportChannelImpl, - public rtc::MessageHandler { - public: - RawTransportChannel(const std::string& content_name, - int component, - RawTransport* transport, - rtc::Thread *worker_thread, - PortAllocator *allocator); - virtual ~RawTransportChannel(); - - // Implementation of normal channel packet sending. - virtual int SendPacket(const char *data, size_t len, - const rtc::PacketOptions& options, int flags); - virtual int SetOption(rtc::Socket::Option opt, int value); - virtual bool GetOption(rtc::Socket::Option opt, int* value); - virtual int GetError(); - - // Implements TransportChannelImpl. - virtual Transport* GetTransport() { return raw_transport_; } - virtual TransportChannelState GetState() const { - return TransportChannelState::STATE_COMPLETED; - } - virtual void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) {} - virtual void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) {} - - // Creates an allocator session to start figuring out which type of - // port we should send to the other client. This will send - // SignalAvailableCandidate once we have decided. - virtual void Connect(); - - // Resets state back to unconnected. - virtual void Reset(); - - // We don't actually worry about signaling since we can't send new candidates. - virtual void OnSignalingReady() {} - - // Handles a message setting the remote address. We are writable once we - // have this since we now know where to send. - virtual void OnCandidate(const Candidate& candidate); - - void OnRemoteAddress(const rtc::SocketAddress& remote_address); - - // Below ICE specific virtual methods not implemented. - virtual IceRole GetIceRole() const { return ICEROLE_UNKNOWN; } - virtual void SetIceRole(IceRole role) {} - virtual void SetIceTiebreaker(uint64 tiebreaker) {} - - virtual bool GetIceProtocolType(IceProtocolType* type) const { return false; } - virtual void SetIceProtocolType(IceProtocolType type) {} - - virtual void SetIceUfrag(const std::string& ice_ufrag) {} - virtual void SetIcePwd(const std::string& ice_pwd) {} - virtual void SetRemoteIceMode(IceMode mode) {} - virtual size_t GetConnectionCount() const { return 1; } - - virtual bool GetStats(ConnectionInfos* infos) { - return false; - } - - // DTLS methods. - virtual bool IsDtlsActive() const { return false; } - - // Default implementation. - virtual bool GetSslRole(rtc::SSLRole* role) const { - return false; - } - - virtual bool SetSslRole(rtc::SSLRole role) { - return false; - } - - // Set up the ciphers to use for DTLS-SRTP. - virtual bool SetSrtpCiphers(const std::vector& ciphers) { - return false; - } - - // Find out which DTLS-SRTP cipher was negotiated. - virtual bool GetSrtpCipher(std::string* cipher) { - return false; - } - - // Find out which DTLS cipher was negotiated. - virtual bool GetSslCipher(std::string* cipher) { - return false; - } - - // Returns false because the channel is not DTLS. - virtual bool GetLocalIdentity(rtc::SSLIdentity** identity) const { - return false; - } - - virtual bool GetRemoteCertificate(rtc::SSLCertificate** cert) const { - return false; - } - - // Allows key material to be extracted for external encryption. - virtual bool ExportKeyingMaterial( - const std::string& label, - const uint8* context, - size_t context_len, - bool use_context, - uint8* result, - size_t result_len) { - return false; - } - - virtual bool SetLocalIdentity(rtc::SSLIdentity* identity) { - return false; - } - - // Set DTLS Remote fingerprint. Must be after local identity set. - virtual bool SetRemoteFingerprint( - const std::string& digest_alg, - const uint8* digest, - size_t digest_len) { - return false; - } - - void SetReceivingTimeout(int timeout) override {} - - private: - RawTransport* raw_transport_; - rtc::Thread *worker_thread_; - PortAllocator* allocator_; - PortAllocatorSession* allocator_session_; - StunPort* stun_port_; - RelayPort* relay_port_; - PortInterface* port_; - bool use_relay_; - rtc::SocketAddress remote_address_; - - // Called when the allocator creates another port. - void OnPortReady(PortAllocatorSession* session, PortInterface* port); - - // Called when one of the ports we are using has determined its address. - void OnCandidatesReady(PortAllocatorSession *session, - const std::vector& candidates); - - // Called once we have chosen the port to use for communication with the - // other client. This will send its address and prepare the port for use. - void SetPort(PortInterface* port); - - // Called once we have a port and a remote address. This will set mark the - // channel as writable and signal the route to the client. - void SetWritable(); - - // Called when we receive a packet from the other client. - void OnReadPacket(PortInterface* port, const char* data, size_t size, - const rtc::SocketAddress& addr); - - // Handles a message to destroy unused ports. - virtual void OnMessage(rtc::Message *msg); - - DISALLOW_COPY_AND_ASSIGN(RawTransportChannel); -}; - -} // namespace cricket - -#endif // defined(FEATURE_ENABLE_PSTN) -#endif // WEBRTC_P2P_BASE_RAWTRANSPORTCHANNEL_H_ +// TODO(pthatcher): Remove this file once Chrome's build files no +// longer refer to it. diff --git a/webrtc/p2p/base/session.cc b/webrtc/p2p/base/session.cc index fb8480ee5e..6c185113f9 100644 --- a/webrtc/p2p/base/session.cc +++ b/webrtc/p2p/base/session.cc @@ -37,10 +37,6 @@ TransportProxy::~TransportProxy() { } } -const std::string& TransportProxy::type() const { - return transport_->get()->type(); -} - TransportChannel* TransportProxy::GetChannel(int component) { ASSERT(rtc::Thread::Current() == worker_thread_); return GetChannelProxy(component); @@ -339,7 +335,6 @@ BaseSession::BaseSession(rtc::Thread* signaling_thread, port_allocator_(port_allocator), sid_(sid), content_type_(content_type), - transport_type_(NS_GINGLE_P2P), initiator_(initiator), identity_(NULL), ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_10), @@ -578,7 +573,6 @@ void BaseSession::DestroyTransportProxy( } Transport* BaseSession::CreateTransport(const std::string& content_name) { - ASSERT(transport_type_ == NS_GINGLE_P2P); Transport* transport = new DtlsTransport( signaling_thread(), worker_thread(), content_name, port_allocator(), identity_); @@ -778,8 +772,7 @@ void BaseSession::LogState(State old_state, State new_state) { LOG(LS_INFO) << "Session:" << id() << " Old state:" << StateToString(old_state) << " New state:" << StateToString(new_state) - << " Type:" << content_type() - << " Transport:" << transport_type(); + << " Type:" << content_type(); } // static diff --git a/webrtc/p2p/base/session.h b/webrtc/p2p/base/session.h index 19ce3611b0..79ccafd293 100644 --- a/webrtc/p2p/base/session.h +++ b/webrtc/p2p/base/session.h @@ -238,8 +238,6 @@ class BaseSession : public sigslot::has_slots<>, // from local/remote_description(). Remove these functions and members. // Returns the XML namespace identifying the type of this session. const std::string& content_type() const { return content_type_; } - // Returns the XML namespace identifying the transport used for this session. - const std::string& transport_type() const { return transport_type_; } // Indicates whether we initiated this session. bool initiator() const { return initiator_; } @@ -445,7 +443,6 @@ class BaseSession : public sigslot::has_slots<>, PortAllocator* const port_allocator_; const std::string sid_; const std::string content_type_; - const std::string transport_type_; bool initiator_; rtc::SSLIdentity* identity_; rtc::SSLProtocolVersion ssl_max_version_; diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index b96ae842b4..8b68a976b6 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -56,25 +56,6 @@ struct ChannelParams : public rtc::MessageData { Candidate* candidate; }; -static std::string IceProtoToString(TransportProtocol proto) { - std::string proto_str; - switch (proto) { - case ICEPROTO_GOOGLE: - proto_str = "gice"; - break; - case ICEPROTO_HYBRID: - proto_str = "hybrid"; - break; - case ICEPROTO_RFC5245: - proto_str = "ice"; - break; - default: - ASSERT(false); - break; - } - return proto_str; -} - static bool VerifyIceParams(const TransportDescription& desc) { // For legacy protocols. if (desc.ice_ufrag.empty() && desc.ice_pwd.empty()) @@ -119,12 +100,10 @@ static bool IceCredentialsChanged(const TransportDescription& old_desc, Transport::Transport(rtc::Thread* signaling_thread, rtc::Thread* worker_thread, const std::string& content_name, - const std::string& type, PortAllocator* allocator) : signaling_thread_(signaling_thread), worker_thread_(worker_thread), content_name_(content_name), - type_(type), allocator_(allocator), destroyed_(false), readable_(TRANSPORT_STATE_NONE), @@ -134,7 +113,6 @@ Transport::Transport(rtc::Thread* signaling_thread, connect_requested_(false), ice_role_(ICEROLE_UNKNOWN), tiebreaker_(0), - protocol_(ICEPROTO_HYBRID), remote_ice_mode_(ICEMODE_FULL), channel_receiving_timeout_(-1) { } @@ -352,7 +330,7 @@ void Transport::ConnectChannels_w() { // initiate request initiated by the remote. LOG(LS_INFO) << "Transport::ConnectChannels_w: No local description has " << "been set. Will generate one."; - TransportDescription desc(NS_GINGLE_P2P, std::vector(), + TransportDescription desc(std::vector(), rtc::CreateRandomString(ICE_UFRAG_LENGTH), rtc::CreateRandomString(ICE_PWD_LENGTH), ICEMODE_FULL, CONNECTIONROLE_NONE, NULL, @@ -828,21 +806,6 @@ bool Transport::SetRemoteTransportDescription_w( bool Transport::ApplyLocalTransportDescription_w(TransportChannelImpl* ch, std::string* error_desc) { ASSERT(worker_thread()->IsCurrent()); - // If existing protocol_type is HYBRID, we may have not chosen the final - // protocol type, so update the channel protocol type from the - // local description. Otherwise, skip updating the protocol type. - // We check for HYBRID to avoid accidental changes; in the case of a - // session renegotiation, the new offer will have the google-ice ICE option, - // so we need to make sure we don't switch back from ICE mode to HYBRID - // when this happens. - // There are some other ways we could have solved this, but this is the - // simplest. The ultimate solution will be to get rid of GICE altogether. - IceProtocolType protocol_type; - if (ch->GetIceProtocolType(&protocol_type) && - protocol_type == ICEPROTO_HYBRID) { - ch->SetIceProtocolType( - TransportProtocolFromDescription(local_description())); - } ch->SetIceCredentials(local_description_->ice_ufrag, local_description_->ice_pwd); return true; @@ -858,7 +821,6 @@ bool Transport::ApplyRemoteTransportDescription_w(TransportChannelImpl* ch, bool Transport::ApplyNegotiatedTransportDescription_w( TransportChannelImpl* channel, std::string* error_desc) { ASSERT(worker_thread()->IsCurrent()); - channel->SetIceProtocolType(protocol_); channel->SetRemoteIceMode(remote_ice_mode_); return true; } @@ -868,39 +830,6 @@ bool Transport::NegotiateTransportDescription_w(ContentAction local_role, ASSERT(worker_thread()->IsCurrent()); // TODO(ekr@rtfm.com): This is ICE-specific stuff. Refactor into // P2PTransport. - const TransportDescription* offer; - const TransportDescription* answer; - - if (local_role == CA_OFFER) { - offer = local_description_.get(); - answer = remote_description_.get(); - } else { - offer = remote_description_.get(); - answer = local_description_.get(); - } - - TransportProtocol offer_proto = TransportProtocolFromDescription(offer); - TransportProtocol answer_proto = TransportProtocolFromDescription(answer); - - // If offered protocol is gice/ice, then we expect to receive matching - // protocol in answer, anything else is treated as an error. - // HYBRID is not an option when offered specific protocol. - // If offered protocol is HYBRID and answered protocol is HYBRID then - // gice is preferred protocol. - // TODO(mallinath) - Answer from local or remote should't have both ice - // and gice support. It should always pick which protocol it wants to use. - // Once WebRTC stops supporting gice (for backward compatibility), HYBRID in - // answer must be treated as error. - if ((offer_proto == ICEPROTO_GOOGLE || offer_proto == ICEPROTO_RFC5245) && - (offer_proto != answer_proto)) { - std::ostringstream desc; - desc << "Offer and answer protocol mismatch: " - << IceProtoToString(offer_proto) - << " vs " - << IceProtoToString(answer_proto); - return BadTransportDescription(desc.str(), error_desc); - } - protocol_ = answer_proto == ICEPROTO_HYBRID ? ICEPROTO_GOOGLE : answer_proto; // If transport is in ICEROLE_CONTROLLED and remote end point supports only // ice_lite, this local end point should take CONTROLLING role. @@ -974,16 +903,4 @@ void Transport::OnMessage(rtc::Message* msg) { } } -// We're GICE if the namespace is NS_GOOGLE_P2P, or if NS_JINGLE_ICE_UDP is -// used and the GICE ice-option is set. -TransportProtocol TransportProtocolFromDescription( - const TransportDescription* desc) { - ASSERT(desc != NULL); - if (desc->transport_type == NS_JINGLE_ICE_UDP) { - return (desc->HasOption(ICE_OPTION_GICE)) ? - ICEPROTO_HYBRID : ICEPROTO_RFC5245; - } - return ICEPROTO_GOOGLE; -} - } // namespace cricket diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 6064bdb54d..4093240f3e 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -141,7 +141,6 @@ class Transport : public rtc::MessageHandler, Transport(rtc::Thread* signaling_thread, rtc::Thread* worker_thread, const std::string& content_name, - const std::string& type, PortAllocator* allocator); virtual ~Transport(); @@ -152,8 +151,6 @@ class Transport : public rtc::MessageHandler, // Returns the content_name of this transport. const std::string& content_name() const { return content_name_; } - // Returns the type of this transport. - const std::string& type() const { return type_; } // Returns the port allocator object for this transport. PortAllocator* port_allocator() { return allocator_; } @@ -211,8 +208,6 @@ class Transport : public rtc::MessageHandler, // Get a copy of the remote certificate in use by the specified channel. bool GetRemoteCertificate(rtc::SSLCertificate** cert); - TransportProtocol protocol() const { return protocol_; } - // Create, destroy, and lookup the channels of this type by their components. TransportChannelImpl* CreateChannel(int component); // Note: GetChannel may lead to race conditions, since the mutex is not held @@ -322,7 +317,7 @@ class Transport : public rtc::MessageHandler, std::string* error_desc); // Negotiates the transport parameters based on the current local and remote - // transport description, such at the version of ICE to use, and whether DTLS + // transport description, such as the ICE role to use, and whether DTLS // should be activated. // Derived classes can negotiate their specific parameters here, but must call // the base as well. @@ -448,7 +443,6 @@ class Transport : public rtc::MessageHandler, rtc::Thread* const signaling_thread_; rtc::Thread* const worker_thread_; const std::string content_name_; - const std::string type_; PortAllocator* const allocator_; bool destroyed_; TransportState readable_; @@ -458,7 +452,6 @@ class Transport : public rtc::MessageHandler, bool connect_requested_; IceRole ice_role_; uint64 tiebreaker_; - TransportProtocol protocol_; IceMode remote_ice_mode_; int channel_receiving_timeout_; rtc::scoped_ptr local_description_; @@ -475,9 +468,6 @@ class Transport : public rtc::MessageHandler, DISALLOW_COPY_AND_ASSIGN(Transport); }; -// Extract a TransportProtocol from a TransportDescription. -TransportProtocol TransportProtocolFromDescription( - const TransportDescription* desc); } // namespace cricket diff --git a/webrtc/p2p/base/transport_unittest.cc b/webrtc/p2p/base/transport_unittest.cc index 7b8df64eab..43b761a64c 100644 --- a/webrtc/p2p/base/transport_unittest.cc +++ b/webrtc/p2p/base/transport_unittest.cc @@ -107,8 +107,7 @@ TEST_F(TransportTest, TestDestroyAllClearsPosts) { TEST_F(TransportTest, TestChannelIceParameters) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); transport_->SetIceTiebreaker(99U); - cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_OFFER, NULL)); @@ -119,8 +118,7 @@ TEST_F(TransportTest, TestChannelIceParameters) { EXPECT_EQ(kIceUfrag1, channel_->ice_ufrag()); EXPECT_EQ(kIcePwd1, channel_->ice_pwd()); - cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_ANSWER, NULL)); @@ -150,8 +148,7 @@ TEST_F(TransportTest, TestIceControlledToControllingOnIceRestart) { EXPECT_TRUE(SetupChannel()); transport_->SetIceRole(cricket::ICEROLE_CONTROLLED); - cricket::TransportDescription desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription(desc, cricket::CA_OFFER, NULL)); @@ -160,8 +157,7 @@ TEST_F(TransportTest, TestIceControlledToControllingOnIceRestart) { NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLED, transport_->ice_role()); - cricket::TransportDescription new_local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2); + cricket::TransportDescription new_local_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc, cricket::CA_OFFER, NULL)); @@ -175,8 +171,7 @@ TEST_F(TransportTest, TestIceControllingToControlledOnIceRestart) { EXPECT_TRUE(SetupChannel()); transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); - cricket::TransportDescription desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(desc, cricket::CA_OFFER, NULL)); @@ -185,8 +180,7 @@ TEST_F(TransportTest, TestIceControllingToControlledOnIceRestart) { NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); - cricket::TransportDescription new_local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2); + cricket::TransportDescription new_local_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc, cricket::CA_ANSWER, NULL)); @@ -200,14 +194,13 @@ TEST_F(TransportTest, TestIceControllingOnIceRestartIfRemoteIsIceLite) { EXPECT_TRUE(SetupChannel()); transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); - cricket::TransportDescription desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(desc, cricket::CA_OFFER, NULL)); cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), + std::vector(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, cricket::CONNECTIONROLE_NONE, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, @@ -216,8 +209,7 @@ TEST_F(TransportTest, TestIceControllingOnIceRestartIfRemoteIsIceLite) { EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); - cricket::TransportDescription new_local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2); + cricket::TransportDescription new_local_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc, cricket::CA_ANSWER, NULL)); @@ -228,15 +220,13 @@ TEST_F(TransportTest, TestIceControllingOnIceRestartIfRemoteIsIceLite) { // This test verifies that the Completed and Failed states can be reached. TEST_F(TransportTest, TestChannelCompletedAndFailed) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); - cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_OFFER, NULL)); EXPECT_TRUE(SetupChannel()); - cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_ANSWER, NULL)); @@ -266,14 +256,13 @@ TEST_F(TransportTest, TestChannelCompletedAndFailed) { TEST_F(TransportTest, TestSetRemoteIceLiteInOffer) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLED); cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), + std::vector(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, cricket::CONNECTIONROLE_ACTPASS, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_OFFER, NULL)); - cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_ANSWER, NULL)); @@ -286,8 +275,7 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInOffer) { // Tests ice-lite in remote answer. TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); - cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); + cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_OFFER, NULL)); @@ -297,7 +285,7 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { // Channels will be created in ICEFULL_MODE. EXPECT_EQ(cricket::ICEMODE_FULL, channel_->remote_ice_mode()); cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), + std::vector(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, cricket::CONNECTIONROLE_NONE, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h index 705550b52a..75d56aa09b 100644 --- a/webrtc/p2p/base/transportchannelimpl.h +++ b/webrtc/p2p/base/transportchannelimpl.h @@ -21,6 +21,12 @@ namespace cricket { class Candidate; +// TODO(pthatcher): Remove this once it's no longer used in +// remoting/protocol/libjingle_transport_factory.cc +enum IceProtocolType { + ICEPROTO_RFC5245 // Standard RFC 5245 version of ICE. +}; + // Base class for real implementations of TransportChannel. This includes some // methods called only by Transport, which do not need to be exposed to the // client. @@ -36,9 +42,9 @@ class TransportChannelImpl : public TransportChannel { virtual IceRole GetIceRole() const = 0; virtual void SetIceRole(IceRole role) = 0; virtual void SetIceTiebreaker(uint64 tiebreaker) = 0; - // To toggle G-ICE/ICE. - virtual bool GetIceProtocolType(IceProtocolType* type) const = 0; - virtual void SetIceProtocolType(IceProtocolType type) = 0; + // TODO(pthatcher): Remove this once it's no longer called in + // remoting/protocol/libjingle_transport_factory.cc + virtual void SetIceProtocolType(IceProtocolType type) {} // SetIceCredentials only need to be implemented by the ICE // transport channels. Non-ICE transport channels can just ignore. // The ufrag and pwd should be set before the Connect() is called. diff --git a/webrtc/p2p/base/transportdescription.h b/webrtc/p2p/base/transportdescription.h index 5ab1cd6a12..8ea1f4bc2e 100644 --- a/webrtc/p2p/base/transportdescription.h +++ b/webrtc/p2p/base/transportdescription.h @@ -35,16 +35,6 @@ enum SecurePolicy { SEC_REQUIRED }; -// The transport protocol we've elected to use. -enum TransportProtocol { - ICEPROTO_GOOGLE, // Google version of ICE protocol. - ICEPROTO_HYBRID, // ICE, but can fall back to the Google version. - ICEPROTO_RFC5245 // Standard RFC 5245 version of ICE. -}; -// The old name for TransportProtocol. -// TODO(juberti): remove this. -typedef TransportProtocol IceProtocolType; - // Whether our side of the call is driving the negotiation, or the other side. enum IceRole { ICEROLE_CONTROLLING = 0, @@ -86,33 +76,28 @@ struct TransportDescription { : ice_mode(ICEMODE_FULL), connection_role(CONNECTIONROLE_NONE) {} - TransportDescription(const std::string& transport_type, - const std::vector& transport_options, + TransportDescription(const std::vector& transport_options, const std::string& ice_ufrag, const std::string& ice_pwd, IceMode ice_mode, ConnectionRole role, const rtc::SSLFingerprint* identity_fingerprint, const Candidates& candidates) - : transport_type(transport_type), - transport_options(transport_options), + : transport_options(transport_options), ice_ufrag(ice_ufrag), ice_pwd(ice_pwd), ice_mode(ice_mode), connection_role(role), identity_fingerprint(CopyFingerprint(identity_fingerprint)), candidates(candidates) {} - TransportDescription(const std::string& transport_type, - const std::string& ice_ufrag, + TransportDescription(const std::string& ice_ufrag, const std::string& ice_pwd) - : transport_type(transport_type), - ice_ufrag(ice_ufrag), + : ice_ufrag(ice_ufrag), ice_pwd(ice_pwd), ice_mode(ICEMODE_FULL), connection_role(CONNECTIONROLE_NONE) {} TransportDescription(const TransportDescription& from) - : transport_type(from.transport_type), - transport_options(from.transport_options), + : transport_options(from.transport_options), ice_ufrag(from.ice_ufrag), ice_pwd(from.ice_pwd), ice_mode(from.ice_mode), @@ -125,7 +110,6 @@ struct TransportDescription { if (this == &from) return *this; - transport_type = from.transport_type; transport_options = from.transport_options; ice_ufrag = from.ice_ufrag; ice_pwd = from.ice_pwd; @@ -155,7 +139,6 @@ struct TransportDescription { return new rtc::SSLFingerprint(*from); } - std::string transport_type; // xmlns of std::vector transport_options; std::string ice_ufrag; std::string ice_pwd; diff --git a/webrtc/p2p/base/transportdescriptionfactory.cc b/webrtc/p2p/base/transportdescriptionfactory.cc index 1230ba52c1..7654fdfcf5 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.cc +++ b/webrtc/p2p/base/transportdescriptionfactory.cc @@ -19,11 +19,8 @@ namespace cricket { -static TransportProtocol kDefaultProtocol = ICEPROTO_RFC5245; - TransportDescriptionFactory::TransportDescriptionFactory() - : protocol_(kDefaultProtocol), - secure_(SEC_DISABLED), + : secure_(SEC_DISABLED), identity_(NULL) { } @@ -32,16 +29,6 @@ TransportDescription* TransportDescriptionFactory::CreateOffer( const TransportDescription* current_description) const { rtc::scoped_ptr desc(new TransportDescription()); - // Set the transport type depending on the selected protocol. - if (protocol_ == ICEPROTO_RFC5245) { - desc->transport_type = NS_JINGLE_ICE_UDP; - } else if (protocol_ == ICEPROTO_HYBRID) { - desc->transport_type = NS_JINGLE_ICE_UDP; - desc->AddOption(ICE_OPTION_GICE); - } else if (protocol_ == ICEPROTO_GOOGLE) { - desc->transport_type = NS_GINGLE_P2P; - } - // Generate the ICE credentials if we don't already have them. if (!current_description || options.ice_restart) { desc->ice_ufrag = rtc::CreateRandomString(ICE_UFRAG_LENGTH); @@ -67,33 +54,14 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( const TransportDescription* offer, const TransportOptions& options, const TransportDescription* current_description) const { - // A NULL offer is treated as a GICE transport description. // TODO(juberti): Figure out why we get NULL offers, and fix this upstream. - rtc::scoped_ptr desc(new TransportDescription()); - - // Figure out which ICE variant to negotiate; prefer RFC 5245 ICE, but fall - // back to G-ICE if needed. Note that we never create a hybrid answer, since - // we know what the other side can support already. - if (offer && offer->transport_type == NS_JINGLE_ICE_UDP && - (protocol_ == ICEPROTO_RFC5245 || protocol_ == ICEPROTO_HYBRID)) { - // Offer is ICE or hybrid, we support ICE or hybrid: use ICE. - desc->transport_type = NS_JINGLE_ICE_UDP; - } else if (offer && offer->transport_type == NS_JINGLE_ICE_UDP && - offer->HasOption(ICE_OPTION_GICE) && - protocol_ == ICEPROTO_GOOGLE) { - desc->transport_type = NS_GINGLE_P2P; - // Offer is hybrid, we support GICE: use GICE. - } else if ((!offer || offer->transport_type == NS_GINGLE_P2P) && - (protocol_ == ICEPROTO_HYBRID || protocol_ == ICEPROTO_GOOGLE)) { - // Offer is GICE, we support hybrid or GICE: use GICE. - desc->transport_type = NS_GINGLE_P2P; - } else { - // Mismatch. - LOG(LS_WARNING) << "Failed to create TransportDescription answer " - "because of incompatible transport types"; + if (!offer) { + LOG(LS_WARNING) << "Failed to create TransportDescription answer " << + "because offer is NULL"; return NULL; } + rtc::scoped_ptr desc(new TransportDescription()); // Generate the ICE credentials if we don't already have them or ice is // being restarted. if (!current_description || options.ice_restart) { @@ -157,4 +125,3 @@ bool TransportDescriptionFactory::SetSecurityInfo( } } // namespace cricket - diff --git a/webrtc/p2p/base/transportdescriptionfactory.h b/webrtc/p2p/base/transportdescriptionfactory.h index a137f72115..2bd86617ed 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.h +++ b/webrtc/p2p/base/transportdescriptionfactory.h @@ -36,8 +36,6 @@ class TransportDescriptionFactory { // The identity to use when setting up DTLS. rtc::SSLIdentity* identity() const { return identity_; } - // Specifies the transport protocol to be use. - void set_protocol(TransportProtocol protocol) { protocol_ = protocol; } // Specifies the transport security policy to use. void set_secure(SecurePolicy s) { secure_ = s; } // Specifies the identity to use (only used when secure is not SEC_DISABLED). @@ -56,7 +54,6 @@ class TransportDescriptionFactory { bool SetSecurityInfo(TransportDescription* description, ConnectionRole role) const; - TransportProtocol protocol_; SecurePolicy secure_; rtc::SSLIdentity* identity_; }; diff --git a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc index 48267b57bb..16a539f2e6 100644 --- a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc +++ b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc @@ -30,11 +30,10 @@ class TransportDescriptionFactoryTest : public testing::Test { id2_(new rtc::FakeSSLIdentity("User2")) { } - void CheckDesc(const TransportDescription* desc, const std::string& type, + void CheckDesc(const TransportDescription* desc, const std::string& opt, const std::string& ice_ufrag, const std::string& ice_pwd, const std::string& dtls_alg) { ASSERT_TRUE(desc != NULL); - EXPECT_EQ(type, desc->transport_type); EXPECT_EQ(!opt.empty(), desc->HasOption(opt)); if (ice_ufrag.empty() && ice_pwd.empty()) { EXPECT_EQ(static_cast(cricket::ICE_UFRAG_LENGTH), @@ -118,61 +117,37 @@ class TransportDescriptionFactoryTest : public testing::Test { scoped_ptr id2_; }; -// Test that in the default case, we generate the expected G-ICE offer. -TEST_F(TransportDescriptionFactoryTest, TestOfferGice) { - f1_.set_protocol(cricket::ICEPROTO_GOOGLE); +TEST_F(TransportDescriptionFactoryTest, TestOfferDefault) { scoped_ptr desc(f1_.CreateOffer( TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_GINGLE_P2P, "", "", "", ""); + CheckDesc(desc.get(), "", "", "", ""); } -// Test generating a hybrid offer. -TEST_F(TransportDescriptionFactoryTest, TestOfferHybrid) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); - scoped_ptr desc(f1_.CreateOffer( - TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "google-ice", "", "", ""); -} - -// Test generating an ICE-only offer. -TEST_F(TransportDescriptionFactoryTest, TestOfferIce) { - f1_.set_protocol(cricket::ICEPROTO_RFC5245); - scoped_ptr desc(f1_.CreateOffer( - TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); -} - -// Test generating a hybrid offer with DTLS. -TEST_F(TransportDescriptionFactoryTest, TestOfferHybridDtls) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); +TEST_F(TransportDescriptionFactoryTest, TestOfferDtls) { f1_.set_secure(cricket::SEC_ENABLED); f1_.set_identity(id1_.get()); std::string digest_alg; ASSERT_TRUE(id1_->certificate().GetSignatureDigestAlgorithm(&digest_alg)); scoped_ptr desc(f1_.CreateOffer( TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "google-ice", "", "", - digest_alg); + CheckDesc(desc.get(), "", "", "", digest_alg); // Ensure it also works with SEC_REQUIRED. f1_.set_secure(cricket::SEC_REQUIRED); desc.reset(f1_.CreateOffer(TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "google-ice", "", "", - digest_alg); + CheckDesc(desc.get(), "", "", "", digest_alg); } -// Test generating a hybrid offer with DTLS fails with no identity. -TEST_F(TransportDescriptionFactoryTest, TestOfferHybridDtlsWithNoIdentity) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); +// Test generating an offer with DTLS fails with no identity. +TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsWithNoIdentity) { f1_.set_secure(cricket::SEC_ENABLED); scoped_ptr desc(f1_.CreateOffer( TransportOptions(), NULL)); ASSERT_TRUE(desc.get() == NULL); } -// Test updating a hybrid offer with DTLS to pick ICE. +// Test updating an offer with DTLS to pick ICE. // The ICE credentials should stay the same in the new offer. -TEST_F(TransportDescriptionFactoryTest, TestOfferHybridDtlsReofferIceDtls) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); +TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) { f1_.set_secure(cricket::SEC_ENABLED); f1_.set_identity(id1_.get()); std::string digest_alg; @@ -180,104 +155,26 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferHybridDtlsReofferIceDtls) { scoped_ptr old_desc(f1_.CreateOffer( TransportOptions(), NULL)); ASSERT_TRUE(old_desc.get() != NULL); - f1_.set_protocol(cricket::ICEPROTO_RFC5245); scoped_ptr desc( f1_.CreateOffer(TransportOptions(), old_desc.get())); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", + CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, digest_alg); } -// Test that we can answer a GICE offer with GICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerGiceToGice) { - f1_.set_protocol(cricket::ICEPROTO_GOOGLE); - f2_.set_protocol(cricket::ICEPROTO_GOOGLE); +TEST_F(TransportDescriptionFactoryTest, TestAnswerDefault) { scoped_ptr offer(f1_.CreateOffer( TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); scoped_ptr desc(f2_.CreateAnswer( offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_GINGLE_P2P, "", "", "", ""); - // Should get the same result when answering as hybrid. - f2_.set_protocol(cricket::ICEPROTO_HYBRID); + CheckDesc(desc.get(), "", "", "", ""); desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_GINGLE_P2P, "", "", "", ""); -} - -// Test that we can answer a hybrid offer with GICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerGiceToHybrid) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); - f2_.set_protocol(cricket::ICEPROTO_GOOGLE); - scoped_ptr offer(f1_.CreateOffer( - TransportOptions(), NULL)); - ASSERT_TRUE(offer.get() != NULL); - scoped_ptr desc( - f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_GINGLE_P2P, "", "", "", ""); -} - -// Test that we can answer a hybrid offer with ICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerIceToHybrid) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); - f2_.set_protocol(cricket::ICEPROTO_RFC5245); - scoped_ptr offer(f1_.CreateOffer( - TransportOptions(), NULL)); - ASSERT_TRUE(offer.get() != NULL); - scoped_ptr desc( - f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); - // Should get the same result when answering as hybrid. - f2_.set_protocol(cricket::ICEPROTO_HYBRID); - desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), - NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); -} - -// Test that we can answer an ICE offer with ICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerIceToIce) { - f1_.set_protocol(cricket::ICEPROTO_RFC5245); - f2_.set_protocol(cricket::ICEPROTO_RFC5245); - scoped_ptr offer(f1_.CreateOffer( - TransportOptions(), NULL)); - ASSERT_TRUE(offer.get() != NULL); - scoped_ptr desc(f2_.CreateAnswer( - offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); - // Should get the same result when answering as hybrid. - f2_.set_protocol(cricket::ICEPROTO_HYBRID); - desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), - NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); -} - -// Test that we can't answer a GICE offer with ICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerIceToGice) { - f1_.set_protocol(cricket::ICEPROTO_GOOGLE); - f2_.set_protocol(cricket::ICEPROTO_RFC5245); - scoped_ptr offer( - f1_.CreateOffer(TransportOptions(), NULL)); - ASSERT_TRUE(offer.get() != NULL); - scoped_ptr desc( - f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - ASSERT_TRUE(desc.get() == NULL); -} - -// Test that we can't answer an ICE offer with GICE. -TEST_F(TransportDescriptionFactoryTest, TestAnswerGiceToIce) { - f1_.set_protocol(cricket::ICEPROTO_RFC5245); - f2_.set_protocol(cricket::ICEPROTO_GOOGLE); - scoped_ptr offer( - f1_.CreateOffer(TransportOptions(), NULL)); - ASSERT_TRUE(offer.get() != NULL); - scoped_ptr desc(f2_.CreateAnswer( - offer.get(), TransportOptions(), NULL)); - ASSERT_TRUE(desc.get() == NULL); + CheckDesc(desc.get(), "", "", "", ""); } // Test that we can update an answer properly; ICE credentials shouldn't change. -TEST_F(TransportDescriptionFactoryTest, TestAnswerIceToIceReanswer) { - f1_.set_protocol(cricket::ICEPROTO_RFC5245); - f2_.set_protocol(cricket::ICEPROTO_RFC5245); +TEST_F(TransportDescriptionFactoryTest, TestReanswer) { scoped_ptr offer( f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); @@ -288,29 +185,25 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerIceToIceReanswer) { f2_.CreateAnswer(offer.get(), TransportOptions(), old_desc.get())); ASSERT_TRUE(desc.get() != NULL); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", + CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, ""); } // Test that we handle answering an offer with DTLS with no DTLS. -TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridToHybridDtls) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); +TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) { f1_.set_secure(cricket::SEC_ENABLED); f1_.set_identity(id1_.get()); - f2_.set_protocol(cricket::ICEPROTO_HYBRID); scoped_ptr offer( f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); scoped_ptr desc( f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); + CheckDesc(desc.get(), "", "", "", ""); } // Test that we handle answering an offer without DTLS if we have DTLS enabled, // but fail if we require DTLS. -TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridDtlsToHybrid) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); - f2_.set_protocol(cricket::ICEPROTO_HYBRID); +TEST_F(TransportDescriptionFactoryTest, TestAnswerNoDtlsToDtls) { f2_.set_secure(cricket::SEC_ENABLED); f2_.set_identity(id2_.get()); scoped_ptr offer( @@ -318,7 +211,7 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridDtlsToHybrid) { ASSERT_TRUE(offer.get() != NULL); scoped_ptr desc( f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", ""); + CheckDesc(desc.get(), "", "", "", ""); f2_.set_secure(cricket::SEC_REQUIRED); desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); @@ -327,12 +220,10 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridDtlsToHybrid) { // Test that we handle answering an DTLS offer with DTLS, both if we have // DTLS enabled and required. -TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridDtlsToHybridDtls) { - f1_.set_protocol(cricket::ICEPROTO_HYBRID); +TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) { f1_.set_secure(cricket::SEC_ENABLED); f1_.set_identity(id1_.get()); - f2_.set_protocol(cricket::ICEPROTO_HYBRID); f2_.set_secure(cricket::SEC_ENABLED); f2_.set_identity(id2_.get()); // f2_ produces the answer that is being checked in this test, so the @@ -345,11 +236,11 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerHybridDtlsToHybridDtls) { ASSERT_TRUE(offer.get() != NULL); scoped_ptr desc( f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", digest_alg2); + CheckDesc(desc.get(), "", "", "", digest_alg2); f2_.set_secure(cricket::SEC_REQUIRED); desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); - CheckDesc(desc.get(), cricket::NS_JINGLE_ICE_UDP, "", "", "", digest_alg2); + CheckDesc(desc.get(), "", "", "", digest_alg2); } // Test that ice ufrag and password is changed in an updated offer and answer diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index d0a58ad395..2041d4f2b6 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -201,11 +201,7 @@ class TurnPortTest : public testing::Test, kIceUfrag1, kIcePwd1, server_address, credentials, 0, std::string())); - // Set ICE protocol type to ICEPROTO_RFC5245, as port by default will be - // in Hybrid mode. Protocol type is necessary to send correct type STUN ping - // messages. // This TURN port will be the controlling. - turn_port_->SetIceProtocolType(cricket::ICEPROTO_RFC5245); turn_port_->SetIceRole(cricket::ICEROLE_CONTROLLING); ConnectSignals(); } @@ -223,11 +219,7 @@ class TurnPortTest : public testing::Test, kIceUfrag1, kIcePwd1, server_address, credentials, 0, origin)); - // Set ICE protocol type to ICEPROTO_RFC5245, as port by default will be - // in Hybrid mode. Protocol type is necessary to send correct type STUN ping - // messages. // This TURN port will be the controlling. - turn_port_->SetIceProtocolType(cricket::ICEPROTO_RFC5245); turn_port_->SetIceRole(cricket::ICEROLE_CONTROLLING); ConnectSignals(); } @@ -249,11 +241,7 @@ class TurnPortTest : public testing::Test, turn_port_.reset(cricket::TurnPort::Create( main_, &socket_factory_, &network_, socket_.get(), kIceUfrag1, kIcePwd1, server_address, credentials, 0, std::string())); - // Set ICE protocol type to ICEPROTO_RFC5245, as port by default will be - // in Hybrid mode. Protocol type is necessary to send correct type STUN ping - // messages. // This TURN port will be the controlling. - turn_port_->SetIceProtocolType(cricket::ICEPROTO_RFC5245); turn_port_->SetIceRole(cricket::ICEROLE_CONTROLLING); ConnectSignals(); } @@ -273,9 +261,7 @@ class TurnPortTest : public testing::Test, kLocalAddr2.ipaddr(), 0, 0, kIceUfrag2, kIcePwd2, std::string(), false)); - // Set protocol type to RFC5245, as turn port is also in same mode. // UDP port will be controlled. - udp_port_->SetIceProtocolType(cricket::ICEPROTO_RFC5245); udp_port_->SetIceRole(cricket::ICEROLE_CONTROLLED); udp_port_->SignalPortComplete.connect( this, &TurnPortTest::OnUdpPortComplete); diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index fe263f597f..2a9ee3b8ba 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -692,14 +692,6 @@ AllocationSequence::AllocationSequence(BasicPortAllocatorSession* session, } bool AllocationSequence::Init() { - if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET) && - !IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_UFRAG)) { - LOG(LS_ERROR) << "Shared socket option can't be set without " - << "shared ufrag."; - ASSERT(false); - return false; - } - if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { udp_socket_.reset(session_->socket_factory()->CreateUdpSocket( rtc::SocketAddress(ip_, 0), session_->allocator()->min_port(), diff --git a/webrtc/p2p/client/connectivitychecker.cc b/webrtc/p2p/client/connectivitychecker.cc index 1fb9165670..8a7fc0d740 100644 --- a/webrtc/p2p/client/connectivitychecker.cc +++ b/webrtc/p2p/client/connectivitychecker.cc @@ -115,9 +115,6 @@ bool ConnectivityChecker::Initialize() { socket_factory_.reset(CreateSocketFactory(worker_)); port_allocator_.reset(CreatePortAllocator(network_manager_.get(), user_agent_, relay_token_)); - uint32 new_allocator_flags = port_allocator_->flags(); - new_allocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; - port_allocator_->set_flags(new_allocator_flags); return true; } diff --git a/webrtc/p2p/client/fakeportallocator.h b/webrtc/p2p/client/fakeportallocator.h index bd8532144f..9e53bc04aa 100644 --- a/webrtc/p2p/client/fakeportallocator.h +++ b/webrtc/p2p/client/fakeportallocator.h @@ -33,7 +33,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { const std::string& ice_ufrag, const std::string& ice_pwd) : PortAllocatorSession(content_name, component, ice_ufrag, ice_pwd, - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG), + cricket::kDefaultPortAllocatorFlags), worker_thread_(worker_thread), factory_(factory), network_("network", "unittest", diff --git a/webrtc/p2p/client/httpportallocator.cc b/webrtc/p2p/client/httpportallocator.cc index c072da27e1..b36de5671d 100644 --- a/webrtc/p2p/client/httpportallocator.cc +++ b/webrtc/p2p/client/httpportallocator.cc @@ -168,12 +168,10 @@ void HttpPortAllocatorSessionBase::TryCreateRelaySession() { std::string HttpPortAllocatorSessionBase::GetSessionRequestUrl() { std::string url = std::string(HttpPortAllocator::kCreateSessionURL); - if (allocator()->flags() & PORTALLOCATOR_ENABLE_SHARED_UFRAG) { - ASSERT(!username().empty()); - ASSERT(!password().empty()); - url = url + "?username=" + rtc::s_url_encode(username()) + - "&password=" + rtc::s_url_encode(password()); - } + ASSERT(!username().empty()); + ASSERT(!password().empty()); + url = url + "?username=" + rtc::s_url_encode(username()) + + "&password=" + rtc::s_url_encode(password()); return url; } diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index b6de0c215a..5614425573 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -253,7 +253,6 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { } session_->set_flags(session_->flags() | cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); allocator().set_allow_tcp_listen(false); session_->StartGettingPorts(); @@ -777,8 +776,7 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithRelayOnly) { TEST_F(PortAllocatorTest, TestCandidateFilterWithHostOnly) { AddInterface(kClientAddr); - allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); + allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); allocator().set_candidate_filter(cricket::CF_HOST); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -795,8 +793,7 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnly) { AddInterface(kPrivateAddr); ResetWithStunServerAndNat(kStunAddr); - allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); + allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); allocator().set_candidate_filter(cricket::CF_REFLEXIVE); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -816,8 +813,7 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnly) { // Host is not behind the NAT. TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnlyAndNoNAT) { AddInterface(kClientAddr); - allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | - cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); + allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); allocator().set_candidate_filter(cricket::CF_REFLEXIVE); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -830,11 +826,8 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnlyAndNoNAT) { } } -// Test that when the PORTALLOCATOR_ENABLE_SHARED_UFRAG is enabled we got same -// ufrag and pwd for the collected candidates. +// Test that we get the same ufrag and pwd for all candidates. TEST_F(PortAllocatorTest, TestEnableSharedUfrag) { - allocator().set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); AddInterface(kClientAddr); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -854,30 +847,6 @@ TEST_F(PortAllocatorTest, TestEnableSharedUfrag) { EXPECT_TRUE(candidate_allocation_done_); } -// Test that when the PORTALLOCATOR_ENABLE_SHARED_UFRAG isn't enabled we got -// different ufrag and pwd for the collected candidates. -TEST_F(PortAllocatorTest, TestDisableSharedUfrag) { - allocator().set_flags(allocator().flags() & - ~cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); - AddInterface(kClientAddr); - EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout); - EXPECT_PRED5(CheckCandidate, candidates_[0], - cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", kClientAddr); - EXPECT_PRED5(CheckCandidate, candidates_[1], - cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp", kClientAddr); - EXPECT_EQ(4U, ports_.size()); - // Port should generate random ufrag and pwd. - EXPECT_NE(kIceUfrag0, candidates_[0].username()); - EXPECT_NE(kIceUfrag0, candidates_[1].username()); - EXPECT_NE(candidates_[0].username(), candidates_[1].username()); - EXPECT_NE(kIcePwd0, candidates_[0].password()); - EXPECT_NE(kIcePwd0, candidates_[1].password()); - EXPECT_NE(candidates_[0].password(), candidates_[1].password()); - EXPECT_TRUE(candidate_allocation_done_); -} - // Test that when PORTALLOCATOR_ENABLE_SHARED_SOCKET is enabled only one port // is allocated for udp and stun. Also verify there is only one candidate // (local) if stun candidate is same as local candidate, which will be the case @@ -885,7 +854,6 @@ TEST_F(PortAllocatorTest, TestDisableSharedUfrag) { TEST_F(PortAllocatorTest, TestSharedSocketWithoutNat) { AddInterface(kClientAddr); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -904,7 +872,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNat) { ResetWithStunServerAndNat(kStunAddr); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); session_->StartGettingPorts(); @@ -929,7 +896,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) { allocator_->set_step_delay(cricket::kMinimumStepDelay); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -967,7 +933,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithServerAddressResolve) { allocator_->set_step_delay(cricket::kMinimumStepDelay); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -987,7 +952,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurn) { AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -1026,7 +990,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnAsStun) { // webrtc issue 3537. allocator_->set_step_delay(0); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -1061,7 +1024,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnTcpOnly) { AddTurnServers(rtc::SocketAddress(), kTurnTcpIntAddr); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -1094,7 +1056,6 @@ TEST_F(PortAllocatorTest, TestNonSharedSocketWithNatUsingTurnAsStun) { AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_DISABLE_TCP); EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); @@ -1133,7 +1094,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnAndStun) { AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); allocator_->set_flags(allocator().flags() | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | cricket::PORTALLOCATOR_DISABLE_TCP); @@ -1163,7 +1123,6 @@ TEST_F(PortAllocatorTest, TestSharedSocketNoUdpAllowed) { allocator().set_flags(allocator().flags() | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr); AddInterface(kClientAddr); @@ -1188,7 +1147,6 @@ TEST_F(PortAllocatorTest, TestNetworkPermissionBlocked) { allocator().set_flags(allocator().flags() | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); EXPECT_EQ(0U, allocator_->flags() & cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION); @@ -1207,7 +1165,6 @@ TEST_F(PortAllocatorTest, TestEnableIPv6Addresses) { allocator().set_flags(allocator().flags() | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_ENABLE_IPV6 | - cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); AddInterface(kClientIPv6Addr); AddInterface(kClientAddr); @@ -1266,22 +1223,12 @@ TEST(HttpPortAllocatorTest, TestSessionRequestUrl) { rtc::FakeNetworkManager network_manager; cricket::HttpPortAllocator alloc(&network_manager, "unit test agent"); - // Disable PORTALLOCATOR_ENABLE_SHARED_UFRAG. - alloc.set_flags(alloc.flags() & ~cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); rtc::scoped_ptr session( static_cast( alloc.CreateSessionInternal( "test content", 0, kIceUfrag0, kIcePwd0))); std::string url = session->GetSessionRequestUrl(); LOG(LS_INFO) << "url: " << url; - EXPECT_EQ(std::string(cricket::HttpPortAllocator::kCreateSessionURL), url); - - // Enable PORTALLOCATOR_ENABLE_SHARED_UFRAG. - alloc.set_flags(alloc.flags() | cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); - session.reset(static_cast( - alloc.CreateSessionInternal("test content", 0, kIceUfrag0, kIcePwd0))); - url = session->GetSessionRequestUrl(); - LOG(LS_INFO) << "url: " << url; std::vector parts; rtc::split(url, '?', &parts); ASSERT_EQ(2U, parts.size()); diff --git a/webrtc/p2p/p2p.gyp b/webrtc/p2p/p2p.gyp index ac7a4641de..74546edec0 100644 --- a/webrtc/p2p/p2p.gyp +++ b/webrtc/p2p/p2p.gyp @@ -42,10 +42,6 @@ 'base/portinterface.h', 'base/pseudotcp.cc', 'base/pseudotcp.h', - 'base/rawtransport.cc', - 'base/rawtransport.h', - 'base/rawtransportchannel.cc', - 'base/rawtransportchannel.h', 'base/relayport.cc', 'base/relayport.h', 'base/relayserver.cc',