JNI: Rethrow callback exceptions instead of wrapping them

Cargo.lock

@@ -848,6 +848,7 @@ dependencies = [
  "jni",
  "libc",
  "libsignal-protocol-rust",
+ "log",
  "paste",
 ]
 

java/tests/src/test/java/org/whispersystems/libsignal/SessionBuilderTest.java

@@ -293,8 +293,7 @@ public class SessionBuilderTest extends TestCase {
       plaintext = bobSessionCipher.decrypt(incomingMessage);
       throw new AssertionError("Decrypt should have failed!");
     } catch (InvalidKeyIdException e) {
-      // Note: This is the message coming from libsignal-client, NOT from TestNoSignedPreKeysStore.
-      assertEquals("invalid signed prekey identifier", e.getMessage());
+      assertEquals("TestNoSignedPreKeysStore rejected loading 22", e.getMessage());
     }
   }
 
@@ -335,10 +334,8 @@ public class SessionBuilderTest extends TestCase {
       throw new AssertionError("Decrypt should have failed!");
     } catch (InvalidKeyIdException e) {
       throw new AssertionError("libsignal-client swallowed the exception");
-    } catch (RuntimeException e) {
-      assertEquals("exception thrown while calling 'loadSignedPreKey'", e.getMessage());
-      assertNotNull(e.getCause());
-      assertTrue(e.getCause() instanceof TestBadSignedPreKeysStore.CustomException);
+    } catch (TestBadSignedPreKeysStore.CustomException e) {
+      // success!
     }
   }
 

rust/bridge/jni/src/lib.rs

@@ -919,7 +919,7 @@ impl<'a> JniPreKeyStore<'a> {
             &callback_args,
             callback_sig,
             "loadPreKey",
-            Some("org/whispersystems/libsignal/InvalidKeyIdException"),
+            None,
         )?;
         match pk {
             Some(pk) => Ok(pk),
@@ -1020,7 +1020,7 @@ impl<'a> JniSignedPreKeyStore<'a> {
             &callback_args,
             callback_sig,
             "loadSignedPreKey",
-            Some("org/whispersystems/libsignal/InvalidKeyIdException"),
+            None,
         )?;
         match spk {
             Some(spk) => Ok(spk),

rust/bridge/shared/Cargo.toml

@@ -13,6 +13,7 @@ license = "AGPL-3.0-only"
 [dependencies]
 libsignal-protocol-rust = { path = "../../protocol" }
 aes-gcm-siv = { path = "../../aes-gcm-siv" }
+log = "0.4.11"
 paste = "1.0"
 
 libc = { version = "0.2", optional = true }

rust/bridge/shared/src/jni/mod.rs

@@ -3,7 +3,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 //
 
-use jni::objects::{JObject, JThrowable, JValue};
+use jni::objects::{JObject, JValue};
 use jni::sys::{_jobject, jboolean, jint, jlong};
 
 use aes_gcm_siv::Error as AesGcmSivError;
@@ -20,25 +20,42 @@ pub use error::*;
 pub type ObjectHandle = jlong;
 
 fn throw_error(env: &JNIEnv, error: SignalJniError) {
-    if let SignalJniError::Signal(SignalProtocolError::ApplicationCallbackError(
+    // Handle special cases first.
+    let error = match error {
+        SignalJniError::Signal(SignalProtocolError::ApplicationCallbackError(
             callback,
-        underlying_exception,
-    )) = error
-    {
-        let underlying_exception = underlying_exception.downcast::<ThrownException>().unwrap();
-        let message = env
-            .new_string(format!("exception thrown while calling '{}'", callback))
-            .unwrap();
-        let throwable = env
-            .new_object(
-                "java/lang/RuntimeException",
-                "(Ljava/lang/String;Ljava/lang/Throwable;)V",
-                &[message.into(), underlying_exception.as_obj().into()],
-            )
-            .unwrap();
-        env.throw(JThrowable::from(throwable)).unwrap();
+            exception,
+        )) => {
+            match exception.downcast::<ThrownException>() {
+                Ok(exception) => {
+                    if let Err(e) = env.throw(exception.as_obj()) {
+                        log::error!("failed to rethrow exception from {}: {}", callback, e);
+                    }
                     return;
                 }
+                Err(other_underlying_error) => {
+                    // Fall through to generic handling below.
+                    SignalJniError::Signal(SignalProtocolError::ApplicationCallbackError(
+                        callback,
+                        other_underlying_error,
+                    ))
+                }
+            }
+        }
+
+        SignalJniError::Signal(SignalProtocolError::UntrustedIdentity(ref addr)) => {
+            let result = env.throw_new(
+                "org/whispersystems/libsignal/UntrustedIdentityException",
+                addr.name(),
+            );
+            if let Err(e) = result {
+                log::error!("failed to throw exception for {}: {}", error, e);
+            }
+            return;
+        }
+
+        e => e,
+    };
 
     let exception_type = match error {
         SignalJniError::NullHandle => "java/lang/NullPointerException",
@@ -83,10 +100,6 @@ fn throw_error(env: &JNIEnv, error: SignalJniError) {
             "org/whispersystems/libsignal/LegacyMessageException"
         }
 
-        SignalJniError::Signal(SignalProtocolError::UntrustedIdentity(_)) => {
-            "org/whispersystems/libsignal/UntrustedIdentityException"
-        }
-
         SignalJniError::Signal(SignalProtocolError::InvalidState(_, _))
         | SignalJniError::Signal(SignalProtocolError::NoSenderKeyState)
         | SignalJniError::Signal(SignalProtocolError::InvalidSessionStructure) => {
@@ -105,14 +118,9 @@ fn throw_error(env: &JNIEnv, error: SignalJniError) {
         SignalJniError::Jni(_) => "java/lang/RuntimeException",
     };
 
-    let error_string = match error {
-        SignalJniError::Signal(SignalProtocolError::UntrustedIdentity(addr)) => {
-            addr.name().to_string()
+    if let Err(e) = env.throw_new(exception_type, error.to_string()) {
+        log::error!("failed to throw exception for {}: {}", error, e);
     }
-        e => format!("{}", e),
-    };
-
-    env.throw_new(exception_type, error_string).unwrap();
 }
 
 // A dummy value to return when we are throwing an exception