Bug 1438539: Added sanity check for connection attributes. r=bwc
authorJohannes Willbold <j.willbold@mozilla.com>
Mon, 25 Jun 2018 15:02:58 -0700
changeset 423995 4330430ac509e07af2f6444657ac0715e589d5c2
parent 423994 aa8226ea0c48b40e9232c14dc526ce3428edb799
child 423996 40eebd227af715e57e99c97ec0e46aca0ae90e4e
push id34197
push usercsabou@mozilla.com
push dateThu, 28 Jun 2018 09:44:02 +0000
treeherdermozilla-central@db455160668d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1438539
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -635,16 +635,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,
@@ -724,19 +736,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;
@@ -1018,16 +1038,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);