Skip to content

bridge: When converting panics to errors, check for String too #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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