From f9945b2d1aa2d78b19987219ea872605167d7b5f Mon Sep 17 00:00:00 2001 From: Honghai Zhang Date: Tue, 15 Dec 2015 12:20:13 -0800 Subject: [PATCH] Only try to pair protocol matching candidates for creating connections. If the local port and the remote candidate's protocols do not match, do not even try to pair them. This avoids printing out confusing logs like "Attempt to change a remote candidate..." in p2ptransportchannel when two remote candidates have the same port number but different protocols. BUG= R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1516613002 . Cr-Commit-Position: refs/heads/master@{#11034} --- webrtc/p2p/base/p2ptransportchannel.cc | 3 +++ webrtc/p2p/base/port_unittest.cc | 25 +++++++++++++++++++++++++ webrtc/p2p/base/portinterface.h | 2 ++ webrtc/p2p/base/relayport.h | 4 ++++ webrtc/p2p/base/stunport.cc | 5 +++-- webrtc/p2p/base/stunport.h | 3 +++ webrtc/p2p/base/tcpport.cc | 4 +--- webrtc/p2p/base/tcpport.h | 3 +++ webrtc/p2p/base/turnport.cc | 2 +- webrtc/p2p/base/turnport.h | 4 ++++ 10 files changed, 49 insertions(+), 6 deletions(-) diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index da8b5f76c2..c95b04f0d2 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -738,6 +738,9 @@ bool P2PTransportChannel::CreateConnections(const Candidate& remote_candidate, bool P2PTransportChannel::CreateConnection(PortInterface* port, const Candidate& remote_candidate, PortInterface* origin_port) { + if (!port->SupportsProtocol(remote_candidate.protocol())) { + return false; + } // Look for an existing connection with this remote address. If one is not // found, then we can create a new connection for this address. Connection* connection = port->GetConnection(remote_candidate.address()); diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 896cef1159..4bff58adc7 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -141,6 +141,10 @@ class TestPort : public Port { ICE_TYPE_PREFERENCE_HOST, 0, true); } + virtual bool SupportsProtocol(const std::string& protocol) const { + return true; + } + // Exposed for testing candidate building. void AddCandidateAddress(const rtc::SocketAddress& addr) { AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", "", Type(), @@ -2495,3 +2499,24 @@ TEST_F(PortTest, TestControlledToControllingNotDestroyed) { rtc::Thread::Current()->ProcessMessages(kTimeout); EXPECT_FALSE(destroyed()); } + +TEST_F(PortTest, TestSupportsProtocol) { + rtc::scoped_ptr udp_port(CreateUdpPort(kLocalAddr1)); + EXPECT_TRUE(udp_port->SupportsProtocol(UDP_PROTOCOL_NAME)); + EXPECT_FALSE(udp_port->SupportsProtocol(TCP_PROTOCOL_NAME)); + + rtc::scoped_ptr stun_port( + CreateStunPort(kLocalAddr1, nat_socket_factory1())); + EXPECT_TRUE(stun_port->SupportsProtocol(UDP_PROTOCOL_NAME)); + EXPECT_FALSE(stun_port->SupportsProtocol(TCP_PROTOCOL_NAME)); + + rtc::scoped_ptr tcp_port(CreateTcpPort(kLocalAddr1)); + EXPECT_TRUE(tcp_port->SupportsProtocol(TCP_PROTOCOL_NAME)); + EXPECT_TRUE(tcp_port->SupportsProtocol(SSLTCP_PROTOCOL_NAME)); + EXPECT_FALSE(tcp_port->SupportsProtocol(UDP_PROTOCOL_NAME)); + + rtc::scoped_ptr turn_port( + CreateTurnPort(kLocalAddr1, nat_socket_factory1(), PROTO_UDP, PROTO_UDP)); + EXPECT_TRUE(turn_port->SupportsProtocol(UDP_PROTOCOL_NAME)); + EXPECT_FALSE(turn_port->SupportsProtocol(TCP_PROTOCOL_NAME)); +} diff --git a/webrtc/p2p/base/portinterface.h b/webrtc/p2p/base/portinterface.h index 0f77036ac1..d1c371d356 100644 --- a/webrtc/p2p/base/portinterface.h +++ b/webrtc/p2p/base/portinterface.h @@ -53,6 +53,8 @@ class PortInterface { virtual bool SharedSocket() const = 0; + virtual bool SupportsProtocol(const std::string& protocol) const = 0; + // PrepareAddress will attempt to get an address for this port that other // clients can send to. It may take some time before the address is ready. // Once it is ready, we will send SignalAddressReady. If errors are diff --git a/webrtc/p2p/base/relayport.h b/webrtc/p2p/base/relayport.h index 8452b5b430..4b74b91ade 100644 --- a/webrtc/p2p/base/relayport.h +++ b/webrtc/p2p/base/relayport.h @@ -60,6 +60,10 @@ class RelayPort : public Port { virtual int SetOption(rtc::Socket::Option opt, int value); virtual int GetOption(rtc::Socket::Option opt, int* value); virtual int GetError(); + virtual bool SupportsProtocol(const std::string& protocol) const { + // Relay port may create both TCP and UDP connections. + return true; + } const ProtocolAddress * ServerAddress(size_t index) const; bool IsReady() { return ready_; } diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc index f99965bd72..67cf789da8 100644 --- a/webrtc/p2p/base/stunport.cc +++ b/webrtc/p2p/base/stunport.cc @@ -253,9 +253,10 @@ void UDPPort::MaybePrepareStunCandidate() { } Connection* UDPPort::CreateConnection(const Candidate& address, - CandidateOrigin origin) { - if (address.protocol() != "udp") + CandidateOrigin origin) { + if (!SupportsProtocol(address.protocol())) { return NULL; + } if (!IsCompatibleAddress(address.address())) { return NULL; diff --git a/webrtc/p2p/base/stunport.h b/webrtc/p2p/base/stunport.h index ccd3f36cc3..ecf61a782d 100644 --- a/webrtc/p2p/base/stunport.h +++ b/webrtc/p2p/base/stunport.h @@ -95,6 +95,9 @@ class UDPPort : public Port { OnReadPacket(socket, data, size, remote_addr, packet_time); return true; } + virtual bool SupportsProtocol(const std::string& protocol) const { + return protocol == UDP_PROTOCOL_NAME; + } void set_stun_keepalive_delay(int delay) { stun_keepalive_delay_ = delay; diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 721acc2007..23afc2f031 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -125,9 +125,7 @@ TCPPort::~TCPPort() { Connection* TCPPort::CreateConnection(const Candidate& address, CandidateOrigin origin) { - // We only support TCP protocols - if ((address.protocol() != TCP_PROTOCOL_NAME) && - (address.protocol() != SSLTCP_PROTOCOL_NAME)) { + if (!SupportsProtocol(address.protocol())) { return NULL; } diff --git a/webrtc/p2p/base/tcpport.h b/webrtc/p2p/base/tcpport.h index a64c5eeab9..568dc65f3d 100644 --- a/webrtc/p2p/base/tcpport.h +++ b/webrtc/p2p/base/tcpport.h @@ -55,6 +55,9 @@ class TCPPort : public Port { virtual int GetOption(rtc::Socket::Option opt, int* value); virtual int SetOption(rtc::Socket::Option opt, int value); virtual int GetError(); + virtual bool SupportsProtocol(const std::string& protocol) const { + return protocol == TCP_PROTOCOL_NAME || protocol == SSLTCP_PROTOCOL_NAME; + } protected: TCPPort(rtc::Thread* thread, diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 55b3b9479c..328683b5ae 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -442,7 +442,7 @@ void TurnPort::OnAllocateMismatch() { Connection* TurnPort::CreateConnection(const Candidate& address, CandidateOrigin origin) { // TURN-UDP can only connect to UDP candidates. - if (address.protocol() != UDP_PROTOCOL_NAME) { + if (!SupportsProtocol(address.protocol())) { return NULL; } diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h index 7b88364e46..4be9249c8f 100644 --- a/webrtc/p2p/base/turnport.h +++ b/webrtc/p2p/base/turnport.h @@ -107,6 +107,10 @@ class TurnPort : public Port { const rtc::PacketTime& packet_time); virtual void OnReadyToSend(rtc::AsyncPacketSocket* socket); + virtual bool SupportsProtocol(const std::string& protocol) const { + // Turn port only connects to UDP candidates. + return protocol == UDP_PROTOCOL_NAME; + } void OnSocketConnect(rtc::AsyncPacketSocket* socket); void OnSocketClose(rtc::AsyncPacketSocket* socket, int error);