Bug 1438539: Added sanity check for connection attributes. r=bwc draft
authorJohannes Willbold <j.willbold@mozilla.com>
Mon, 25 Jun 2018 15:02:58 -0700
changeset 810452 ea354701995bf62c4231e654d12ad038a6e29bb0
parent 810379 6e8e861540e6d8c85c73ab7b2afa1f027fb3750c
push id114001
push userbmo:johannes.willbold@rub.de
push dateMon, 25 Jun 2018 22:24:38 +0000
reviewersbwc
bugs1438539
milestone63.0a1
Bug 1438539: Added sanity check for connection attributes. r=bwc Added sanity check that ensure that there is either a connection on the session level or in each media secion. Extended create_dummy_sdp_session Alterted Rust unit tests: parse_minimal_sdp_with_emtpy_lines parse_minimal_sdp MozReview-Commit-ID: yVEhUvns57
media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
media/webrtc/signaling/src/sdp/rsdparsa/src/lib.rs
media/webrtc/signaling/src/sdp/rsdparsa/tests/unit_tests.rs
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
@@ -255,18 +255,16 @@ RsdparsaSdpMediaSection::LoadConnection(
   RustSdpConnection conn;
   nsresult nr;
   if (sdp_media_has_connection(mSection)) {
     nr = sdp_get_media_connection(mSection, &conn);
     if (NS_SUCCEEDED(nr)) {
       mConnection = convertRustConnection(conn);
     }
   } else if (sdp_session_has_connection(mSession.get())){
-    // TODO: rsdparsa needs to ensure there is a connection at the session level
-    // if it is missing at a media level. See Bug 1438539.
     nr = sdp_get_session_connection(mSession.get(), &conn);
     if (NS_SUCCEEDED(nr)) {
       mConnection = convertRustConnection(conn);
     }
   }
 }
 
 } // namespace mozilla
--- a/media/webrtc/signaling/src/sdp/rsdparsa/src/lib.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa/src/lib.rs
@@ -628,16 +628,28 @@ fn sanity_check_sdp_session(session: &Sd
 
     if !session.has_media() {
         return Err(SdpParserError::Sequence {
                        message: "Missing media setion".to_string(),
                        line_number: 0,
                    });
     }
 
+    if session.get_connection().is_none() {
+        for msection in &session.media {
+            if !msection.has_connection() {
+                return Err(SdpParserError::Sequence {
+                    message: "Each media section must define a connection
+                              if it is not defined on session level".to_string(),
+                    line_number: 0,
+                });
+            }
+        }
+    }
+
     // Check that extmaps are not defined on session and media level
     if session.get_attribute(SdpAttributeType::Extmap).is_some() {
         for msection in &session.media {
             if msection.get_attribute(SdpAttributeType::Extmap).is_some() {
                 return Err(SdpParserError::Sequence {
                                message: "Extmap can't be define at session and media level"
                                    .to_string(),
                                line_number: 0,
@@ -671,19 +683,27 @@ fn sanity_check_sdp_session(session: &Sd
 
     Ok(())
 }
 
 #[cfg(test)]
 fn create_dummy_sdp_session() -> SdpSession {
     let origin = parse_origin("mozilla 506705521068071134 0 IN IP4 0.0.0.0");
     assert!(origin.is_ok());
-    let sdp_session;
+    let connection = parse_connection("IN IP4 198.51.100.7");
+    assert!(connection.is_ok());
+    let mut sdp_session;
     if let SdpType::Origin(o) = origin.unwrap() {
         sdp_session = SdpSession::new(0, o, "-".to_string());
+
+        if let Ok(SdpType::Connection(c)) = connection {
+            sdp_session.connection = Some(c);
+        } else {
+            panic!("Sdp type is not Connection")
+        }
     } else {
         panic!("SdpType is not Origin");
     }
     sdp_session
 }
 
 #[cfg(test)]
 use media_type::create_dummy_media_section;
@@ -965,16 +985,17 @@ m=foobar 0 UDP/TLS/RTP/SAVPF 0\r\n",
                     .is_err());
 }
 
 #[test]
 fn test_parse_sdp_unsupported_warning() {
     assert!(parse_sdp("v=0\r\n
 o=- 0 0 IN IP4 0.0.0.0\r\n
 s=-\r\n
+c=IN IP4 198.51.100.7\r\n
 t=0 0\r\n
 m=audio 0 UDP/TLS/RTP/SAVPF 0\r\n
 a=unsupported\r\n",
                       false)
                     .is_ok());
 }
 
 #[test]
--- a/media/webrtc/signaling/src/sdp/rsdparsa/tests/unit_tests.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa/tests/unit_tests.rs
@@ -1,25 +1,26 @@
 extern crate rsdparsa;
 
 #[test]
 fn parse_minimal_sdp() {
     let sdp = "v=0\r\n
 o=- 0 0 IN IP4 0.0.0.0\r\n
 s=-\r\n
+c=IN IP4 0.0.0.0\r\n
 t=0 0\r\n
 m=audio 0 UDP/TLS/RTP/SAVPF 0\r\n";
     let sdp_res = rsdparsa::parse_sdp(sdp, true);
     assert!(sdp_res.is_ok());
     let sdp_opt = sdp_res.ok();
     assert!(sdp_opt.is_some());
     let sdp = sdp_opt.unwrap();
     assert_eq!(sdp.version, 0);
     assert_eq!(sdp.session, "-");
-    assert!(sdp.connection.is_none());
+    assert!(sdp.connection.is_some());
     assert_eq!(sdp.attribute.len(), 0);
     assert_eq!(sdp.media.len(), 1);
 
     let msection = &(sdp.media[0]);
     assert_eq!(*msection.get_type(),
                rsdparsa::media_type::SdpMediaValue::Audio);
     assert_eq!(msection.get_port(), 0);
     assert_eq!(*msection.get_proto(),
@@ -32,16 +33,17 @@ m=audio 0 UDP/TLS/RTP/SAVPF 0\r\n";
 
 #[test]
 fn parse_minimal_sdp_with_emtpy_lines() {
     let sdp = "v=0\r\n
 \r\n
 o=- 0 0 IN IP4 0.0.0.0\r\n
  \r\n
 s=-\r\n
+c=IN IP4 0.0.0.0\r\n
 t=0 0\r\n
 m=audio 0 UDP/TLS/RTP/SAVPF 0\r\n";
     let sdp_res = rsdparsa::parse_sdp(sdp, false);
     assert!(sdp_res.is_ok());
     let sdp_opt = sdp_res.ok();
     assert!(sdp_opt.is_some());
     let sdp = sdp_opt.unwrap();
     assert_eq!(sdp.version, 0);