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

webrtc/pc/webrtcsdp_unittest.cc

 /*
  *  Copyright 2011 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.
  */
 
@@ skipping 177 matching lines @@
     "a=ssrc:3 label:video_track_id_1\r\n";
 
 // SDP reference string without the candidates.
 static const char kSdpString[] =
     "v=0\r\n"
     "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
     "s=-\r\n"
     "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"
     "a=sendrecv\r\n"
     "a=rtcp-mux\r\n"
     "a=rtcp-rsize\r\n"
     "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
     "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
     "dummy_session_params\r\n"
     "a=rtpmap:111 opus/48000/2\r\n"
     "a=rtpmap:103 ISAC/16000\r\n"
     "a=rtpmap:104 ISAC/32000\r\n"
     "a=ssrc:1 cname:stream_1_cname\r\n"
     "a=ssrc:1 msid:local_stream_1 audio_track_id_1\r\n"
     "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"
     "a=sendrecv\r\n"
     "a=crypto:1 AES_CM_128_HMAC_SHA1_80 "
     "inline:d0RmdmcmVCspeEc3QGZiNWpVLFJhQX1cfHAwJSoj|2^20|1:32\r\n"
     "a=rtpmap:120 VP8/90000\r\n"
     "a=ssrc-group:FEC 2 3\r\n"
     "a=ssrc:2 cname:stream_1_cname\r\n"
     "a=ssrc:2 msid:local_stream_1 video_track_id_1\r\n"
@@ skipping 641 matching lines @@
     webrtc::InitializeAndroidObjects();
 #endif
     // AudioContentDescription
     audio_desc_ = CreateAudioContentDescription();
     StreamParams audio_stream;
     audio_stream.id = kAudioTrackId1;
     audio_stream.cname = kStream1Cname;
     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
     video_desc_ = CreateVideoContentDescription();
     StreamParams video_stream;
     video_stream.id = kVideoTrackId1;
     video_stream.cname = kStream1Cname;
     video_stream.sync_label = kStreamLabel1;
     video_stream.ssrcs.push_back(kVideoTrack1Ssrc1);
     video_stream.ssrcs.push_back(kVideoTrack1Ssrc2);
     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
     EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
         kAudioContentName, TransportDescription(kUfragVoice, kPwdVoice))));
     EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
         kVideoContentName, TransportDescription(kUfragVideo, kPwdVideo))));
 
     // v4 host
     int port = 1234;
@@ skipping 595 matching lines @@
 
   void AddSctpDataChannel(bool use_sctpmap) {
     std::unique_ptr<DataContentDescription> data(new DataContentDescription());
     data_desc_ = data.get();
     data_desc_->set_use_sctpmap(use_sctpmap);
     data_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
     DataCodec codec(cricket::kGoogleSctpDataCodecPlType,
                     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))));
   }
 
   void AddRtpDataChannel() {
     std::unique_ptr<DataContentDescription> data(new DataContentDescription());
     data_desc_ = data.get();
 
     data_desc_->AddCodec(DataCodec(101, "google-data"));
     StreamParams data_stream;
     data_stream.id = kDataChannelMsid;
     data_stream.cname = kDataChannelCname;
     data_stream.sync_label = kDataChannelLabel;
     data_stream.ssrcs.push_back(kDataChannelSsrc);
     data_desc_->AddStream(data_stream);
     data_desc_->AddCrypto(CryptoParams(
         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))));
   }
 
   bool TestDeserializeDirection(cricket::MediaContentDirection direction) {
     std::string new_sdp = kSdpFullString;
     ReplaceDirection(direction, &new_sdp);
     JsepSessionDescription new_jdesc(kDummyString);
 
@@ skipping 258 matching lines @@
   // SessionDescription with desc and candidates.
   std::string message = webrtc::SdpSerialize(jdesc_, false);
   TestMismatch(std::string(kSdpFullString), message);
 }
 
 TEST_F(WebRtcSdpTest, SerializeSessionDescriptionEmpty) {
   JsepSessionDescription jdesc_empty(kDummyString);
   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) {
   AddFingerprint();
   JsepSessionDescription jdesc_with_fingerprint(kDummyString);
   ASSERT_TRUE(jdesc_with_fingerprint.Initialize(desc_.Copy(),
                                                 kSessionId, kSessionVersion));
   std::string message = webrtc::SdpSerialize(jdesc_with_fingerprint, false);
 
   std::string sdp_with_fingerprint = kSdpString;
@@ skipping 1562 matching lines @@
       "s=-\r\n"
       "t=0 0\r\n"
       "m=audio 9 RTP/SAVPF 111\r\n"
       "c=IN IP4 0.0.0.0\r\n"
       "a=rtpmap:111 opus/48000/2\r\n"
       "a=msid: track_id\r\n";
 
   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.