mobile: More JNI cleanup (envoyproxy#34181)

This PR cleans up the JNI code by removing the Envoy::JNI::Exception and Envoy::JNI::String and adds exceptionClear in the JniHelper so that everything behaves the same way without more going through many layers.

Risk Level: low
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: mobile

Signed-off-by: Fredy Wijaya <[email protected]>

mobile/library/jni/BUILD

@@ -19,7 +19,6 @@ envoy_cc_library(
         "//library/common/types:c_types_lib",
         "//library/common/types:managed_types_lib",
         "//library/common/types:matcher_data_lib",
-        "//library/jni/types:jni_exception_lib",
         "@envoy//source/common/buffer:buffer_lib",
         "@envoy//source/common/common:assert_lib",
         "@envoy//source/common/http:header_map_lib",
@@ -65,7 +64,6 @@ envoy_cc_library(
         "//library/common/extensions/cert_validator/platform_bridge:c_types_lib",
         "//library/common/extensions/key_value/platform:config",
         "//library/common/types:managed_types_lib",
-        "//library/jni/types:jni_exception_lib",
         "@envoy//source/common/protobuf",
     ],
     # We need this to ensure that we link this into the .so even though there are no code references.
@@ -88,7 +86,6 @@ envoy_cc_library(
         "//library/common/bridge:utility_lib",
         "//library/common/extensions/cert_validator/platform_bridge:c_types_lib",
         "//library/common/types:c_types_lib",
-        "//library/jni/types:jni_exception_lib",
         "@envoy//bazel:boringssl",
     ],
 )

mobile/library/jni/android_network_utility.cc

@@ -2,7 +2,6 @@
 
 #include "library/common/bridge//utility.h"
 #include "library/jni/jni_utility.h"
-#include "library/jni/types/exception.h"
 #include "openssl/ssl.h"
 
 namespace Envoy {
@@ -88,21 +87,23 @@ static void jvmVerifyX509CertChain(const std::vector<std::string>& cert_chain,
   JniHelper jni_helper(JniHelper::getThreadLocalEnv());
   LocalRefUniquePtr<jobject> result =
       callJvmVerifyX509CertChain(jni_helper, cert_chain, auth_type, hostname);
-  if (Exception::checkAndClear()) {
+  if (jni_helper.exceptionCheck()) {
     *status = CertVerifyStatus::NotYetValid;
+    jni_helper.exceptionCleared();
   } else {
     extractCertVerifyResult(jni_helper, result.get(), status, is_issued_by_known_root,
                             verified_chain);
-    if (Exception::checkAndClear()) {
+    if (jni_helper.exceptionCheck()) {
       *status = CertVerifyStatus::Failed;
+      jni_helper.exceptionCleared();
     }
   }
 }
 
 } // namespace
 
 // `auth_type` and `host` are expected to be UTF-8 encoded.
-LocalRefUniquePtr<jobject> callJvmVerifyX509CertChain(Envoy::JNI::JniHelper& jni_helper,
+LocalRefUniquePtr<jobject> callJvmVerifyX509CertChain(JniHelper& jni_helper,
                                                       const std::vector<std::string>& cert_chain,
                                                       std::string auth_type,
                                                       absl::string_view hostname) {

mobile/library/jni/jni_helper.cc

@@ -92,14 +92,18 @@ void JniHelper::throwNew(const char* java_class_name, const char* message) {
   LocalRefUniquePtr<jclass> java_class = findClass(java_class_name);
   if (java_class != nullptr) {
     jint error = env_->ThrowNew(java_class.get(), message);
-    ASSERT(error == JNI_OK, fmt::format("Failed calling ThrowNew."));
+    ASSERT(error == JNI_OK, "Failed calling ThrowNew.");
   }
 }
 
+jboolean JniHelper::exceptionCheck() { return env_->ExceptionCheck(); }
+
 LocalRefUniquePtr<jthrowable> JniHelper::exceptionOccurred() {
   return {env_->ExceptionOccurred(), LocalRefDeleter(env_)};
 }
 
+void JniHelper::exceptionCleared() { env_->ExceptionClear(); }
+
 GlobalRefUniquePtr<jobject> JniHelper::newGlobalRef(jobject object) {
   GlobalRefUniquePtr<jobject> result(env_->NewGlobalRef(object), GlobalRefDeleter(env_));
   return result;

mobile/library/jni/jni_helper.h

@@ -233,12 +233,27 @@ class JniHelper {
   void throwNew(const char* java_class_name, const char* message);
 
   /**
-   * Determines if an exception is being thrown.
+   * Returns true if an exception is being thrown; false otherwise.
+   *
+   * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#exceptionoccurred
+   */
+  [[nodiscard]] jboolean exceptionCheck();
+
+  /**
+   * Determines if an exception is being thrown. Returns a `nullptr` if there is no exception.
    *
    * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#exceptionoccurred
    */
   [[nodiscard]] LocalRefUniquePtr<jthrowable> exceptionOccurred();
 
+  /**
+   * Clears any exception that is currently being thrown. If no exception is currently being thrown,
+   * this function has no effect.
+   *
+   * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#exceptionclear
+   */
+  void exceptionCleared();
+
   /**
    * Creates a new global reference to the object referred to by the `object` argument.
    *

mobile/library/jni/jni_impl.cc

@@ -14,7 +14,6 @@
 #include "library/jni/android_network_utility.h"
 #include "library/jni/jni_helper.h"
 #include "library/jni/jni_utility.h"
-#include "library/jni/types/exception.h"
 
 using Envoy::Platform::EngineBuilder;
 
@@ -211,13 +210,18 @@ jvm_on_headers(const char* method, const Envoy::Types::ManagedEnvoyHeaders& head
   Envoy::JNI::LocalRefUniquePtr<jobjectArray> result = jni_helper.callObjectMethod<jobjectArray>(
       j_context, jmid_onHeaders, static_cast<jlong>(headers.get().length),
       end_stream ? JNI_TRUE : JNI_FALSE, j_stream_intel.get());
-  // TODO(Augustyniak): Pass the name of the filter in here so that we can instrument the origin of
-  // the JNI exception better.
-  bool exception_cleared = Envoy::JNI::Exception::checkAndClear(method);
 
-  if (!exception_cleared) {
+  if (!jni_helper.exceptionCheck()) {
     return result;
   }
+  // TODO(fredyw): The whole code here is a bit weird, but this how it currently works. Consider
+  // rewriting it.
+  auto java_exception = jni_helper.exceptionOccurred();
+  jni_helper.exceptionCleared();
+  std::string java_exception_message =
+      Envoy::JNI::getJavaExceptionMessage(jni_helper, java_exception.get());
+  ENVOY_LOG_EVENT_TO_LOGGER(GET_MISC_LOGGER(), info, "jni_cleared_pending_exception", "{}",
+                            absl::StrFormat("%s||%s||", java_exception_message, method));
 
   // Create a "no operation" result:
   //  1. Tell the filter chain to continue the iteration.

mobile/library/jni/jni_utility.cc

@@ -570,5 +570,14 @@ cppBufferInstanceToJavaNonDirectByteBuffer(JniHelper& jni_helper,
                                            java_byte_buffer_wrap_method_id, java_byte_array.get());
 }
 
+std::string getJavaExceptionMessage(JniHelper& jni_helper, jthrowable throwable) {
+  auto java_throwable_class = jni_helper.findClass("java/lang/Throwable");
+  auto java_get_message_method_id =
+      jni_helper.getMethodId(java_throwable_class.get(), "getMessage", "()Ljava/lang/String;");
+  auto java_exception_message =
+      jni_helper.callObjectMethod<jstring>(throwable, java_get_message_method_id);
+  return javaStringToCppString(jni_helper, java_exception_message.get());
+}
+
 } // namespace JNI
 } // namespace Envoy

mobile/library/jni/jni_utility.h

@@ -217,5 +217,8 @@ LocalRefUniquePtr<jobject>
 cppBufferInstanceToJavaNonDirectByteBuffer(JniHelper& jni_helper,
                                            const Buffer::Instance& cpp_buffer_instance);
 
+/** Gets the Java exception message from the `throwable`. */
+std::string getJavaExceptionMessage(JniHelper& jni_helper, jthrowable throwable);
+
 } // namespace JNI
 } // namespace Envoy