Default destination used by c line should be IPv4 only to avoid parsing error in legacy client.
Make sure the IP family overwrites the preference of candidates. Also,
make sure only UDP is used as default destination.
BUG=4269
R=juberti@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/36009004
Cr-Commit-Position: refs/heads/master@{#8258}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8258 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc
index f99a9da..9a6ca47 100644
--- a/talk/app/webrtc/webrtcsdp.cc
+++ b/talk/app/webrtc/webrtcsdp.cc
@@ -668,8 +668,9 @@
return preference;
}
-// Get ip and port of the default destination from the |candidates| with
-// the given value of |component_id|.
+// Get ip and port of the default destination from the |candidates| with the
+// given value of |component_id|. The default candidate should be the one most
+// likely to work, typically IPv4 relay.
// RFC 5245
// The value of |component_id| currently supported are 1 (RTP) and 2 (RTCP).
// TODO: Decide the default destination in webrtcsession and
@@ -682,25 +683,35 @@
*port = kDummyPort;
*ip = kDummyAddress;
int current_preference = kPreferenceUnknown;
+ int current_family = AF_UNSPEC;
for (std::vector<Candidate>::const_iterator it = candidates.begin();
it != candidates.end(); ++it) {
if (it->component() != component_id) {
continue;
}
- const int preference = GetCandidatePreferenceFromType(it->type());
- // See if this candidate is more preferable then the current one.
- if (preference <= current_preference) {
+ // Default destination should be UDP only.
+ if (it->protocol() != cricket::UDP_PROTOCOL_NAME) {
continue;
}
- current_preference = preference;
- *port = it->address().PortAsString();
- *ip = it->address().ipaddr().ToString();
- int family = it->address().ipaddr().family();
+ const int preference = GetCandidatePreferenceFromType(it->type());
+ const int family = it->address().ipaddr().family();
+ // See if this candidate is more preferable then the current one if it's the
+ // same family. Or if the current family is IPv4 already so we could safely
+ // ignore all IPv6 ones. WebRTC bug 4269.
+ // http://code.google.com/p/webrtc/issues/detail?id=4269
+ if ((preference <= current_preference && current_family == family) ||
+ (current_family == AF_INET && family == AF_INET6)) {
+ continue;
+ }
if (family == AF_INET) {
addr_type->assign(kConnectionIpv4Addrtype);
} else if (family == AF_INET6) {
addr_type->assign(kConnectionIpv6Addrtype);
}
+ current_preference = preference;
+ current_family = family;
+ *port = it->address().PortAsString();
+ *ip = it->address().ipaddr().ToString();
}
}
diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc
index 4a2aab1..e622944 100644
--- a/talk/app/webrtc/webrtcsdp_unittest.cc
+++ b/talk/app/webrtc/webrtcsdp_unittest.cc
@@ -1363,6 +1363,127 @@
EXPECT_EQ("", webrtc::SdpSerialize(jdesc_empty));
}
+// This tests serialization of SDP with only IPv6 candidates and verifies that
+// IPv6 is used as default address in c line according to preference.
+TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithIPv6Only) {
+ // Only test 1 m line.
+ desc_.RemoveContentByName("video_content_name");
+ // Stun has a high preference than local host.
+ cricket::Candidate candidate1(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+ rtc::SocketAddress("::1", 1234), kCandidatePriority, "", "",
+ cricket::STUN_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ cricket::Candidate candidate2(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+ rtc::SocketAddress("::2", 1235), kCandidatePriority, "", "",
+ cricket::LOCAL_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ JsepSessionDescription jdesc(kDummyString);
+ ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+ // Only add the candidates to audio m line.
+ JsepIceCandidate jice1("audio_content_name", 0, candidate1);
+ JsepIceCandidate jice2("audio_content_name", 0, candidate2);
+ ASSERT_TRUE(jdesc.AddCandidate(&jice1));
+ ASSERT_TRUE(jdesc.AddCandidate(&jice2));
+ std::string message = webrtc::SdpSerialize(jdesc);
+
+ // Audio line should have a c line like this one.
+ EXPECT_NE(message.find("c=IN IP6 ::1"), std::string::npos);
+ // Shouldn't have a IP4 c line.
+ EXPECT_EQ(message.find("c=IN IP4"), std::string::npos);
+}
+
+// This tests serialization of SDP with both IPv4 and IPv6 candidates and
+// verifies that IPv4 is used as default address in c line even if the
+// preference of IPv4 is lower.
+TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBothIPFamilies) {
+ // Only test 1 m line.
+ desc_.RemoveContentByName("video_content_name");
+ cricket::Candidate candidate_v4(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+ rtc::SocketAddress("192.168.1.5", 1234), kCandidatePriority, "", "",
+ cricket::STUN_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ cricket::Candidate candidate_v6(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+ rtc::SocketAddress("::1", 1234), kCandidatePriority, "", "",
+ cricket::LOCAL_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ JsepSessionDescription jdesc(kDummyString);
+ ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+ // Only add the candidates to audio m line.
+ JsepIceCandidate jice_v4("audio_content_name", 0, candidate_v4);
+ JsepIceCandidate jice_v6("audio_content_name", 0, candidate_v6);
+ ASSERT_TRUE(jdesc.AddCandidate(&jice_v4));
+ ASSERT_TRUE(jdesc.AddCandidate(&jice_v6));
+ std::string message = webrtc::SdpSerialize(jdesc);
+
+ // Audio line should have a c line like this one.
+ EXPECT_NE(message.find("c=IN IP4 192.168.1.5"), std::string::npos);
+ // Shouldn't have a IP6 c line.
+ EXPECT_EQ(message.find("c=IN IP6"), std::string::npos);
+}
+
+// This tests serialization of SDP with both UDP and TCP candidates and
+// verifies that UDP is used as default address in c line even if the
+// preference of UDP is lower.
+TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBothProtocols) {
+ // Only test 1 m line.
+ desc_.RemoveContentByName("video_content_name");
+ // Stun has a high preference than local host.
+ cricket::Candidate candidate1(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "tcp",
+ rtc::SocketAddress("::1", 1234), kCandidatePriority, "", "",
+ cricket::STUN_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ cricket::Candidate candidate2(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+ rtc::SocketAddress("fe80::1234:5678:abcd:ef12", 1235), kCandidatePriority,
+ "", "", cricket::LOCAL_PORT_TYPE, kCandidateGeneration,
+ kCandidateFoundation1);
+ JsepSessionDescription jdesc(kDummyString);
+ ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+ // Only add the candidates to audio m line.
+ JsepIceCandidate jice1("audio_content_name", 0, candidate1);
+ JsepIceCandidate jice2("audio_content_name", 0, candidate2);
+ ASSERT_TRUE(jdesc.AddCandidate(&jice1));
+ ASSERT_TRUE(jdesc.AddCandidate(&jice2));
+ std::string message = webrtc::SdpSerialize(jdesc);
+
+ // Audio line should have a c line like this one.
+ EXPECT_NE(message.find("c=IN IP6 fe80::1234:5678:abcd:ef12"),
+ std::string::npos);
+ // Shouldn't have a IP4 c line.
+ EXPECT_EQ(message.find("c=IN IP4"), std::string::npos);
+}
+
+// This tests serialization of SDP with only TCP candidates and verifies that
+// null IPv4 is used as default address in c line.
+TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithTCPOnly) {
+ // Only test 1 m line.
+ desc_.RemoveContentByName("video_content_name");
+ // Stun has a high preference than local host.
+ cricket::Candidate candidate1(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "tcp",
+ rtc::SocketAddress("::1", 1234), kCandidatePriority, "", "",
+ cricket::STUN_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ cricket::Candidate candidate2(
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "tcp",
+ rtc::SocketAddress("::2", 1235), kCandidatePriority, "", "",
+ cricket::LOCAL_PORT_TYPE, kCandidateGeneration, kCandidateFoundation1);
+ JsepSessionDescription jdesc(kDummyString);
+ ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+ // Only add the candidates to audio m line.
+ JsepIceCandidate jice1("audio_content_name", 0, candidate1);
+ JsepIceCandidate jice2("audio_content_name", 0, candidate2);
+ ASSERT_TRUE(jdesc.AddCandidate(&jice1));
+ ASSERT_TRUE(jdesc.AddCandidate(&jice2));
+ std::string message = webrtc::SdpSerialize(jdesc);
+
+ // Audio line should have a c line like this one when no any default exists.
+ EXPECT_NE(message.find("c=IN IP4 0.0.0.0"), std::string::npos);
+}
+
// This tests serialization of SDP with a=crypto and a=fingerprint, as would be
// the case in a DTLS offer.
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithFingerprint) {
