diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index d6cbd09b1..a7ccca1be 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -433,7 +433,7 @@ pub unsafe extern "C" fn dc_set_config_from_qr( pub unsafe extern "C" fn dc_get_info(context: *mut dc_context_t) -> *mut libc::c_char { if context.is_null() { eprintln!("ignoring careless call to dc_get_info()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_context = &*context; let guard = ffi_context.inner.read().unwrap(); @@ -1455,7 +1455,7 @@ pub unsafe extern "C" fn dc_get_msg_info( ) -> *mut libc::c_char { if context.is_null() { eprintln!("ignoring careless call to dc_get_msg_info()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_context = &*context; ffi_context @@ -2406,7 +2406,7 @@ pub unsafe extern "C" fn dc_chat_get_type(chat: *mut dc_chat_t) -> libc::c_int { pub unsafe extern "C" fn dc_chat_get_name(chat: *mut dc_chat_t) -> *mut libc::c_char { if chat.is_null() { eprintln!("ignoring careless call to dc_chat_get_name()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_chat = &*chat; ffi_chat.chat.get_name().strdup() @@ -2728,7 +2728,7 @@ pub unsafe extern "C" fn dc_msg_get_sort_timestamp(msg: *mut dc_msg_t) -> i64 { pub unsafe extern "C" fn dc_msg_get_text(msg: *mut dc_msg_t) -> *mut libc::c_char { if msg.is_null() { eprintln!("ignoring careless call to dc_msg_get_text()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_msg = &*msg; ffi_msg.message.get_text().unwrap_or_default().strdup() @@ -2747,8 +2747,7 @@ pub unsafe extern "C" fn dc_msg_get_file(msg: *mut dc_msg_t) -> *mut libc::c_cha ffi_msg .message .get_file(ctx) - .and_then(|p| p.to_c_string().ok()) - .map(|cs| dc_strdup(cs.as_ptr())) + .map(|p| p.strdup()) .unwrap_or_else(|| "".strdup()) }) .unwrap_or_else(|_| "".strdup()) @@ -2758,7 +2757,7 @@ pub unsafe extern "C" fn dc_msg_get_file(msg: *mut dc_msg_t) -> *mut libc::c_cha pub unsafe extern "C" fn dc_msg_get_filename(msg: *mut dc_msg_t) -> *mut libc::c_char { if msg.is_null() { eprintln!("ignoring careless call to dc_msg_get_filename()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_msg = &*msg; ffi_msg.message.get_filename().unwrap_or_default().strdup() @@ -2768,13 +2767,13 @@ pub unsafe extern "C" fn dc_msg_get_filename(msg: *mut dc_msg_t) -> *mut libc::c pub unsafe extern "C" fn dc_msg_get_filemime(msg: *mut dc_msg_t) -> *mut libc::c_char { if msg.is_null() { eprintln!("ignoring careless call to dc_msg_get_filemime()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_msg = &*msg; if let Some(x) = ffi_msg.message.get_filemime() { x.strdup() } else { - dc_strdup(ptr::null()) + "".strdup() } } @@ -3098,7 +3097,7 @@ pub unsafe extern "C" fn dc_contact_get_id(contact: *mut dc_contact_t) -> u32 { pub unsafe extern "C" fn dc_contact_get_addr(contact: *mut dc_contact_t) -> *mut libc::c_char { if contact.is_null() { eprintln!("ignoring careless call to dc_contact_get_addr()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_contact = &*contact; ffi_contact.contact.get_addr().strdup() @@ -3108,7 +3107,7 @@ pub unsafe extern "C" fn dc_contact_get_addr(contact: *mut dc_contact_t) -> *mut pub unsafe extern "C" fn dc_contact_get_name(contact: *mut dc_contact_t) -> *mut libc::c_char { if contact.is_null() { eprintln!("ignoring careless call to dc_contact_get_name()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_contact = &*contact; ffi_contact.contact.get_name().strdup() @@ -3120,7 +3119,7 @@ pub unsafe extern "C" fn dc_contact_get_display_name( ) -> *mut libc::c_char { if contact.is_null() { eprintln!("ignoring careless call to dc_contact_get_display_name()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_contact = &*contact; ffi_contact.contact.get_display_name().strdup() @@ -3132,7 +3131,7 @@ pub unsafe extern "C" fn dc_contact_get_name_n_addr( ) -> *mut libc::c_char { if contact.is_null() { eprintln!("ignoring careless call to dc_contact_get_name_n_addr()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_contact = &*contact; ffi_contact.contact.get_name_n_addr().strdup() @@ -3144,7 +3143,7 @@ pub unsafe extern "C" fn dc_contact_get_first_name( ) -> *mut libc::c_char { if contact.is_null() { eprintln!("ignoring careless call to dc_contact_get_first_name()"); - return dc_strdup(ptr::null()); + return "".strdup(); } let ffi_contact = &*contact; ffi_contact.contact.get_first_name().strdup() diff --git a/deltachat-ffi/src/string.rs b/deltachat-ffi/src/string.rs index 7cfd1c357..256b110b0 100644 --- a/deltachat-ffi/src/string.rs +++ b/deltachat-ffi/src/string.rs @@ -9,7 +9,7 @@ use std::ptr; /// # Examples /// /// ```rust,norun -/// use deltachat::dc_tools::{dc_strdup, to_string_lossy}; +/// use crate::string::{dc_strdup, to_string_lossy}; /// unsafe { /// let str_a = b"foobar\x00" as *const u8 as *const libc::c_char; /// let str_a_copy = dc_strdup(str_a); @@ -17,7 +17,7 @@ use std::ptr; /// assert_ne!(str_a, str_a_copy); /// } /// ``` -pub unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char { +unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char { let ret: *mut libc::c_char; if !s.is_null() { ret = libc::strdup(s); @@ -32,7 +32,7 @@ pub unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char { /// Error type for the [OsStrExt] trait #[derive(Debug, Fail, PartialEq)] -pub enum CStringError { +pub(crate) enum CStringError { /// The string contains an interior null byte #[fail(display = "String contains an interior null byte")] InteriorNullByte, @@ -66,7 +66,7 @@ pub enum CStringError { /// let mut c_ptr: *mut libc::c_char = dc_strdup(path_c.as_ptr()); /// } /// ``` -pub trait OsStrExt { +pub(crate) trait OsStrExt { /// Convert a [std::ffi::OsStr] to an [std::ffi::CString] /// /// This is useful to convert e.g. a [std::path::Path] to @@ -131,15 +131,16 @@ fn os_str_to_c_string_unicode( } /// Convenience methods/associated functions for working with [CString] -/// -/// This is helps transitioning from unsafe code. -pub trait CStringExt { - /// Create a new [CString], yolo style +trait CStringExt { + /// Create a new [CString], best effort /// - /// This unwrap the result, panicking when there are embedded NULL - /// bytes. - fn yolo>>(t: T) -> CString { - CString::new(t).expect("String contains null byte, can not be CString") + /// Like the [to_string_lossy] this doesn't give up in the face of + /// bad input (embedded null bytes in this case) instead it does + /// the best it can by stripping the embedded null bytes. + fn new_lossy>>(t: T) -> CString { + let mut s = t.into(); + s.retain(|&c| c != 0); + CString::new(s).unwrap_or_default() } } @@ -151,7 +152,7 @@ impl CStringExt for CString {} /// Rust strings to raw C strings. This can be clumsy to do correctly /// and the compiler sometimes allows it in an unsafe way. These /// methods make it more succinct and help you get it right. -pub trait StrExt { +pub(crate) trait Strdup { /// Allocate a new raw C `*char` version of this string. /// /// This allocates a new raw C string which must be freed using @@ -168,35 +169,44 @@ pub trait StrExt { unsafe fn strdup(&self) -> *mut libc::c_char; } -impl> StrExt for T { +impl> Strdup for T { unsafe fn strdup(&self) -> *mut libc::c_char { - let tmp = CString::yolo(self.as_ref()); + let tmp = CString::new_lossy(self.as_ref()); + dc_strdup(tmp.as_ptr()) + } +} + +// We can not implement for AsRef because we already implement +// AsRev and this conflicts. So implement for Path directly. +impl Strdup for std::path::Path { + unsafe fn strdup(&self) -> *mut libc::c_char { + let tmp = self.to_c_string().unwrap_or_else(|_| CString::default()); dc_strdup(tmp.as_ptr()) } } /// Convenience methods to turn optional strings into C strings. /// -/// This is the same as the [StrExt] trait but a different trait name -/// to work around the type system not allowing to implement [StrExt] -/// for `Option` When we already have an [StrExt] impl +/// This is the same as the [Strdup] trait but a different trait name +/// to work around the type system not allowing to implement [Strdup] +/// for `Option` When we already have an [Strdup] impl /// for `AsRef<&str>`. /// /// When the [Option] is [Option::Some] this behaves just like -/// [StrExt::strdup], when it is [Option::None] a null pointer is +/// [Strdup::strdup], when it is [Option::None] a null pointer is /// returned. -pub trait OptStrExt { +pub(crate) trait OptStrdup { /// Allocate a new raw C `*char` version of this string, or NULL. /// - /// See [StrExt::strdup] for details. + /// See [Strdup::strdup] for details. unsafe fn strdup(&self) -> *mut libc::c_char; } -impl> OptStrExt for Option { +impl> OptStrdup for Option { unsafe fn strdup(&self) -> *mut libc::c_char { match self { Some(s) => { - let tmp = CString::yolo(s.as_ref()); + let tmp = CString::new_lossy(s.as_ref()); dc_strdup(tmp.as_ptr()) } None => ptr::null_mut(), @@ -204,7 +214,7 @@ impl> OptStrExt for Option { } } -pub fn to_string_lossy(s: *const libc::c_char) -> String { +pub(crate) fn to_string_lossy(s: *const libc::c_char) -> String { if s.is_null() { return "".into(); } @@ -214,7 +224,7 @@ pub fn to_string_lossy(s: *const libc::c_char) -> String { cstr.to_string_lossy().to_string() } -pub fn to_opt_string_lossy(s: *const libc::c_char) -> Option { +pub(crate) fn to_opt_string_lossy(s: *const libc::c_char) -> Option { if s.is_null() { return None; } @@ -235,7 +245,7 @@ pub fn to_opt_string_lossy(s: *const libc::c_char) -> Option { /// /// [Path]: std::path::Path #[cfg(not(target_os = "windows"))] -pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path { +pub(crate) fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path { assert!(!s.is_null(), "cannot be used on null pointers"); use std::os::unix::ffi::OsStrExt; unsafe { @@ -247,7 +257,7 @@ pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path { // as_path() implementation for windows, documented above. #[cfg(target_os = "windows")] -pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path { +pub(crate) fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path { as_path_unicode(s) } @@ -354,8 +364,14 @@ mod tests { } #[test] - fn test_cstring_yolo() { - assert_eq!(CString::new("hello").unwrap(), CString::yolo("hello")); + fn test_cstring_new_lossy() { + assert!(CString::new("hel\x00lo").is_err()); + assert!(CString::new(String::from("hel\x00o")).is_err()); + let r = CString::new("hello").unwrap(); + assert_eq!(CString::new_lossy("hello"), r); + assert_eq!(CString::new_lossy("hel\x00lo"), r); + assert_eq!(CString::new_lossy(String::from("hello")), r); + assert_eq!(CString::new_lossy(String::from("hel\x00lo")), r); } #[test] diff --git a/tests/stress.rs b/tests/stress.rs index 55e9e35a8..ed562ee77 100644 --- a/tests/stress.rs +++ b/tests/stress.rs @@ -44,9 +44,9 @@ fn stress_functions(context: &Context) { // assert!(dc_is_configured(context) != 0, "Missing configured context"); // let setupcode = dc_create_setup_code(context); - // let setupcode_c = CString::yolo(setupcode.clone()); + // let setupcode_c = CString::new(setupcode.clone()).unwrap(); // let setupfile = dc_render_setup_file(context, &setupcode).unwrap(); - // let setupfile_c = CString::yolo(setupfile); + // let setupfile_c = CString::new(setupfile).unwrap(); // let mut headerline_2: *const libc::c_char = ptr::null(); // let payload = dc_decrypt_setup_file(context, setupcode_c.as_ptr(), setupfile_c.as_ptr());