Parse the connection data in SDP (c= line).

webrtc/pc/webrtcsdp_unittest.cc

Index: webrtc/pc/webrtcsdp_unittest.cc
diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc
index f5cac8921f3a3fddadf0672f372ce9f2c4713d1d..2e5939c30243597270c98c5009f13db3d502d815 100644
--- a/webrtc/pc/webrtcsdp_unittest.cc
+++ b/webrtc/pc/webrtcsdp_unittest.cc
@@ -195,7 +195,7 @@ static const char kSdpString[] =
     "t=0 0\r\n"
     "a=msid-semantic: WMS local_stream_1\r\n"
     "m=audio 9 RTP/SAVPF 111 103 104\r\n"
-    "c=IN IP4 0.0.0.0\r\n"
+    "c=IN IP4 74.125.127.126\r\n"

[Taylor Brandstetter, 2017/03/17 00:20:43]

Since you changed the tests back to how they were initially, this is probably
not needed; same with the "set_connection_address" calls below.

[Zhi Huang, 2017/03/17 23:42:07]

Since the way of generating the c= line has been changed, we can't go back and
also fixing all the related tests seems to be the only option then.

This change is just to make the fix easier.

     "a=rtcp:9 IN IP4 0.0.0.0\r\n"
     "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n"
     "a=mid:audio_content_name\r\n"
@@ -213,7 +213,7 @@ static const char kSdpString[] =
     "a=ssrc:1 mslabel:local_stream_1\r\n"
     "a=ssrc:1 label:audio_track_id_1\r\n"
     "m=video 9 RTP/SAVPF 120\r\n"
-    "c=IN IP4 0.0.0.0\r\n"
+    "c=IN IP4 74.125.224.39\r\n"
     "a=rtcp:9 IN IP4 0.0.0.0\r\n"
     "a=ice-ufrag:ufrag_video\r\na=ice-pwd:pwd_video\r\n"
     "a=mid:video_content_name\r\n"
@@ -875,6 +875,9 @@ class WebRtcSdpTest : public testing::Test {
     audio_stream.sync_label = kStreamLabel1;
     audio_stream.ssrcs.push_back(kAudioTrack1Ssrc);
     audio_desc_->AddStream(audio_stream);
+    rtc::SocketAddress audio_addr_;
+    audio_addr_.SetIP("74.125.127.126");
+    audio_desc_->set_connection_address(audio_addr_);
     desc_.AddContent(kAudioContentName, NS_JINGLE_RTP, audio_desc_);
 
     // VideoContentDescription
@@ -888,6 +891,9 @@ class WebRtcSdpTest : public testing::Test {
     cricket::SsrcGroup ssrc_group(kFecSsrcGroupSemantics, video_stream.ssrcs);
     video_stream.ssrc_groups.push_back(ssrc_group);
     video_desc_->AddStream(video_stream);
+    rtc::SocketAddress video_addr_;
+    video_addr_.SetIP("74.125.224.39");
+    video_desc_->set_connection_address(video_addr_);
     desc_.AddContent(kVideoContentName, NS_JINGLE_RTP, video_desc_);
 
     // TransportInfo
@@ -1503,6 +1509,9 @@ class WebRtcSdpTest : public testing::Test {
                     cricket::kGoogleSctpDataCodecName);
     codec.SetParam(cricket::kCodecParamPort, kDefaultSctpPort);
     data_desc_->AddCodec(codec);
+    rtc::SocketAddress addr;
+    addr.SetIP("0.0.0.0");
+    data_desc_->set_connection_address(addr);
     desc_.AddContent(kDataContentName, NS_JINGLE_DRAFT_SCTP, data.release());
     EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
         kDataContentName, TransportDescription(kUfragData, kPwdData))));
@@ -1523,6 +1532,9 @@ class WebRtcSdpTest : public testing::Test {
         1, "AES_CM_128_HMAC_SHA1_80",
         "inline:FvLcvU2P3ZWmQxgPAgcDu7Zl9vftYElFOjEzhWs5", ""));
     data_desc_->set_protocol(cricket::kMediaProtocolSavpf);
+    rtc::SocketAddress addr;
+    addr.SetIP("0.0.0.0");
+    data_desc_->set_connection_address(addr);
     desc_.AddContent(kDataContentName, NS_JINGLE_RTP, data.release());
     EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
         kDataContentName, TransportDescription(kUfragData, kPwdData))));
@@ -1801,127 +1813,6 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionEmpty) {
   EXPECT_EQ("", webrtc::SdpSerialize(jdesc_empty, false));
 }
 
-// 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, false);
-
-  // 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, false);
-
-  // 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, false);
-
-  // 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, false);
-
-  // 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) {
@@ -3504,3 +3395,102 @@ TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithMissingStreamId) {
   JsepSessionDescription jdesc_output(kDummyString);
   EXPECT_FALSE(SdpDeserialize(kSdpWithMissingStreamId, &jdesc_output));
 }
+
+// Tests that if both session-level address and media-level address exist, use
+// the media-level address.
+TEST_F(WebRtcSdpTest, ParseConnectionData) {
+  JsepSessionDescription jsep_desc(kDummyString);
+
+  // Sesssion-level address.
+  std::string sdp = kSdpFullString;
+  InjectAfter("s=-\r\n", "c=IN IP4 192.168.0.3\r\n", &sdp);
+  EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
+
+  // Remove the media level addresses.
+  // The port is expected to be the default value 0.

[Taylor Brandstetter, 2017/03/17 00:20:43]

This comment about the port can probably be removed.

[Zhi Huang, 2017/03/17 23:42:07]

Done.

+  const auto& content1 = jsep_desc.description()->contents()[0];
+  EXPECT_EQ("74.125.127.126:2345",
+            static_cast<cricket::MediaContentDescription*>(content1.description)
+                ->connection_address()
+                .ToString());
+  const auto& content2 = jsep_desc.description()->contents()[1];
+  EXPECT_EQ("74.125.224.39:3457",
+            static_cast<cricket::MediaContentDescription*>(content2.description)
+                ->connection_address()
+                .ToString());
+}
+
+// Tests that the session-level connection address will be used if the media
+// level-addresses are not specified.
+TEST_F(WebRtcSdpTest, ParseConnectionDataSessionLevelOnly) {
+  JsepSessionDescription jsep_desc(kDummyString);
+
+  // Sesssion-level address.
+  std::string sdp = kSdpString;
+  InjectAfter("s=-\r\n", "c=IN IP4 192.168.0.3\r\n", &sdp);
+  Replace("c=IN IP4 74.125.127.126\r\n", "", &sdp);
+  Replace("c=IN IP4 74.125.224.39\r\n", "", &sdp);
+  EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
+
+  // Remove the media level addresses.
+  // The port is expected to be the default value 0.
+  const auto& content1 = jsep_desc.description()->contents()[0];
+  EXPECT_EQ("192.168.0.3:0",

[Taylor Brandstetter, 2017/03/17 00:20:43]

These should actually expect a port, since there should still be a port from the
m= line.

[Zhi Huang, 2017/03/17 23:42:06]

Done.

+            static_cast<cricket::MediaContentDescription*>(content1.description)
+                ->connection_address()
+                .ToString());
+  const auto& content2 = jsep_desc.description()->contents()[1];
+  EXPECT_EQ("192.168.0.3:0",
+            static_cast<cricket::MediaContentDescription*>(content2.description)
+                ->connection_address()
+                .ToString());
+}
+
+TEST_F(WebRtcSdpTest, ParseConnectionDataIPv6) {
+  JsepSessionDescription jsep_desc(kDummyString);
+
+  std::string sdp = kSdpString;
+  EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
+  Replace("c=IN IP4 74.125.127.126\r\n",
+          "c=IN IP6 2001:0db8:85a3:0000:0000:8a2e:0370:7335\r\n", &sdp);
+  Replace("c=IN IP4 74.125.224.39\r\n",
+          "c=IN IP6 2001:0db8:85a3:0000:0000:8a2e:0370:7336\r\n", &sdp);
+  EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
+  const auto& content3 = jsep_desc.description()->contents()[0];
+  EXPECT_EQ("[2001:db8:85a3::8a2e:370:7335]:2345",
+            static_cast<cricket::MediaContentDescription*>(content3.description)
+                ->connection_address()
+                .ToString());
+  const auto& content4 = jsep_desc.description()->contents()[1];
+  EXPECT_EQ("[2001:db8:85a3::8a2e:370:7336]:3457",
+            static_cast<cricket::MediaContentDescription*>(content4.description)
+                ->connection_address()
+                .ToString());
+}
+
+// Test that the invalid or unsupprted connection data cannot be parsed.
+TEST_F(WebRtcSdpTest, ParseConnectionDataFailure) {
+  JsepSessionDescription jsep_desc(kDummyString);
+  std::string sdp = kSdpString;
+  EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
+
+  // unsupported IPv4 address.

[Taylor Brandstetter, 2017/03/17 00:20:43]

nit: Capitalize first word of comments

[Zhi Huang, 2017/03/17 23:42:06]

Done.

+  sdp = kSdpFullString;
+  Replace("c=IN IP4 74.125.224.39\r\n", "c=IN IP4 74.125.224.39/127\r\n", &sdp);
+  EXPECT_FALSE(SdpDeserialize(sdp, &jsep_desc));
+
+  // unsupported IPv6 address.
+  sdp = kSdpFullString;
+  Replace("c=IN IP4 74.125.224.39\r\n", "c=IN IP6 a/3\r\n", &sdp);
+  EXPECT_FALSE(SdpDeserialize(sdp, &jsep_desc));
+
+  // mismatched address type.
+  sdp = kSdpFullString;
+  Replace("c=IN IP4 74.125.224.39\r\n", "c=IN IP6 74.125.224.39\r\n", &sdp);
+  EXPECT_FALSE(SdpDeserialize(sdp, &jsep_desc));
+
+  sdp = kSdpFullString;

[Taylor Brandstetter, 2017/03/17 00:20:43]

nit: Is this also "unsupported IPv6 address"? Might also be helpful to say
"Unsupported multicast IPv6 address", to be more specific.

[Zhi Huang, 2017/03/17 23:42:07]

Done.

+  Replace("c=IN IP4 74.125.224.39\r\n",
+          "c=IN IP4 2001:0db8:85a3:0000:0000:8a2e:0370:7334/3\r\n", &sdp);
+  EXPECT_FALSE(SdpDeserialize(sdp, &jsep_desc));
+}

[Taylor Brandstetter, 2017/03/17 00:20:43]

There should also be a test for the serialization. The test could just create a
SessionDescription, call "set_connection_address", then serialize and
deserialize and make sure the address is the same. That's typically how we test
it. Or it could search for the string, but that seems more fragile.

[Zhi Huang, 2017/03/17 23:42:07]

Done.