diff --git a/mobile/library/jni/BUILD b/mobile/library/jni/BUILD index 74d416c6638af..c9e7bb72825c8 100644 --- a/mobile/library/jni/BUILD +++ b/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", ], ) diff --git a/mobile/library/jni/android_network_utility.cc b/mobile/library/jni/android_network_utility.cc index 5732fa1535f0d..c4c1ca172f06d 100644 --- a/mobile/library/jni/android_network_utility.cc +++ b/mobile/library/jni/android_network_utility.cc @@ -88,13 +88,15 @@ static void jvmVerifyX509CertChain(const std::vector& cert_chain, JniHelper jni_helper(JniHelper::getThreadLocalEnv()); LocalRefUniquePtr result = callJvmVerifyX509CertChain(jni_helper, cert_chain, auth_type, hostname); - if (Exception::checkAndClear()) { + if (jni_helper.exceptionOccurred() != nullptr) { *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.exceptionOccurred() != nullptr) { *status = CertVerifyStatus::Failed; + jni_helper.exceptionCleared(); } } } diff --git a/mobile/library/jni/jni_helper.cc b/mobile/library/jni/jni_helper.cc index 41dc4dbed9824..63be8c5cb549a 100644 --- a/mobile/library/jni/jni_helper.cc +++ b/mobile/library/jni/jni_helper.cc @@ -100,6 +100,8 @@ LocalRefUniquePtr JniHelper::exceptionOccurred() { return {env_->ExceptionOccurred(), LocalRefDeleter(env_)}; } +void JniHelper::exceptionCleared() { env_->ExceptionClear(); } + GlobalRefUniquePtr JniHelper::newGlobalRef(jobject object) { GlobalRefUniquePtr result(env_->NewGlobalRef(object), GlobalRefDeleter(env_)); return result; diff --git a/mobile/library/jni/jni_helper.h b/mobile/library/jni/jni_helper.h index 3dd936e1571cf..bb6a5fda11c06 100644 --- a/mobile/library/jni/jni_helper.h +++ b/mobile/library/jni/jni_helper.h @@ -233,12 +233,20 @@ class JniHelper { void throwNew(const char* java_class_name, const char* message); /** - * Determines if an exception is being thrown. + * 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 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. * diff --git a/mobile/library/jni/jni_impl.cc b/mobile/library/jni/jni_impl.cc index 2ef56cff14fc8..9e70e2ea12846 100644 --- a/mobile/library/jni/jni_impl.cc +++ b/mobile/library/jni/jni_impl.cc @@ -211,11 +211,9 @@ jvm_on_headers(const char* method, const Envoy::Types::ManagedEnvoyHeaders& head Envoy::JNI::LocalRefUniquePtr result = jni_helper.callObjectMethod( j_context, jmid_onHeaders, static_cast(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.exceptionOccurred() != nullptr) { + jni_helper.exceptionCleared(); return result; } diff --git a/mobile/library/jni/types/BUILD b/mobile/library/jni/types/BUILD deleted file mode 100644 index f98656bc7a096..0000000000000 --- a/mobile/library/jni/types/BUILD +++ /dev/null @@ -1,36 +0,0 @@ -load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_library", "envoy_mobile_package") - -licenses(["notice"]) # Apache 2 - -envoy_mobile_package() - -envoy_cc_library( - name = "jni_exception_lib", - srcs = [ - "exception.cc", - ], - hdrs = [ - "exception.h", - ], - repository = "@envoy", - deps = [ - ":jni_string_lib", - "//library/jni:jni_helper_lib", - "@envoy//source/common/common:assert_lib", - ], - # We need this to ensure that we link this into the .so even though there are no code references. - alwayslink = True, -) - -envoy_cc_library( - name = "jni_string_lib", - hdrs = [ - "string.h", - ], - repository = "@envoy", - deps = [ - "//library/jni:jni_helper_lib", - ], - # We need this to ensure that we link this into the .so even though there are no code references. - alwayslink = True, -) diff --git a/mobile/library/jni/types/exception.cc b/mobile/library/jni/types/exception.cc deleted file mode 100644 index f06eed9ca0f4f..0000000000000 --- a/mobile/library/jni/types/exception.cc +++ /dev/null @@ -1,155 +0,0 @@ -#include "library/jni/types/exception.h" - -#include "source/common/common/assert.h" - -#include "fmt/format.h" -#include "library/jni/types/string.h" - -namespace Envoy { -namespace JNI { - -bool Exception::checkAndClear(const std::string& details) { - auto env = JniHelper::getThreadLocalEnv(); - if (env->ExceptionCheck() == JNI_TRUE) { - jthrowable throwable = env->ExceptionOccurred(); - env->ExceptionClear(); - - const auto exception = Exception(env, throwable); - ENVOY_LOG_EVENT_TO_LOGGER(GET_MISC_LOGGER(), info, "jni_cleared_pending_exception", "{}", - exception.description(details)); - return true; - } else { - return false; - } -} - -/** - * @brief Creates a description of an exception in the following format: - * GENERIC_EXCEPTION_DESCRIPTION||DETAIL||EXCEPTION_STACKTRACE||EXCEPTION_CAUSE_STACKTRACE - */ -std::string Exception::description(const std::string& detail) const { - auto throwable_cause_description = causedByThrowableDescription(); - std::vector descriptionComponents = { - throwableDescription(throwable_), - detail == "" ? "NO_DETAIL" : detail, - throwableStacktraceDescription(throwable_), - throwable_cause_description == "" ? "CAUSE_UNKNOWN_OR_NONEXISTENT" - : throwable_cause_description, - }; - - return fmt::format("{}", fmt::join(descriptionComponents, "||")); -} - -std::string Exception::throwableDescription(jthrowable throwable) const { - jclass jcls_throwable = env_->FindClass("java/lang/Throwable"); - if (exceptionCheck()) { - return "Throwable: class not found"; - } - - jmethodID mid_throwable_toString = - env_->GetMethodID(jcls_throwable, "toString", "()Ljava/lang/String;"); - if (exceptionCheck()) { - return "Throwable.toString: method not found"; - } - - jstring j_description = - static_cast(env_->CallObjectMethod(throwable, mid_throwable_toString)); - if (exceptionCheck()) { - return "Throwable.toString: exception was thrown during method call"; - } - - return String(j_description).get(); -} - -std::string Exception::throwableStacktraceDescription(jthrowable throwable) const { - jclass jcls_throwable = env_->FindClass("java/lang/Throwable"); - if (exceptionCheck()) { - return "Throwable: class not found"; - } - - jmethodID mid_throwable_getStackTrace = - env_->GetMethodID(jcls_throwable, "getStackTrace", "()[Ljava/lang/StackTraceElement;"); - if (exceptionCheck()) { - return "Throwable.getStackTrace: method not found"; - } - - jclass jcls_frame = env_->FindClass("java/lang/StackTraceElement"); - if (exceptionCheck()) { - return "StackTraceElement: class not found"; - } - - jmethodID mid_frame_toString = env_->GetMethodID(jcls_frame, "toString", "()Ljava/lang/String;"); - if (exceptionCheck()) { - return "StackTraceElement.toString: method not found"; - } - - jobjectArray j_frames = - static_cast(env_->CallObjectMethod(throwable, mid_throwable_getStackTrace)); - if (exceptionCheck()) { - return "StackTraceElement.getStrackTrace: exception was thrown during method call"; - } - - jsize frames_length = env_->GetArrayLength(j_frames); - if (exceptionCheck()) { - return "GetArrayLength: exception was thrown during method call"; - } - - std::vector lines; - - jsize i = 0; - for (i = 0; i < frames_length; i++) { - jobject j_frame = env_->GetObjectArrayElement(j_frames, i); - if (exceptionCheck()) { - lines.push_back("GetObjectArrayElement: exception was thrown during method call"); - break; - } - - jstring j_description = - static_cast(env_->CallObjectMethod(j_frame, mid_frame_toString)); - if (exceptionCheck()) { - lines.push_back("StackTraceElement.toString: exception was thrown during method call"); - break; - } - - lines.push_back(String(j_description).get()); - env_->DeleteLocalRef(j_frame); - } - - return fmt::format("{}", fmt::join(lines, ";")); -} - -std::string Exception::causedByThrowableDescription() const { - jclass jcls_throwable = env_->FindClass("java/lang/Throwable"); - if (exceptionCheck()) { - return "Throwable: class not found"; - } - - jmethodID mid_throwable_getCause = - env_->GetMethodID(jcls_throwable, "getCause", "()Ljava/lang/Throwable;"); - if (exceptionCheck()) { - return "Throwable.getCause: method not found"; - } - - jthrowable j_throwable = - static_cast(env_->CallObjectMethod(throwable_, mid_throwable_getCause)); - if (exceptionCheck()) { - return "Throwable.getCause: exception was thrown during method call"; - } - - if (j_throwable == nullptr) { - return ""; - } - - return throwableDescription(j_throwable); -} - -bool Exception::exceptionCheck() const { - if (env_->ExceptionCheck() == JNI_TRUE) { - env_->ExceptionClear(); - return true; - } - return false; -} - -} // namespace JNI -} // namespace Envoy diff --git a/mobile/library/jni/types/exception.h b/mobile/library/jni/types/exception.h deleted file mode 100644 index 16b1837257904..0000000000000 --- a/mobile/library/jni/types/exception.h +++ /dev/null @@ -1,42 +0,0 @@ -#pragma once - -#include - -#include "library/jni/jni_helper.h" - -namespace Envoy { -namespace JNI { - -/** - * @brief A convenience wrapper for checking for and reporting JNI exceptions. - */ -class Exception { -public: - /** - * @brief Checks and clears any pending exceptions. Reports pending exceptions using - * ENVOY_LOG_EVENT_TO_LOGGER macro. The macro ends up propagating the information to - * platform layer using platform's logger and event tracker APIs. The corresponding log/event - * uses emits `jni_cleared_pending_exception` log. - * - * @param detail Information that will be attached to a pending exception log if any is emitted. - * @return true If a pending exception was present and cleared. - * @return false If there was no pending exception. - */ - static bool checkAndClear(const std::string& detail = ""); - -private: - Exception(JNIEnv* env, jthrowable throwable) : env_(env), throwable_(throwable) {} - - std::string description(const std::string& detail) const; - std::string throwableDescription(jthrowable) const; - std::string throwableStacktraceDescription(jthrowable) const; - std::string causedByThrowableDescription() const; - - bool exceptionCheck() const; - - JNIEnv* env_; - jthrowable throwable_; -}; - -} // namespace JNI -} // namespace Envoy diff --git a/mobile/library/jni/types/string.h b/mobile/library/jni/types/string.h deleted file mode 100644 index 1ef98a61c07fb..0000000000000 --- a/mobile/library/jni/types/string.h +++ /dev/null @@ -1,48 +0,0 @@ -#pragma once - -#include "library/jni/jni_helper.h" - -namespace Envoy { -namespace JNI { - -/** - * @brief A convenience wrapper that makes working with jstring easier and less error prone. - * It takes care of managing freeing JNI resources when it's deallocated. - */ -class String { -public: - String(const String&) = delete; - - /** - * @brief Construct a new String object. Retrieves a pointer to an array of bytes representing - * the string in modified UTF-8 encoding. - */ - explicit String(jstring jni_string) - : env_(JniHelper::getThreadLocalEnv()), jni_string_(jni_string), - string_(env_->GetStringUTFChars(jni_string, nullptr)) {} - - ~String() { - if (string_ != nullptr) { - env_->ReleaseStringUTFChars(jni_string_, string_); - } - } - - /** - * @brief Returns a string that represents the underlying array of bytes. - */ - std::string get() { - if (string_ != nullptr) { - return std::string(string_); - } - return "nullptr"; - } - void operator=(const String&) = delete; - -private: - JNIEnv* env_; - jstring jni_string_; - const char* string_; -}; - -} // namespace JNI -} // namespace Envoy diff --git a/mobile/test/java/io/envoyproxy/envoymobile/jni/JniHelperTest.java b/mobile/test/java/io/envoyproxy/envoymobile/jni/JniHelperTest.java index 8dea8a7ee6737..56abdc6f0c9fb 100644 --- a/mobile/test/java/io/envoyproxy/envoymobile/jni/JniHelperTest.java +++ b/mobile/test/java/io/envoyproxy/envoymobile/jni/JniHelperTest.java @@ -42,6 +42,8 @@ public static native Object getObjectField(Class clazz, Object instance, Stri public static native Class getObjectClass(Object object); public static native Object newObject(Class clazz, String name, String signature); public static native void throwNew(String className, String message); + public static native boolean exceptionOccurred(Class clazz, String name, String signature); + public static native boolean exceptionClear(Class clazz, String name, String signature); public static native int getArrayLength(int[] array); public static native byte[] newByteArray(int length); public static native char[] newCharArray(int length); @@ -149,6 +151,11 @@ public void voidMethod() {} public static void staticVoidMethod() {} public static String staticObjectMethod() { return "Hello"; } + //================================================================================ + // Methods used for Exception* tests. + //================================================================================ + public static void alwaysThrow() { throw new RuntimeException("Test"); } + static class Foo { private final int field = 1; } @Test @@ -235,6 +242,18 @@ public void testThrowNew() { assertThat(exception).hasMessageThat().contains("Test"); } + @Test + public void testExceptionOccurred() { + RuntimeException exception = assertThrows( + RuntimeException.class, () -> exceptionOccurred(JniHelperTest.class, "alwaysThrow", "()V")); + assertThat(exception).hasMessageThat().contains("Test"); + } + + @Test + public void testExceptionClear() { + assertThat(exceptionClear(JniHelperTest.class, "alwaysThrow", "()V")).isTrue(); + } + @Test public void testGetArrayLength() { assertThat(getArrayLength(new int[] {1, 2, 3})).isEqualTo(3); diff --git a/mobile/test/jni/jni_helper_test.cc b/mobile/test/jni/jni_helper_test.cc index d93882babb764..6498c7a25735e 100644 --- a/mobile/test/jni/jni_helper_test.cc +++ b/mobile/test/jni/jni_helper_test.cc @@ -1,5 +1,7 @@ #include +#include + #include "library/jni/jni_helper.h" // NOLINT(namespace-envoy) @@ -91,6 +93,33 @@ extern "C" JNIEXPORT void JNICALL Java_io_envoyproxy_envoymobile_jni_JniHelperTe jni_helper.throwNew(class_name_ptr.get(), message_ptr.get()); } +extern "C" JNIEXPORT jboolean JNICALL +Java_io_envoyproxy_envoymobile_jni_JniHelperTest_exceptionOccurred(JNIEnv* env, jclass, + jclass clazz, jstring name, + jstring signature) { + Envoy::JNI::JniHelper jni_helper(env); + Envoy::JNI::StringUtfUniquePtr name_ptr = jni_helper.getStringUtfChars(name, nullptr); + Envoy::JNI::StringUtfUniquePtr sig_ptr = jni_helper.getStringUtfChars(signature, nullptr); + jmethodID method_id = jni_helper.getStaticMethodId(clazz, name_ptr.get(), sig_ptr.get()); + jni_helper.callStaticVoidMethod(clazz, method_id); + return jni_helper.exceptionOccurred() != nullptr; +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_io_envoyproxy_envoymobile_jni_JniHelperTest_exceptionClear(JNIEnv* env, jclass, jclass clazz, + jstring name, jstring signature) { + Envoy::JNI::JniHelper jni_helper(env); + Envoy::JNI::StringUtfUniquePtr name_ptr = jni_helper.getStringUtfChars(name, nullptr); + Envoy::JNI::StringUtfUniquePtr sig_ptr = jni_helper.getStringUtfChars(signature, nullptr); + jmethodID method_id = jni_helper.getStaticMethodId(clazz, name_ptr.get(), sig_ptr.get()); + jni_helper.callStaticVoidMethod(clazz, method_id); + if (jni_helper.exceptionOccurred() != nullptr) { + jni_helper.exceptionCleared(); + return true; + } + return false; +} + extern "C" JNIEXPORT jobject JNICALL Java_io_envoyproxy_envoymobile_jni_JniHelperTest_newObject( JNIEnv* env, jclass, jclass clazz, jstring name, jstring signature) { Envoy::JNI::JniHelper jni_helper(env);