From 310b093aa41d00be744b6f4bce77fbb657a80096 Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Tue, 17 Nov 2015 19:15:50 -0800 Subject: [PATCH] Fix active tcp port to 9 In tcp only call: Tested with hangout. Tested with firefox. To test firefox, goto about:config, search for media.peerconnection.ice.tcp and turn it on. Existing test case should be suffice to cover this. R=juberti@google.com TBR=jubert@webrtc.org BUG=webrtc:3849 Review URL: https://codereview.webrtc.org/1217463004 . Cr-Commit-Position: refs/heads/master@{#10683} --- .../p2p/base/p2ptransportchannel_unittest.cc | 32 +++++++++++++++---- webrtc/p2p/base/tcpport.cc | 11 ++++--- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index b24879b654..1395038bed 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -650,6 +650,21 @@ class P2PTransportChannelTestBase : public testing::Test, GetEndpoint(endpoint)->save_candidates_ = true; } + // Tcp candidate verification has to be done when they are generated. + void VerifySavedTcpCandidates(int endpoint, const std::string& tcptype) { + for (auto& data : GetEndpoint(endpoint)->saved_candidates_) { + EXPECT_EQ(data->candidate.protocol(), cricket::TCP_PROTOCOL_NAME); + EXPECT_EQ(data->candidate.tcptype(), tcptype); + if (data->candidate.tcptype() == cricket::TCPTYPE_ACTIVE_STR) { + EXPECT_EQ(data->candidate.address().port(), cricket::DISCARD_PORT); + } else if (data->candidate.tcptype() == cricket::TCPTYPE_PASSIVE_STR) { + EXPECT_NE(data->candidate.address().port(), cricket::DISCARD_PORT); + } else { + FAIL() << "Unknown tcptype: " << data->candidate.tcptype(); + } + } + } + void ResumeCandidates(int endpoint) { Endpoint* ed = GetEndpoint(endpoint); std::vector::iterator it = ed->saved_candidates_.begin(); @@ -1290,8 +1305,19 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { SetAllowTcpListen(0, true); // actpass. SetAllowTcpListen(1, false); // active. + // Pause candidate so we could verify the candidate properties. + PauseCandidates(0); + PauseCandidates(1); CreateChannels(1); + // Verify tcp candidates. + VerifySavedTcpCandidates(0, cricket::TCPTYPE_PASSIVE_STR); + VerifySavedTcpCandidates(1, cricket::TCPTYPE_ACTIVE_STR); + + // Resume candidates. + ResumeCandidates(0); + ResumeCandidates(1); + EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && ep2_ch1()->receiving() && ep2_ch1()->writable(), 1000); @@ -1300,12 +1326,6 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); - std::string kTcpProtocol = "tcp"; - EXPECT_EQ(kTcpProtocol, RemoteCandidate(ep1_ch1())->protocol()); - EXPECT_EQ(kTcpProtocol, LocalCandidate(ep1_ch1())->protocol()); - EXPECT_EQ(kTcpProtocol, RemoteCandidate(ep2_ch1())->protocol()); - EXPECT_EQ(kTcpProtocol, LocalCandidate(ep2_ch1())->protocol()); - TestSendRecv(1); DestroyChannels(); } diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 2590d0aca8..721acc2007 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -184,10 +184,13 @@ void TCPPort::PrepareAddress() { } else { LOG_J(LS_INFO, this) << "Not listening due to firewall restrictions."; // Note: We still add the address, since otherwise the remote side won't - // recognize our incoming TCP connections. - AddAddress(rtc::SocketAddress(ip(), 0), rtc::SocketAddress(ip(), 0), - rtc::SocketAddress(), TCP_PROTOCOL_NAME, "", TCPTYPE_ACTIVE_STR, - LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); + // recognize our incoming TCP connections. According to + // https://tools.ietf.org/html/rfc6544#section-4.5, for active candidate, + // the port must be set to the discard port, i.e. 9. + AddAddress(rtc::SocketAddress(ip(), DISCARD_PORT), + rtc::SocketAddress(ip(), 0), rtc::SocketAddress(), + TCP_PROTOCOL_NAME, "", TCPTYPE_ACTIVE_STR, LOCAL_PORT_TYPE, + ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); } }