Skip to content

Commit

Permalink
bridge: When converting panics to errors, check for String too
Browse files Browse the repository at this point in the history
  • Loading branch information
jrose-signal committed Oct 25, 2021
1 parent 58b5256 commit 11ebe52
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 22 deletions.
14 changes: 1 addition & 13 deletions rust/bridge/jni/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
use jni::objects::{GlobalRef, JClass, JObject, JValue};
use jni::sys::jint;
use jni::{JNIEnv, JavaVM};
use libsignal_bridge::jni_signature;
use std::any::Any;
use libsignal_bridge::{describe_panic, jni_signature};
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::process::abort;

Expand Down Expand Up @@ -118,17 +117,6 @@ impl log::Log for JniLogger {
fn flush(&self) {}
}

// See https://github.com/rust-lang/rfcs/issues/1389
fn describe_panic(any: &Box<dyn Any + Send>) -> String {
if let Some(msg) = any.downcast_ref::<&str>() {
msg.to_string()
} else if let Some(msg) = any.downcast_ref::<String>() {
msg.to_string()
} else {
"(break on rust_panic to debug)".to_string()
}
}

/// A low-level version of `run_ffi_safe` that just aborts on errors.
///
/// This is important for logging failures because we might want to log during the normal
Expand Down
10 changes: 5 additions & 5 deletions rust/bridge/shared/src/ffi/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use device_transfer::Error as DeviceTransferError;
use libsignal_protocol::*;
use signal_crypto::Error as SignalCryptoError;

use crate::support::describe_panic;

/// The top-level error type (opaquely) returned to C clients when something goes wrong.
#[derive(Debug)]
pub enum SignalFfiError {
Expand Down Expand Up @@ -39,11 +41,9 @@ impl fmt::Display for SignalFfiError {
SignalFfiError::InsufficientOutputSize(n, h) => {
write!(f, "needed {} elements only {} provided", n, h)
}

SignalFfiError::UnexpectedPanic(e) => match e.downcast_ref::<&'static str>() {
Some(s) => write!(f, "unexpected panic: {}", s),
None => write!(f, "unknown unexpected panic"),
},
SignalFfiError::UnexpectedPanic(e) => {
write!(f, "unexpected panic: {}", describe_panic(e))
}
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions rust/bridge/shared/src/jni/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use hsm_enclave::Error as HsmEnclaveError;
use libsignal_protocol::*;
use signal_crypto::Error as SignalCryptoError;

use crate::support::describe_panic;

use super::*;

/// The top-level error type for when something goes wrong.
Expand Down Expand Up @@ -47,10 +49,9 @@ impl fmt::Display for SignalJniError {
SignalJniError::HsmEnclave(e) => {
write!(f, "{}", e)
}
SignalJniError::UnexpectedPanic(e) => match e.downcast_ref::<&'static str>() {
Some(s) => write!(f, "unexpected panic: {}", s),
None => write!(f, "unknown unexpected panic"),
},
SignalJniError::UnexpectedPanic(e) => {
write!(f, "unexpected panic: {}", describe_panic(e))
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions rust/bridge/shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub mod node;

#[macro_use]
mod support;
pub use support::describe_panic;

pub mod crypto;
pub mod protocol;
Expand Down
11 changes: 11 additions & 0 deletions rust/bridge/shared/src/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ pub(crate) use paste::paste;
mod transform_helper;
pub(crate) use transform_helper::*;

// See https://github.com/rust-lang/rfcs/issues/1389
pub fn describe_panic(any: &Box<dyn std::any::Any + Send>) -> String {
if let Some(msg) = any.downcast_ref::<&str>() {
msg.to_string()
} else if let Some(msg) = any.downcast_ref::<String>() {
msg.to_string()
} else {
"(break on rust_panic to debug)".to_string()
}
}

/// Exposes a Rust type to each of the bridges as a boxed value.
///
/// Full form:
Expand Down

0 comments on commit 11ebe52

Please sign in to comment.