CStr UTF-8 improvements

Author: kornelski

boring/src/asn1.rs

@@ -63,20 +63,19 @@ foreign_type_and_impl_send_sync! {
 
 impl fmt::Display for Asn1GeneralizedTimeRef {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        unsafe {
-            let mem_bio = match MemBio::new() {
-                Err(_) => return f.write_str("error"),
-                Ok(m) => m,
-            };
-            let print_result = cvt(ffi::ASN1_GENERALIZEDTIME_print(
-                mem_bio.as_ptr(),
-                self.as_ptr(),
-            ));
-            match print_result {
-                Err(_) => f.write_str("error"),
-                Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())),
-            }
-        }
+        let bio = MemBio::new().ok();
+        let msg = bio
+            .as_ref()
+            .and_then(|mem_bio| unsafe {
+                cvt(ffi::ASN1_GENERALIZEDTIME_print(
+                    mem_bio.as_ptr(),
+                    self.as_ptr(),
+                ))
+                .ok()?;
+                str::from_utf8(mem_bio.get_buf()).ok()
+            })
+            .unwrap_or("error");
+        f.write_str(msg)
     }
 }
 
@@ -528,7 +527,20 @@ impl Asn1BitStringRef {
     #[corresponds(ASN1_STRING_get0_data)]
     #[must_use]
     pub fn as_slice(&self) -> &[u8] {
-        unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) }
+        unsafe {
+            let ptr = ASN1_STRING_get0_data(self.as_ptr().cast());
+            if ptr.is_null() {
+                return &[];
+            }
+            slice::from_raw_parts(ptr, self.len())
+        }
+    }
+
+    /// Returns the Asn1BitString as a str, if possible.
+    #[corresponds(ASN1_STRING_get0_data)]
+    #[must_use]
+    pub fn to_str(&self) -> Option<&str> {
+        str::from_utf8(self.as_slice()).ok()
     }
 
     /// Returns the number of bytes in the string.
@@ -601,10 +613,11 @@ impl fmt::Display for Asn1ObjectRef {
                 self.as_ptr(),
                 0,
             );
-            match str::from_utf8(&buf[..len as usize]) {
-                Err(_) => fmt.write_str("error"),
-                Ok(s) => fmt.write_str(s),
-            }
+            fmt.write_str(
+                buf.get(..len as usize)
+                    .and_then(|s| str::from_utf8(s).ok())
+                    .unwrap_or("error"),
+            )
         }
     }
 }

boring/src/bio.rs

@@ -68,7 +68,10 @@ impl MemBio {
         unsafe {
             let mut ptr = ptr::null_mut();
             let len = ffi::BIO_get_mem_data(self.0, &mut ptr);
-            slice::from_raw_parts(ptr as *const _ as *const _, len as usize)
+            if ptr.is_null() {
+                return &[];
+            }
+            slice::from_raw_parts(ptr.cast_const().cast(), len as usize)
         }
     }
 }

boring/src/error.rs

@@ -132,9 +132,9 @@ impl Error {
                     // The memory referenced by data is only valid until that slot is overwritten
                     // in the error stack, so we'll need to copy it off if it's dynamic
                     let data = if flags & ffi::ERR_FLAG_STRING != 0 {
-                        let bytes = CStr::from_ptr(data as *const _).to_bytes();
-                        let data = String::from_utf8_lossy(bytes).into_owned();
-                        Some(data.into())
+                        Some(Cow::Owned(
+                            CStr::from_ptr(data.cast()).to_string_lossy().into_owned(),
+                        ))
                     } else {
                         None
                     };
@@ -196,8 +196,10 @@ impl Error {
             if cstr.is_null() {
                 return None;
             }
-            let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
-            str::from_utf8(bytes).ok()
+            CStr::from_ptr(cstr.cast())
+                .to_str()
+                .ok()
+                .filter(|&msg| msg != "unknown library")
         }
     }
 
@@ -232,8 +234,7 @@ impl Error {
             if cstr.is_null() {
                 return None;
             }
-            let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
-            str::from_utf8(bytes).ok()
+            CStr::from_ptr(cstr.cast()).to_str().ok()
         }
     }
 
@@ -250,8 +251,9 @@ impl Error {
             if self.file.is_null() {
                 return "";
             }
-            let bytes = CStr::from_ptr(self.file as *const _).to_bytes();
-            str::from_utf8(bytes).unwrap_or_default()
+            CStr::from_ptr(self.file.cast())
+                .to_str()
+                .unwrap_or_default()
         }
     }
 

boring/src/nid.rs

@@ -88,7 +88,9 @@ impl Nid {
     pub fn long_name(&self) -> Result<&'static str, ErrorStack> {
         unsafe {
             let nameptr = cvt_p(ffi::OBJ_nid2ln(self.0) as *mut c_char)?;
-            str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).map_err(ErrorStack::internal_error)
+            CStr::from_ptr(nameptr)
+                .to_str()
+                .map_err(ErrorStack::internal_error)
         }
     }
 
@@ -98,7 +100,9 @@ impl Nid {
     pub fn short_name(&self) -> Result<&'static str, ErrorStack> {
         unsafe {
             let nameptr = cvt_p(ffi::OBJ_nid2sn(self.0) as *mut c_char)?;
-            str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).map_err(ErrorStack::internal_error)
+            CStr::from_ptr(nameptr)
+                .to_str()
+                .map_err(ErrorStack::internal_error)
         }
     }
 

boring/src/ssl/callbacks.rs

@@ -451,14 +451,14 @@ where
 {
     // SAFETY: boring provides valid inputs.
     let ssl = unsafe { SslRef::from_ptr(ssl as *mut _) };
-    let line = unsafe { str::from_utf8_unchecked(CStr::from_ptr(line).to_bytes()) };
+    let line = unsafe { CStr::from_ptr(line).to_string_lossy() };
 
     let callback = ssl
         .ssl_context()
         .ex_data(SslContext::cached_ex_index::<F>())
         .expect("BUG: get session callback missing");
 
-    callback(ssl, line);
+    callback(ssl, &line);
 }
 
 pub(super) unsafe extern "C" fn raw_sign<M>(

boring/src/ssl/mod.rs

@@ -2564,7 +2564,7 @@ impl SslCipherRef {
             CStr::from_ptr(ptr as *const _)
         };
 
-        str::from_utf8(version.to_bytes()).unwrap()
+        version.to_str().unwrap()
     }
 
     /// Returns the number of bits used for the cipher.
@@ -2590,7 +2590,7 @@ impl SslCipherRef {
             // SSL_CIPHER_description requires a buffer of at least 128 bytes.
             let mut buf = [0; 128];
             let ptr = ffi::SSL_CIPHER_description(self.as_ptr(), buf.as_mut_ptr(), 128);
-            String::from_utf8(CStr::from_ptr(ptr as *const _).to_bytes().to_vec()).unwrap()
+            CStr::from_ptr(ptr.cast()).to_string_lossy().into_owned()
         }
     }
 
@@ -3216,6 +3216,8 @@ impl SslRef {
     }
 
     /// Returns a short string describing the state of the session.
+    ///
+    /// Returns empty string if the state wasn't valid UTF-8
     #[corresponds(SSL_state_string)]
     #[must_use]
     pub fn state_string(&self) -> &'static str {
@@ -3224,10 +3226,12 @@ impl SslRef {
             CStr::from_ptr(ptr as *const _)
         };
 
-        str::from_utf8(state.to_bytes()).unwrap()
+        state.to_str().unwrap_or_default()
     }
 
     /// Returns a longer string describing the state of the session.
+    ///
+    /// Returns empty string if the state wasn't valid UTF-8
     #[corresponds(SSL_state_string_long)]
     #[must_use]
     pub fn state_string_long(&self) -> &'static str {
@@ -3236,7 +3240,7 @@ impl SslRef {
             CStr::from_ptr(ptr as *const _)
         };
 
-        str::from_utf8(state.to_bytes()).unwrap()
+        state.to_str().unwrap_or_default()
     }
 
     /// Sets the host name to be sent to the server for Server Name Indication (SNI).
@@ -3348,6 +3352,8 @@ impl SslRef {
     }
 
     /// Returns a string describing the protocol version of the session.
+    ///
+    /// This may panic if the string isn't valid UTF-8 for some reason. Use [`Self::version2`] instead.
     #[corresponds(SSL_get_version)]
     #[must_use]
     pub fn version_str(&self) -> &'static str {
@@ -3356,7 +3362,7 @@ impl SslRef {
             CStr::from_ptr(ptr as *const _)
         };
 
-        str::from_utf8(version.to_bytes()).unwrap()
+        version.to_str().unwrap()
     }
 
     /// Sets the minimum supported protocol version.

boring/src/string.rs

@@ -13,6 +13,9 @@ foreign_type_and_impl_send_sync! {
     type CType = c_char;
     fn drop = free;
 
+    /// # Safety
+    ///
+    /// MUST be UTF-8
     pub struct OpensslString;
 }
 

boring/src/x509/mod.rs

@@ -19,7 +19,6 @@ use std::mem;
 use std::net::IpAddr;
 use std::path::Path;
 use std::ptr;
-use std::slice;
 use std::str;
 use std::sync::{LazyLock, Once};
 
@@ -1542,6 +1541,8 @@ impl X509VerifyError {
     }
 
     /// Return a human readable error string from the verification error.
+    ///
+    /// Returns empty string if the message was not UTF-8
     #[corresponds(X509_verify_cert_error_string)]
     #[allow(clippy::trivially_copy_pass_by_ref)]
     #[must_use]
@@ -1550,7 +1551,7 @@ impl X509VerifyError {
 
         unsafe {
             let s = ffi::X509_verify_cert_error_string(c_long::from(self.0));
-            str::from_utf8(CStr::from_ptr(s).to_bytes()).unwrap()
+            CStr::from_ptr(s).to_str().unwrap_or_default()
         }
     }
 }
@@ -1702,14 +1703,12 @@ impl GeneralNameRef {
                 return None;
             }
 
-            let ptr = ASN1_STRING_get0_data((*self.as_ptr()).d.ia5 as *mut _);
-            let len = ffi::ASN1_STRING_length((*self.as_ptr()).d.ia5 as *mut _);
+            let asn = Asn1BitStringRef::from_ptr((*self.as_ptr()).d.ia5);
 
-            let slice = slice::from_raw_parts(ptr, len as usize);
             // IA5Strings are stated to be ASCII (specifically IA5). Hopefully
             // OpenSSL checks that when loading a certificate but if not we'll
             // use this instead of from_utf8_unchecked just in case.
-            str::from_utf8(slice).ok()
+            asn.to_str()
         }
     }
 
@@ -1739,10 +1738,7 @@ impl GeneralNameRef {
                 return None;
             }
 
-            let ptr = ASN1_STRING_get0_data((*self.as_ptr()).d.ip as *mut _);
-            let len = ffi::ASN1_STRING_length((*self.as_ptr()).d.ip as *mut _);
-
-            Some(slice::from_raw_parts(ptr, len as usize))
+            Some(Asn1BitStringRef::from_ptr((*self.as_ptr()).d.ip).as_slice())
         }
     }
 }
@@ -1818,8 +1814,8 @@ impl Stackable for X509Object {
 use crate::ffi::{X509_get0_signature, X509_getm_notAfter, X509_getm_notBefore, X509_up_ref};
 
 use crate::ffi::{
-    ASN1_STRING_get0_data, X509_ALGOR_get0, X509_REQ_get_subject_name, X509_REQ_get_version,
-    X509_STORE_CTX_get0_chain, X509_set1_notAfter, X509_set1_notBefore,
+    X509_ALGOR_get0, X509_REQ_get_subject_name, X509_REQ_get_version, X509_STORE_CTX_get0_chain,
+    X509_set1_notAfter, X509_set1_notBefore,
 };
 
 use crate::ffi::X509_OBJECT_get0_X509;