From 2c4c2c47848220acf8ed749568372e617271ad2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Wed, 20 Nov 2019 23:57:46 +0200 Subject: [PATCH] sdp: Guard against NULL strings/arrays in the SDP data structures While all these are invalid for a proper SDP, they appear as NULL when creating a new, empty SDP. Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/issues/221 --- gstreamer-sdp/src/sdp_bandwidth.rs | 17 ++++++-- gstreamer-sdp/src/sdp_connection.rs | 50 +++++++++++++++++------- gstreamer-sdp/src/sdp_key.rs | 20 ++++++++-- gstreamer-sdp/src/sdp_media.rs | 17 ++++++++ gstreamer-sdp/src/sdp_message.rs | 8 ++++ gstreamer-sdp/src/sdp_origin.rs | 60 +++++++++++++++++++++++------ gstreamer-sdp/src/sdp_time.rs | 42 +++++++++++++++++--- gstreamer-sdp/src/sdp_zone.rs | 27 ++++++++++--- 8 files changed, 199 insertions(+), 42 deletions(-) diff --git a/gstreamer-sdp/src/sdp_bandwidth.rs b/gstreamer-sdp/src/sdp_bandwidth.rs index 5f94b0f66..67149fd1e 100644 --- a/gstreamer-sdp/src/sdp_bandwidth.rs +++ b/gstreamer-sdp/src/sdp_bandwidth.rs @@ -26,8 +26,14 @@ impl SDPBandwidth { } } - pub fn bwtype(&self) -> &str { - unsafe { CStr::from_ptr(self.0.bwtype).to_str().unwrap() } + pub fn bwtype(&self) -> Option<&str> { + unsafe { + if self.0.bwtype.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.bwtype).to_str().unwrap()) + } + } } pub fn value(&self) -> u32 { @@ -37,7 +43,12 @@ impl SDPBandwidth { impl Clone for SDPBandwidth { fn clone(&self) -> Self { - SDPBandwidth::new(self.bwtype(), self.value()) + assert_initialized_main_thread!(); + unsafe { + let mut bw = mem::MaybeUninit::zeroed(); + gst_sdp_sys::gst_sdp_bandwidth_set(bw.as_mut_ptr(), self.0.bwtype, self.0.bandwidth); + SDPBandwidth(bw.assume_init()) + } } } diff --git a/gstreamer-sdp/src/sdp_connection.rs b/gstreamer-sdp/src/sdp_connection.rs index 6783c158e..45f2385d1 100644 --- a/gstreamer-sdp/src/sdp_connection.rs +++ b/gstreamer-sdp/src/sdp_connection.rs @@ -33,16 +33,34 @@ impl SDPConnection { } } - pub fn nettype(&self) -> &str { - unsafe { CStr::from_ptr(self.0.nettype).to_str().unwrap() } + pub fn nettype(&self) -> Option<&str> { + unsafe { + if self.0.nettype.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.nettype).to_str().unwrap()) + } + } } - pub fn addrtype(&self) -> &str { - unsafe { CStr::from_ptr(self.0.addrtype).to_str().unwrap() } + pub fn addrtype(&self) -> Option<&str> { + unsafe { + if self.0.addrtype.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.addrtype).to_str().unwrap()) + } + } } - pub fn address(&self) -> &str { - unsafe { CStr::from_ptr(self.0.address).to_str().unwrap() } + pub fn address(&self) -> Option<&str> { + unsafe { + if self.0.address.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.address).to_str().unwrap()) + } + } } pub fn ttl(&self) -> u32 { @@ -56,13 +74,19 @@ impl SDPConnection { impl Clone for SDPConnection { fn clone(&self) -> Self { - SDPConnection::new( - self.nettype(), - self.addrtype(), - self.address(), - self.ttl(), - self.addr_number(), - ) + assert_initialized_main_thread!(); + unsafe { + let mut conn = mem::MaybeUninit::zeroed(); + gst_sdp_sys::gst_sdp_connection_set( + conn.as_mut_ptr(), + self.0.nettype, + self.0.addrtype, + self.0.address, + self.0.ttl, + self.0.addr_number, + ); + SDPConnection(conn.assume_init()) + } } } diff --git a/gstreamer-sdp/src/sdp_key.rs b/gstreamer-sdp/src/sdp_key.rs index 288e59393..f7c9867e9 100644 --- a/gstreamer-sdp/src/sdp_key.rs +++ b/gstreamer-sdp/src/sdp_key.rs @@ -15,12 +15,24 @@ use gst_sdp_sys; pub struct SDPKey(gst_sdp_sys::GstSDPKey); impl SDPKey { - pub fn type_(&self) -> &str { - unsafe { CStr::from_ptr(self.0.type_).to_str().unwrap() } + pub fn type_(&self) -> Option<&str> { + unsafe { + if self.0.type_.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.type_).to_str().unwrap()) + } + } } - pub fn data(&self) -> &str { - unsafe { CStr::from_ptr(self.0.data).to_str().unwrap() } + pub fn data(&self) -> Option<&str> { + unsafe { + if self.0.data.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.data).to_str().unwrap()) + } + } } } diff --git a/gstreamer-sdp/src/sdp_media.rs b/gstreamer-sdp/src/sdp_media.rs index 0732ded4c..1c21617ce 100644 --- a/gstreamer-sdp/src/sdp_media.rs +++ b/gstreamer-sdp/src/sdp_media.rs @@ -708,3 +708,20 @@ define_iter!( |media: &'a SDPMediaRef, idx| media.get_attribute(idx), |media: &SDPMediaRef| media.attributes_len() ); + +#[cfg(test)] +mod tests { + use super::*; + + fn init() { + gst::init().unwrap(); + } + + #[test] + fn debug_impl() { + init(); + + let sdp = SDPMedia::new(); + assert!(!format!("{:?}", sdp).is_empty()); + } +} diff --git a/gstreamer-sdp/src/sdp_message.rs b/gstreamer-sdp/src/sdp_message.rs index 935d36cec..8da9b84f6 100644 --- a/gstreamer-sdp/src/sdp_message.rs +++ b/gstreamer-sdp/src/sdp_message.rs @@ -1124,4 +1124,12 @@ mod tests { let media = sdp.get_media(0).unwrap(); assert_eq!(media.formats_len(), 1); } + + #[test] + fn debug_impl() { + init(); + + let sdp = SDPMessage::new(); + assert!(!format!("{:?}", sdp).is_empty()); + } } diff --git a/gstreamer-sdp/src/sdp_origin.rs b/gstreamer-sdp/src/sdp_origin.rs index 011bc5394..beb5a8973 100644 --- a/gstreamer-sdp/src/sdp_origin.rs +++ b/gstreamer-sdp/src/sdp_origin.rs @@ -15,28 +15,64 @@ use gst_sdp_sys; pub struct SDPOrigin(pub(crate) gst_sdp_sys::GstSDPOrigin); impl SDPOrigin { - pub fn username(&self) -> &str { - unsafe { CStr::from_ptr(self.0.username).to_str().unwrap() } + pub fn username(&self) -> Option<&str> { + unsafe { + if self.0.username.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.username).to_str().unwrap()) + } + } } - pub fn sess_id(&self) -> &str { - unsafe { CStr::from_ptr(self.0.sess_id).to_str().unwrap() } + pub fn sess_id(&self) -> Option<&str> { + unsafe { + if self.0.sess_id.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.sess_id).to_str().unwrap()) + } + } } - pub fn sess_version(&self) -> &str { - unsafe { CStr::from_ptr(self.0.sess_version).to_str().unwrap() } + pub fn sess_version(&self) -> Option<&str> { + unsafe { + if self.0.sess_version.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.sess_version).to_str().unwrap()) + } + } } - pub fn nettype(&self) -> &str { - unsafe { CStr::from_ptr(self.0.nettype).to_str().unwrap() } + pub fn nettype(&self) -> Option<&str> { + unsafe { + if self.0.nettype.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.nettype).to_str().unwrap()) + } + } } - pub fn addrtype(&self) -> &str { - unsafe { CStr::from_ptr(self.0.addrtype).to_str().unwrap() } + pub fn addrtype(&self) -> Option<&str> { + unsafe { + if self.0.addrtype.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.addrtype).to_str().unwrap()) + } + } } - pub fn addr(&self) -> &str { - unsafe { CStr::from_ptr(self.0.addr).to_str().unwrap() } + pub fn addr(&self) -> Option<&str> { + unsafe { + if self.0.addr.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.addr).to_str().unwrap()) + } + } } } diff --git a/gstreamer-sdp/src/sdp_time.rs b/gstreamer-sdp/src/sdp_time.rs index 0dbb1b477..f638064c2 100644 --- a/gstreamer-sdp/src/sdp_time.rs +++ b/gstreamer-sdp/src/sdp_time.rs @@ -10,6 +10,7 @@ use std::ffi::CStr; use std::fmt; use std::mem; use std::os::raw::c_char; +use std::ptr; use glib::translate::*; use gst_sdp_sys; @@ -32,17 +33,33 @@ impl SDPTime { } } - pub fn start(&self) -> &str { - unsafe { CStr::from_ptr(self.0.start).to_str().unwrap() } + pub fn start(&self) -> Option<&str> { + unsafe { + if self.0.start.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.start).to_str().unwrap()) + } + } } - pub fn stop(&self) -> &str { - unsafe { CStr::from_ptr(self.0.stop).to_str().unwrap() } + pub fn stop(&self) -> Option<&str> { + unsafe { + if self.0.stop.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.stop).to_str().unwrap()) + } + } } pub fn repeat(&self) -> Vec<&str> { #[allow(clippy::cast_ptr_alignment)] unsafe { + if self.0.repeat.is_null() || (*self.0.repeat).data.is_null() { + return vec![]; + } + let arr = (*self.0.repeat).data as *const *const c_char; let len = (*self.0.repeat).len as usize; let mut vec = Vec::with_capacity(len); @@ -56,7 +73,22 @@ impl SDPTime { impl Clone for SDPTime { fn clone(&self) -> Self { - SDPTime::new(self.start(), self.stop(), self.repeat().as_slice()) + assert_initialized_main_thread!(); + #[allow(clippy::cast_ptr_alignment)] + unsafe { + let mut time = mem::MaybeUninit::zeroed(); + gst_sdp_sys::gst_sdp_time_set( + time.as_mut_ptr(), + self.0.start, + self.0.stop, + if self.0.repeat.is_null() { + ptr::null_mut() + } else { + (*self.0.repeat).data as *mut *const c_char + }, + ); + SDPTime(time.assume_init()) + } } } diff --git a/gstreamer-sdp/src/sdp_zone.rs b/gstreamer-sdp/src/sdp_zone.rs index ee623d5c9..fc6a673d1 100644 --- a/gstreamer-sdp/src/sdp_zone.rs +++ b/gstreamer-sdp/src/sdp_zone.rs @@ -30,18 +30,35 @@ impl SDPZone { } } - pub fn time(&self) -> &str { - unsafe { CStr::from_ptr(self.0.time).to_str().unwrap() } + pub fn time(&self) -> Option<&str> { + unsafe { + if self.0.time.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.time).to_str().unwrap()) + } + } } - pub fn typed_time(&self) -> &str { - unsafe { CStr::from_ptr(self.0.typed_time).to_str().unwrap() } + pub fn typed_time(&self) -> Option<&str> { + unsafe { + if self.0.typed_time.is_null() { + None + } else { + Some(CStr::from_ptr(self.0.typed_time).to_str().unwrap()) + } + } } } impl Clone for SDPZone { fn clone(&self) -> Self { - SDPZone::new(self.time(), self.typed_time()) + assert_initialized_main_thread!(); + unsafe { + let mut zone = mem::MaybeUninit::zeroed(); + gst_sdp_sys::gst_sdp_zone_set(zone.as_mut_ptr(), self.0.time, self.0.typed_time); + SDPZone(zone.assume_init()) + } } }