Don't unwrap when Result can be returned instead

This commit is contained in:
Kornel 2025-06-05 02:15:42 +01:00 committed by Kornel
parent 29c05d41cd
commit bcec9462af
8 changed files with 86 additions and 30 deletions

View File

@ -323,7 +323,7 @@ impl Asn1Time {
#[allow(clippy::should_implement_trait)]
pub fn from_str(s: &str) -> Result<Asn1Time, ErrorStack> {
unsafe {
let s = CString::new(s).unwrap();
let s = CString::new(s).map_err(ErrorStack::internal_error)?;
let time = Asn1Time::new()?;
cvt(ffi::ASN1_TIME_set_string(time.as_ptr(), s.as_ptr()))?;
@ -560,7 +560,7 @@ impl Asn1Object {
pub fn from_str(txt: &str) -> Result<Asn1Object, ErrorStack> {
unsafe {
ffi::init();
let txt = CString::new(txt).unwrap();
let txt = CString::new(txt).map_err(ErrorStack::internal_error)?;
let obj: *mut ffi::ASN1_OBJECT = cvt_p(ffi::OBJ_txt2obj(txt.as_ptr() as *const _, 0))?;
Ok(Asn1Object::from_ptr(obj))
}

View File

@ -838,7 +838,7 @@ impl BigNum {
pub fn from_dec_str(s: &str) -> Result<BigNum, ErrorStack> {
unsafe {
ffi::init();
let c_str = CString::new(s.as_bytes()).unwrap();
let c_str = CString::new(s.as_bytes()).map_err(ErrorStack::internal_error)?;
let mut bn = ptr::null_mut();
cvt(ffi::BN_dec2bn(&mut bn, c_str.as_ptr() as *const _))?;
Ok(BigNum::from_ptr(bn))
@ -850,7 +850,7 @@ impl BigNum {
pub fn from_hex_str(s: &str) -> Result<BigNum, ErrorStack> {
unsafe {
ffi::init();
let c_str = CString::new(s.as_bytes()).unwrap();
let c_str = CString::new(s.as_bytes()).map_err(ErrorStack::internal_error)?;
let mut bn = ptr::null_mut();
cvt(ffi::BN_hex2bn(&mut bn, c_str.as_ptr() as *const _))?;
Ok(BigNum::from_ptr(bn))

View File

@ -48,6 +48,12 @@ impl ErrorStack {
error.put();
}
}
/// Used to report errors from the Rust crate
#[cold]
pub(crate) fn internal_error(err: impl error::Error) -> Self {
Self(vec![Error::new_internal(err.to_string())])
}
}
impl ErrorStack {
@ -69,7 +75,11 @@ impl fmt::Display for ErrorStack {
fmt.write_str(" ")?;
}
first = false;
write!(fmt, "[{}]", err.reason().unwrap_or("unknown reason"))?;
write!(
fmt,
"[{}]",
err.reason_internal().unwrap_or("unknown reason")
)?;
}
Ok(())
}
@ -101,6 +111,8 @@ pub struct Error {
unsafe impl Sync for Error {}
unsafe impl Send for Error {}
static BORING_INTERNAL: &CStr = c"boring-rust";
impl Error {
/// Returns the first error on the OpenSSL error stack.
pub fn get() -> Option<Error> {
@ -171,6 +183,9 @@ impl Error {
/// Returns the name of the library reporting the error, if available.
pub fn library(&self) -> Option<&'static str> {
if self.is_internal() {
return None;
}
unsafe {
let cstr = ffi::ERR_lib_error_string(self.code);
if cstr.is_null() {
@ -189,6 +204,9 @@ impl Error {
/// Returns the name of the function reporting the error.
pub fn function(&self) -> Option<&'static str> {
if self.is_internal() {
return None;
}
unsafe {
let cstr = ffi::ERR_func_error_string(self.code);
if cstr.is_null() {
@ -237,6 +255,28 @@ impl Error {
pub fn data(&self) -> Option<&str> {
self.data.as_deref()
}
fn new_internal(msg: String) -> Self {
Self {
code: ffi::ERR_PACK(ffi::ERR_LIB_NONE.0 as _, 0, 0) as _,
file: BORING_INTERNAL.as_ptr(),
line: 0,
data: Some(msg.into()),
}
}
fn is_internal(&self) -> bool {
std::ptr::eq(self.file, BORING_INTERNAL.as_ptr())
}
// reason() needs 'static
fn reason_internal(&self) -> Option<&str> {
if self.is_internal() {
self.data()
} else {
self.reason()
}
}
}
impl fmt::Debug for Error {
@ -268,7 +308,7 @@ impl fmt::Display for Error {
write!(
fmt,
"{}\n\nCode: {:08X}\nLoc: {}:{}",
self.reason().unwrap_or("unknown TLS error"),
self.reason_internal().unwrap_or("unknown TLS error"),
self.code(),
self.file(),
self.line()
@ -277,3 +317,14 @@ impl fmt::Display for Error {
}
impl error::Error for Error {}
#[test]
fn internal_err() {
let e = ErrorStack::internal_error(io::Error::other("hello, boring"));
assert_eq!(1, e.errors().len());
assert!(e.to_string().contains("hello, boring"), "{e} {e:?}");
e.put();
let e = ErrorStack::get();
assert!(e.to_string().contains("hello, boring"), "{e} {e:?}");
}

View File

@ -84,8 +84,8 @@ impl Nid {
#[allow(clippy::trivially_copy_pass_by_ref)]
pub fn long_name(&self) -> Result<&'static str, ErrorStack> {
unsafe {
cvt_p(ffi::OBJ_nid2ln(self.0) as *mut c_char)
.map(|nameptr| str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).unwrap())
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)
}
}
@ -94,8 +94,8 @@ impl Nid {
#[allow(clippy::trivially_copy_pass_by_ref)]
pub fn short_name(&self) -> Result<&'static str, ErrorStack> {
unsafe {
cvt_p(ffi::OBJ_nid2sn(self.0) as *mut c_char)
.map(|nameptr| str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).unwrap())
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)
}
}

View File

@ -34,7 +34,7 @@ impl Pkcs12Ref {
/// Extracts the contents of the `Pkcs12`.
pub fn parse(&self, pass: &str) -> Result<ParsedPkcs12, ErrorStack> {
unsafe {
let pass = CString::new(pass.as_bytes()).unwrap();
let pass = CString::new(pass.as_bytes()).map_err(ErrorStack::internal_error)?;
let mut pkey = ptr::null_mut();
let mut cert = ptr::null_mut();
@ -161,8 +161,8 @@ impl Pkcs12Builder {
T: HasPrivate,
{
unsafe {
let pass = CString::new(password).unwrap();
let friendly_name = CString::new(friendly_name).unwrap();
let pass = CString::new(password).map_err(ErrorStack::internal_error)?;
let friendly_name = CString::new(friendly_name).map_err(ErrorStack::internal_error)?;
let pkey = pkey.as_ptr();
let cert = cert.as_ptr();
let ca = self

View File

@ -408,7 +408,7 @@ impl PKey<Private> {
unsafe {
ffi::init();
let bio = MemBioSlice::new(der)?;
let passphrase = CString::new(passphrase).unwrap();
let passphrase = CString::new(passphrase).map_err(ErrorStack::internal_error)?;
cvt_p(ffi::d2i_PKCS8PrivateKey_bio(
bio.as_ptr(),
ptr::null_mut(),

View File

@ -1269,7 +1269,8 @@ impl SslContextBuilder {
#[cfg(feature = "rpk")]
assert!(!self.is_rpk, "This API is not supported for RPK");
let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes())
.map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_CTX_load_verify_locations(
self.as_ptr(),
@ -1340,7 +1341,8 @@ impl SslContextBuilder {
#[cfg(feature = "rpk")]
assert!(!self.is_rpk, "This API is not supported for RPK");
let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes())
.map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_CTX_use_certificate_file(
self.as_ptr(),
@ -1361,7 +1363,8 @@ impl SslContextBuilder {
&mut self,
file: P,
) -> Result<(), ErrorStack> {
let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes())
.map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_CTX_use_certificate_chain_file(
self.as_ptr(),
@ -1401,7 +1404,8 @@ impl SslContextBuilder {
file: P,
file_type: SslFiletype,
) -> Result<(), ErrorStack> {
let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes())
.map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_CTX_use_PrivateKey_file(
self.as_ptr(),
@ -1564,7 +1568,7 @@ impl SslContextBuilder {
#[corresponds(SSL_CTX_set_tlsext_use_srtp)]
pub fn set_tlsext_use_srtp(&mut self, protocols: &str) -> Result<(), ErrorStack> {
unsafe {
let cstr = CString::new(protocols).unwrap();
let cstr = CString::new(protocols).map_err(ErrorStack::internal_error)?;
let r = ffi::SSL_CTX_set_tlsext_use_srtp(self.as_ptr(), cstr.as_ptr());
// fun fact, set_tlsext_use_srtp has a reversed return code D:
@ -1908,7 +1912,7 @@ impl SslContextBuilder {
/// Sets the context's supported signature algorithms.
#[corresponds(SSL_CTX_set1_sigalgs_list)]
pub fn set_sigalgs_list(&mut self, sigalgs: &str) -> Result<(), ErrorStack> {
let sigalgs = CString::new(sigalgs).unwrap();
let sigalgs = CString::new(sigalgs).map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_CTX_set1_sigalgs_list(self.as_ptr(), sigalgs.as_ptr()) as c_int)
.map(|_| ())
@ -1968,7 +1972,7 @@ impl SslContextBuilder {
#[cfg(not(feature = "kx-safe-default"))]
#[corresponds(SSL_CTX_set1_curves_list)]
pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> {
let curves = CString::new(curves).unwrap();
let curves = CString::new(curves).map_err(ErrorStack::internal_error)?;
unsafe {
cvt_0i(ffi::SSL_CTX_set1_curves_list(
self.as_ptr(),
@ -2802,7 +2806,7 @@ impl SslRef {
#[corresponds(SSL_set1_curves_list)]
pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> {
let curves = CString::new(curves).unwrap();
let curves = CString::new(curves).map_err(ErrorStack::internal_error)?;
unsafe {
cvt_0i(ffi::SSL_set1_curves_list(
self.as_ptr(),
@ -3086,7 +3090,7 @@ impl SslRef {
/// It has no effect for a server-side connection.
#[corresponds(SSL_set_tlsext_host_name)]
pub fn set_hostname(&mut self, hostname: &str) -> Result<(), ErrorStack> {
let cstr = CString::new(hostname).unwrap();
let cstr = CString::new(hostname).map_err(ErrorStack::internal_error)?;
unsafe {
cvt(ffi::SSL_set_tlsext_host_name(self.as_ptr(), cstr.as_ptr() as *mut _) as c_int)
.map(|_| ())
@ -3273,7 +3277,7 @@ impl SslRef {
#[corresponds(SSL_set_tlsext_use_srtp)]
pub fn set_tlsext_use_srtp(&mut self, protocols: &str) -> Result<(), ErrorStack> {
unsafe {
let cstr = CString::new(protocols).unwrap();
let cstr = CString::new(protocols).map_err(ErrorStack::internal_error)?;
let r = ffi::SSL_set_tlsext_use_srtp(self.as_ptr(), cstr.as_ptr());
// fun fact, set_tlsext_use_srtp has a reversed return code D:

View File

@ -894,8 +894,8 @@ impl X509Extension {
name: &str,
value: &str,
) -> Result<X509Extension, ErrorStack> {
let name = CString::new(name).unwrap();
let value = CString::new(value).unwrap();
let name = CString::new(name).map_err(ErrorStack::internal_error)?;
let value = CString::new(value).map_err(ErrorStack::internal_error)?;
let mut ctx;
unsafe {
ffi::init();
@ -940,7 +940,7 @@ impl X509Extension {
name: Nid,
value: &str,
) -> Result<X509Extension, ErrorStack> {
let value = CString::new(value).unwrap();
let value = CString::new(value).map_err(ErrorStack::internal_error)?;
let mut ctx;
unsafe {
ffi::init();
@ -1004,7 +1004,7 @@ impl X509NameBuilder {
#[corresponds(X509_NAME_add_entry_by_txt)]
pub fn append_entry_by_text(&mut self, field: &str, value: &str) -> Result<(), ErrorStack> {
unsafe {
let field = CString::new(field).unwrap();
let field = CString::new(field).map_err(ErrorStack::internal_error)?;
assert!(value.len() <= ValueLen::MAX as usize);
cvt(ffi::X509_NAME_add_entry_by_txt(
self.0.as_ptr(),
@ -1028,7 +1028,7 @@ impl X509NameBuilder {
ty: Asn1Type,
) -> Result<(), ErrorStack> {
unsafe {
let field = CString::new(field).unwrap();
let field = CString::new(field).map_err(ErrorStack::internal_error)?;
assert!(value.len() <= ValueLen::MAX as usize);
cvt(ffi::X509_NAME_add_entry_by_txt(
self.0.as_ptr(),
@ -1116,7 +1116,8 @@ impl X509Name {
///
/// This is commonly used in conjunction with `SslContextBuilder::set_client_ca_list`.
pub fn load_client_ca_file<P: AsRef<Path>>(file: P) -> Result<Stack<X509Name>, ErrorStack> {
let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes())
.map_err(ErrorStack::internal_error)?;
unsafe { cvt_p(ffi::SSL_load_client_CA_file(file.as_ptr())).map(|p| Stack::from_ptr(p)) }
}