From 22369cbecd42850b5fb6c5123c3eaf50c866cc05 Mon Sep 17 00:00:00 2001 From: Fredy Wijaya Date: Wed, 8 Nov 2023 17:27:11 +0000 Subject: [PATCH] mobile: Part 5: Update JNI usages with JniHelper (#30780) This updates the following methods: - `CallObjectMethod` - `CallStaticObjectMethod` This PR adds `[[nodiscard]]` to make sure we always assign the `unique_ptr` into a variable when calling some JNI functions to avoid any leaks. The `callObjectMethod` and `callStaticObjecMethod` from `JniHelper` has been templatized to make them easy to use, especially when casting from `jobject` to a different type. Some usages of `Envoy::JNI::checkAndClear` have also been removed since they are no longer necessary and we don't want to shallow the exceptions in most cases. Signed-off-by: Fredy Wijaya --- .../common/jni/android_network_utility.cc | 42 +++++----- .../common/jni/android_network_utility.h | 7 +- mobile/library/common/jni/jni_helper.cc | 21 ----- mobile/library/common/jni/jni_helper.h | 57 +++++++++---- mobile/library/common/jni/jni_interface.cc | 83 +++++++++---------- mobile/library/common/jni/jni_utility.cc | 53 +++--------- mobile/library/common/jni/jni_utility.h | 21 ----- 7 files changed, 117 insertions(+), 167 deletions(-) diff --git a/mobile/library/common/jni/android_network_utility.cc b/mobile/library/common/jni/android_network_utility.cc index b11983cbac6d..1dd944b87547 100644 --- a/mobile/library/common/jni/android_network_utility.cc +++ b/mobile/library/common/jni/android_network_utility.cc @@ -17,10 +17,8 @@ bool jvm_cert_is_issued_by_known_root(Envoy::JNI::JniHelper& jni_helper, jobject Envoy::JNI::find_class("io.envoyproxy.envoymobile.utilities.AndroidCertVerifyResult"); jmethodID jmid_isIssuedByKnownRoot = jni_helper.getMethodId(jcls_AndroidCertVerifyResult, "isIssuedByKnownRoot", "()Z"); - Envoy::JNI::Exception::checkAndClear("jvm_cert_is_issued_by_known_root:GetMethodID"); ASSERT(jmid_isIssuedByKnownRoot); bool is_issued_by_known_root = jni_helper.callBooleanMethod(result, jmid_isIssuedByKnownRoot); - Envoy::JNI::Exception::checkAndClear("jvm_cert_is_issued_by_known_root:CallBooleanMethod"); jni_helper.getEnv()->DeleteLocalRef(jcls_AndroidCertVerifyResult); return is_issued_by_known_root; } @@ -31,27 +29,23 @@ envoy_cert_verify_status_t jvm_cert_get_status(Envoy::JNI::JniHelper& jni_helper Envoy::JNI::find_class("io.envoyproxy.envoymobile.utilities.AndroidCertVerifyResult"); jmethodID jmid_getStatus = jni_helper.getMethodId(jcls_AndroidCertVerifyResult, "getStatus", "()I"); - Envoy::JNI::Exception::checkAndClear("jvm_cert_get_status:GetMethodID"); ASSERT(jmid_getStatus); envoy_cert_verify_status_t result = CERT_VERIFY_STATUS_FAILED; result = static_cast(jni_helper.callIntMethod(j_result, jmid_getStatus)); - Envoy::JNI::Exception::checkAndClear("jvm_cert_get_status:CallIntMethod"); jni_helper.getEnv()->DeleteLocalRef(jcls_AndroidCertVerifyResult); return result; } -jobjectArray jvm_cert_get_certificate_chain_encoded(Envoy::JNI::JniHelper& jni_helper, - jobject result) { +Envoy::JNI::LocalRefUniquePtr +jvm_cert_get_certificate_chain_encoded(Envoy::JNI::JniHelper& jni_helper, jobject result) { jclass jcls_AndroidCertVerifyResult = Envoy::JNI::find_class("io.envoyproxy.envoymobile.utilities.AndroidCertVerifyResult"); jmethodID jmid_getCertificateChainEncoded = jni_helper.getMethodId(jcls_AndroidCertVerifyResult, "getCertificateChainEncoded", "()[[B"); - Envoy::JNI::Exception::checkAndClear("jvm_cert_get_certificate_chain_encoded:GetMethodID"); - jobjectArray certificate_chain = static_cast( - jni_helper.getEnv()->CallObjectMethod(result, jmid_getCertificateChainEncoded)); - Envoy::JNI::Exception::checkAndClear("jvm_cert_get_certificate_chain_encoded:CallObjectMethod"); + Envoy::JNI::LocalRefUniquePtr certificate_chain = + jni_helper.callObjectMethod(result, jmid_getCertificateChainEncoded); jni_helper.getEnv()->DeleteLocalRef(jcls_AndroidCertVerifyResult); return certificate_chain; } @@ -63,32 +57,33 @@ static void ExtractCertVerifyResult(Envoy::JNI::JniHelper& jni_helper, jobject r *status = jvm_cert_get_status(jni_helper, result); if (*status == CERT_VERIFY_STATUS_OK) { *is_issued_by_known_root = jvm_cert_is_issued_by_known_root(jni_helper, result); - jobjectArray chain_byte_array = jvm_cert_get_certificate_chain_encoded(jni_helper, result); + Envoy::JNI::LocalRefUniquePtr chain_byte_array = + jvm_cert_get_certificate_chain_encoded(jni_helper, result); if (chain_byte_array != nullptr) { - Envoy::JNI::JavaArrayOfByteArrayToStringVector(jni_helper, chain_byte_array, verified_chain); + Envoy::JNI::JavaArrayOfByteArrayToStringVector(jni_helper, chain_byte_array.get(), + verified_chain); } } } // `auth_type` and `host` are expected to be UTF-8 encoded. -jobject call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper, - const std::vector& cert_chain, - std::string auth_type, absl::string_view hostname) { +Envoy::JNI::LocalRefUniquePtr +call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper, + const std::vector& cert_chain, std::string auth_type, + absl::string_view hostname) { jni_log("[Envoy]", "jvm_verify_x509_cert_chain"); jclass jcls_AndroidNetworkLibrary = Envoy::JNI::find_class("io.envoyproxy.envoymobile.utilities.AndroidNetworkLibrary"); jmethodID jmid_verifyServerCertificates = jni_helper.getStaticMethodId( jcls_AndroidNetworkLibrary, "verifyServerCertificates", "([[B[B[B)Lio/envoyproxy/envoymobile/utilities/AndroidCertVerifyResult;"); - Envoy::JNI::Exception::checkAndClear("call_jvm_verify_x509_cert_chain:GetStaticMethodID"); jobjectArray chain_byte_array = Envoy::JNI::ToJavaArrayOfByteArray(jni_helper, cert_chain); jbyteArray auth_string = Envoy::JNI::ToJavaByteArray(jni_helper, auth_type); jbyteArray host_string = Envoy::JNI::ToJavaByteArray( jni_helper, reinterpret_cast(hostname.data()), hostname.length()); - jobject result = jni_helper.getEnv()->CallStaticObjectMethod( - jcls_AndroidNetworkLibrary, jmid_verifyServerCertificates, chain_byte_array, auth_string, - host_string); - Envoy::JNI::Exception::checkAndClear("call_jvm_verify_x509_cert_chain:CallStaticObjectMethod"); + Envoy::JNI::LocalRefUniquePtr result = + jni_helper.callStaticObjectMethod(jcls_AndroidNetworkLibrary, jmid_verifyServerCertificates, + chain_byte_array, auth_string, host_string); jni_helper.getEnv()->DeleteLocalRef(chain_byte_array); jni_helper.getEnv()->DeleteLocalRef(auth_string); jni_helper.getEnv()->DeleteLocalRef(host_string); @@ -103,16 +98,17 @@ static void jvm_verify_x509_cert_chain(const std::vector& cert_chai bool* is_issued_by_known_root, std::vector* verified_chain) { Envoy::JNI::JniHelper jni_helper(Envoy::JNI::get_env()); - jobject result = call_jvm_verify_x509_cert_chain(jni_helper, cert_chain, auth_type, hostname); + Envoy::JNI::LocalRefUniquePtr result = + call_jvm_verify_x509_cert_chain(jni_helper, cert_chain, auth_type, hostname); if (Envoy::JNI::Exception::checkAndClear()) { *status = CERT_VERIFY_STATUS_NOT_YET_VALID; } else { - ExtractCertVerifyResult(jni_helper, result, status, is_issued_by_known_root, verified_chain); + ExtractCertVerifyResult(jni_helper, result.get(), status, is_issued_by_known_root, + verified_chain); if (Envoy::JNI::Exception::checkAndClear()) { *status = CERT_VERIFY_STATUS_FAILED; } } - jni_helper.getEnv()->DeleteLocalRef(result); } envoy_cert_validation_result verify_x509_cert_chain(const std::vector& certs, diff --git a/mobile/library/common/jni/android_network_utility.h b/mobile/library/common/jni/android_network_utility.h index 2ac64bb2e224..654fafd3dad0 100644 --- a/mobile/library/common/jni/android_network_utility.h +++ b/mobile/library/common/jni/android_network_utility.h @@ -13,9 +13,10 @@ /* Calls up through JNI to validate given certificates. */ -jobject call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper, - const std::vector& cert_chain, - std::string auth_type, absl::string_view hostname); +Envoy::JNI::LocalRefUniquePtr +call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper, + const std::vector& cert_chain, std::string auth_type, + absl::string_view hostname); envoy_cert_validation_result verify_x509_cert_chain(const std::vector& certs, absl::string_view hostname); diff --git a/mobile/library/common/jni/jni_helper.cc b/mobile/library/common/jni/jni_helper.cc index f421e4ba6221..a798907210a6 100644 --- a/mobile/library/common/jni/jni_helper.cc +++ b/mobile/library/common/jni/jni_helper.cc @@ -171,16 +171,6 @@ void JniHelper::callVoidMethod(jobject object, jmethodID method_id, ...) { rethrowException(); } -LocalRefUniquePtr JniHelper::callObjectMethod(jobject object, jmethodID method_id, ...) { - va_list args; - va_start(args, method_id); - LocalRefUniquePtr result(env_->CallObjectMethodV(object, method_id, args), - LocalRefDeleter(env_)); - va_end(args); - rethrowException(); - return result; -} - #define DEFINE_CALL_STATIC_METHOD(JAVA_TYPE, JNI_TYPE) \ JNI_TYPE JniHelper::callStatic##JAVA_TYPE##Method(jclass clazz, jmethodID method_id, ...) { \ va_list args; \ @@ -208,17 +198,6 @@ void JniHelper::callStaticVoidMethod(jclass clazz, jmethodID method_id, ...) { rethrowException(); } -LocalRefUniquePtr JniHelper::callStaticObjectMethod(jclass clazz, jmethodID method_id, - ...) { - va_list args; - va_start(args, method_id); - LocalRefUniquePtr result(env_->CallStaticObjectMethodV(clazz, method_id, args), - LocalRefDeleter(env_)); - va_end(args); - rethrowException(); - return result; -} - jlong JniHelper::getDirectBufferCapacity(jobject buffer) { jlong result = env_->GetDirectBufferCapacity(buffer); RELEASE_ASSERT(result != -1, "Failed calling GetDirectBufferCapacity."); diff --git a/mobile/library/common/jni/jni_helper.h b/mobile/library/common/jni/jni_helper.h index 50ed072568ac..49923611349c 100644 --- a/mobile/library/common/jni/jni_helper.h +++ b/mobile/library/common/jni/jni_helper.h @@ -153,14 +153,14 @@ class JniHelper { * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#findclass */ - LocalRefUniquePtr findClass(const char* class_name); + [[nodiscard]] LocalRefUniquePtr findClass(const char* class_name); /** * Returns the class of a given `object`. * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#getobjectclass */ - LocalRefUniquePtr getObjectClass(jobject object); + [[nodiscard]] LocalRefUniquePtr getObjectClass(jobject object); /** * Throws Java exception with the specified class name and error message. @@ -174,31 +174,31 @@ class JniHelper { * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#exceptionoccurred */ - LocalRefUniquePtr exceptionOccurred(); + [[nodiscard]] LocalRefUniquePtr exceptionOccurred(); /** * Creates a new global reference to the object referred to by the `object` argument. * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#newglobalref */ - GlobalRefUniquePtr newGlobalRef(jobject object); + [[nodiscard]] GlobalRefUniquePtr newGlobalRef(jobject object); /** * Creates a new instance of a given `clazz` from the given `method_id`. * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#newobject-newobjecta-newobjectv */ - LocalRefUniquePtr newObject(jclass clazz, jmethodID method_id, ...); + [[nodiscard]] LocalRefUniquePtr newObject(jclass clazz, jmethodID method_id, ...); /** * Creates a new Java string from the given `str`. * * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#newstringutf */ - LocalRefUniquePtr newStringUtf(const char* str); + [[nodiscard]] LocalRefUniquePtr newStringUtf(const char* str); /** Gets the pointer to an array of bytes representing `str`. */ - StringUtfUniquePtr getStringUtfChars(jstring str, jboolean* is_copy); + [[nodiscard]] StringUtfUniquePtr getStringUtfChars(jstring str, jboolean* is_copy); /** * Gets the size of the array. @@ -209,7 +209,7 @@ class JniHelper { /** A macro to create `NewArray`. helper function. */ #define DECLARE_NEW_ARRAY(JAVA_TYPE, JNI_TYPE) \ - LocalRefUniquePtr new##JAVA_TYPE##Array(jsize length); + [[nodiscard]] LocalRefUniquePtr new##JAVA_TYPE##Array(jsize length); /** * Helper functions for `NewArray`. @@ -224,13 +224,13 @@ class JniHelper { DECLARE_NEW_ARRAY(Float, jfloatArray) DECLARE_NEW_ARRAY(Double, jdoubleArray) DECLARE_NEW_ARRAY(Boolean, jbooleanArray) - LocalRefUniquePtr newObjectArray(jsize length, jclass element_class, - jobject initial_element = nullptr); + [[nodiscard]] LocalRefUniquePtr newObjectArray(jsize length, jclass element_class, + jobject initial_element = nullptr); /** A macro to create `GetArrayElement` function. */ #define DECLARE_GET_ARRAY_ELEMENTS(JAVA_TYPE, JNI_ARRAY_TYPE, JNI_ELEMENT_TYPE) \ - ArrayElementsUniquePtr get##JAVA_TYPE##ArrayElements( \ - JNI_ARRAY_TYPE array, jboolean* is_copy); + [[nodiscard]] ArrayElementsUniquePtr \ + get##JAVA_TYPE##ArrayElements(JNI_ARRAY_TYPE array, jboolean* is_copy); /** * Helper functions for `GetArrayElements`. @@ -252,7 +252,7 @@ class JniHelper { * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#getobjectarrayelement */ template - LocalRefUniquePtr getObjectArrayElement(jobjectArray array, jsize index) { + [[nodiscard]] LocalRefUniquePtr getObjectArrayElement(jobjectArray array, jsize index) { LocalRefUniquePtr result(static_cast(env_->GetObjectArrayElement(array, index)), LocalRefDeleter(env_)); rethrowException(); @@ -266,7 +266,8 @@ class JniHelper { */ void setObjectArrayElement(jobjectArray array, jsize index, jobject value); - PrimitiveArrayCriticalUniquePtr getPrimitiveArrayCritical(jarray array, jboolean* is_copy); + [[nodiscard]] PrimitiveArrayCriticalUniquePtr getPrimitiveArrayCritical(jarray array, + jboolean* is_copy); /** * Sets a region of an `array` from a `buffer` with the specified `start` index and `length`. @@ -303,8 +304,19 @@ class JniHelper { DECLARE_CALL_METHOD(Float, jfloat) DECLARE_CALL_METHOD(Double, jdouble) DECLARE_CALL_METHOD(Boolean, jboolean) + void callVoidMethod(jobject object, jmethodID method_id, ...); - LocalRefUniquePtr callObjectMethod(jobject object, jmethodID method_id, ...); + + template + [[nodiscard]] LocalRefUniquePtr callObjectMethod(jobject object, jmethodID method_id, ...) { + va_list args; + va_start(args, method_id); + LocalRefUniquePtr result(static_cast(env_->CallObjectMethodV(object, method_id, args)), + LocalRefDeleter(env_)); + va_end(args); + rethrowException(); + return result; + } /** A macro to create `CallStaticMethod` helper function. */ #define DECLARE_CALL_STATIC_METHOD(JAVA_TYPE, JNI_TYPE) \ @@ -323,8 +335,21 @@ class JniHelper { DECLARE_CALL_STATIC_METHOD(Float, jfloat) DECLARE_CALL_STATIC_METHOD(Double, jdouble) DECLARE_CALL_STATIC_METHOD(Boolean, jboolean) + void callStaticVoidMethod(jclass clazz, jmethodID method_id, ...); - LocalRefUniquePtr callStaticObjectMethod(jclass clazz, jmethodID method_id, ...); + + template + [[nodiscard]] LocalRefUniquePtr callStaticObjectMethod(jclass clazz, jmethodID method_id, + ...) { + va_list args; + va_start(args, method_id); + LocalRefUniquePtr result( + static_cast(env_->CallStaticObjectMethodV(clazz, method_id, args)), + LocalRefDeleter(env_)); + va_end(args); + rethrowException(); + return result; + } /** * Returns the capacity of the memory region referenced by the given `java.nio.Buffer` object. diff --git a/mobile/library/common/jni/jni_interface.cc b/mobile/library/common/jni/jni_interface.cc index 15382289bda9..c8b43ee12ce7 100644 --- a/mobile/library/common/jni/jni_interface.cc +++ b/mobile/library/common/jni/jni_interface.cc @@ -45,7 +45,8 @@ static void jvm_on_engine_running(void* context) { jclass jcls_JvmonEngineRunningContext = jni_helper.getEnv()->GetObjectClass(j_context); jmethodID jmid_onEngineRunning = jni_helper.getMethodId( jcls_JvmonEngineRunningContext, "invokeOnEngineRunning", "()Ljava/lang/Object;"); - Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_onEngineRunning); + Envoy::JNI::LocalRefUniquePtr unused = + jni_helper.callObjectMethod(j_context, jmid_onEngineRunning); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmonEngineRunningContext); // TODO(goaway): This isn't re-used by other engine callbacks, so it's safe to delete here. @@ -365,8 +366,10 @@ static void* jvm_on_data(const char* method, envoy_data data, bool end_stream, jbyteArray j_data = Envoy::JNI::native_data_to_array(jni_helper, data); jlongArray j_stream_intel = Envoy::JNI::native_stream_intel_to_array(jni_helper, stream_intel); - jobject result = Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_onData, j_data, - end_stream ? JNI_TRUE : JNI_FALSE, j_stream_intel); + jobject result = jni_helper + .callObjectMethod(j_context, jmid_onData, j_data, + end_stream ? JNI_TRUE : JNI_FALSE, j_stream_intel) + .release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(j_data); @@ -479,9 +482,10 @@ static void* jvm_on_trailers(const char* method, envoy_headers trailers, jlongArray j_stream_intel = Envoy::JNI::native_stream_intel_to_array(jni_helper, stream_intel); // Note: be careful of JVM types. Before we casted to jlong we were getting integer problems. // TODO: make this cast safer. - // TODO(Augustyniak): check for pending exceptions after returning from JNI call. - jobject result = Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_onTrailers, - (jlong)trailers.length, j_stream_intel); + jobject result = + jni_helper + .callObjectMethod(j_context, jmid_onTrailers, (jlong)trailers.length, j_stream_intel) + .release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmCallbackContext); @@ -657,9 +661,9 @@ jvm_http_filter_on_resume(const char* method, envoy_headers* headers, envoy_data jni_helper.getMethodId(jcls_JvmCallbackContext, method, "(J[BJZ[J)Ljava/lang/Object;"); // Note: be careful of JVM types. Before we casted to jlong we were getting integer problems. // TODO: make this cast safer. - jobjectArray result = static_cast(Envoy::JNI::callObjectMethod( - jni_helper, j_context, jmid_onResume, headers_length, j_in_data, trailers_length, - end_stream ? JNI_TRUE : JNI_FALSE, j_stream_intel)); + Envoy::JNI::LocalRefUniquePtr result = jni_helper.callObjectMethod( + j_context, jmid_onResume, headers_length, j_in_data, trailers_length, + end_stream ? JNI_TRUE : JNI_FALSE, j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmCallbackContext); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); @@ -667,19 +671,19 @@ jvm_http_filter_on_resume(const char* method, envoy_headers* headers, envoy_data jni_helper.getEnv()->DeleteLocalRef(j_in_data); } - jobject status = jni_helper.getEnv()->GetObjectArrayElement(result, 0); + jobject status = jni_helper.getEnv()->GetObjectArrayElement(result.get(), 0); jobjectArray j_headers = - static_cast(jni_helper.getEnv()->GetObjectArrayElement(result, 1)); - jobject j_data = static_cast(jni_helper.getEnv()->GetObjectArrayElement(result, 2)); + static_cast(jni_helper.getEnv()->GetObjectArrayElement(result.get(), 1)); + jobject j_data = + static_cast(jni_helper.getEnv()->GetObjectArrayElement(result.get(), 2)); jobjectArray j_trailers = - static_cast(jni_helper.getEnv()->GetObjectArrayElement(result, 3)); + static_cast(jni_helper.getEnv()->GetObjectArrayElement(result.get(), 3)); int unboxed_status = Envoy::JNI::unbox_integer(jni_helper, status); envoy_headers* pending_headers = Envoy::JNI::to_native_headers_ptr(jni_helper, j_headers); envoy_data* pending_data = Envoy::JNI::buffer_to_native_data_ptr(jni_helper, j_data); envoy_headers* pending_trailers = Envoy::JNI::to_native_headers_ptr(jni_helper, j_trailers); - jni_helper.getEnv()->DeleteLocalRef(result); jni_helper.getEnv()->DeleteLocalRef(status); jni_helper.getEnv()->DeleteLocalRef(j_headers); jni_helper.getEnv()->DeleteLocalRef(j_data); @@ -721,10 +725,9 @@ static void* call_jvm_on_complete(envoy_stream_intel stream_intel, jlongArray j_stream_intel = Envoy::JNI::native_stream_intel_to_array(jni_helper, stream_intel); jlongArray j_final_stream_intel = Envoy::JNI::native_final_stream_intel_to_array(jni_helper, final_stream_intel); - jobject result = jni_helper.getEnv()->CallObjectMethod(j_context, jmid_onComplete, j_stream_intel, - j_final_stream_intel); - - Envoy::JNI::Exception::checkAndClear(); + jobject result = + jni_helper.callObjectMethod(j_context, jmid_onComplete, j_stream_intel, j_final_stream_intel) + .release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(j_final_stream_intel); @@ -747,11 +750,10 @@ static void* call_jvm_on_error(envoy_error error, envoy_stream_intel stream_inte jlongArray j_final_stream_intel = Envoy::JNI::native_final_stream_intel_to_array(jni_helper, final_stream_intel); - jobject result = jni_helper.getEnv()->CallObjectMethod(j_context, jmid_onError, error.error_code, - j_error_message, error.attempt_count, - j_stream_intel, j_final_stream_intel); - - Envoy::JNI::Exception::checkAndClear(); + jobject result = jni_helper + .callObjectMethod(j_context, jmid_onError, error.error_code, j_error_message, + error.attempt_count, j_stream_intel, j_final_stream_intel) + .release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(j_final_stream_intel); @@ -783,10 +785,9 @@ static void* call_jvm_on_cancel(envoy_stream_intel stream_intel, jlongArray j_final_stream_intel = Envoy::JNI::native_final_stream_intel_to_array(jni_helper, final_stream_intel); - jobject result = jni_helper.getEnv()->CallObjectMethod(j_context, jmid_onCancel, j_stream_intel, - j_final_stream_intel); - - Envoy::JNI::Exception::checkAndClear(); + jobject result = + jni_helper.callObjectMethod(j_context, jmid_onCancel, j_stream_intel, j_final_stream_intel) + .release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(j_final_stream_intel); @@ -832,8 +833,8 @@ static void* jvm_on_send_window_available(envoy_stream_intel stream_intel, void* jlongArray j_stream_intel = Envoy::JNI::native_stream_intel_to_array(jni_helper, stream_intel); - jobject result = Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_onSendWindowAvailable, - j_stream_intel); + jobject result = + jni_helper.callObjectMethod(j_context, jmid_onSendWindowAvailable, j_stream_intel).release(); jni_helper.getEnv()->DeleteLocalRef(j_stream_intel); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmObserverContext); @@ -850,11 +851,10 @@ static envoy_data jvm_kv_store_read(envoy_data key, const void* context) { jclass jcls_JvmKeyValueStoreContext = jni_helper.getEnv()->GetObjectClass(j_context); jmethodID jmid_read = jni_helper.getMethodId(jcls_JvmKeyValueStoreContext, "read", "([B)[B"); jbyteArray j_key = Envoy::JNI::native_data_to_array(jni_helper, key); - jbyteArray j_value = - (jbyteArray)Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_read, j_key); - envoy_data native_data = Envoy::JNI::array_to_native_data(jni_helper, j_value); + Envoy::JNI::LocalRefUniquePtr j_value = + jni_helper.callObjectMethod(j_context, jmid_read, j_key); + envoy_data native_data = Envoy::JNI::array_to_native_data(jni_helper, j_value.get()); - jni_helper.getEnv()->DeleteLocalRef(j_value); jni_helper.getEnv()->DeleteLocalRef(j_key); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmKeyValueStoreContext); @@ -910,12 +910,12 @@ static const void* jvm_http_filter_init(const void* context) { jni_helper.getMethodId(jcls_JvmFilterFactoryContext, "create", "()Lio/envoyproxy/envoymobile/engine/JvmFilterContext;"); - jobject j_filter = Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_create); - jni_log_fmt("[Envoy]", "j_filter: %p", j_filter); - jobject retained_filter = jni_helper.getEnv()->NewGlobalRef(j_filter); + Envoy::JNI::LocalRefUniquePtr j_filter = + jni_helper.callObjectMethod(j_context, jmid_create); + jni_log_fmt("[Envoy]", "j_filter: %p", j_filter.get()); + jobject retained_filter = jni_helper.getEnv()->NewGlobalRef(j_filter.get()); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmFilterFactoryContext); - jni_helper.getEnv()->DeleteLocalRef(j_filter); return retained_filter; } @@ -928,12 +928,11 @@ static envoy_data jvm_get_string(const void* context) { jclass jcls_JvmStringAccessorContext = jni_helper.getEnv()->GetObjectClass(j_context); jmethodID jmid_getString = jni_helper.getMethodId(jcls_JvmStringAccessorContext, "getEnvoyString", "()[B"); - jbyteArray j_data = - (jbyteArray)Envoy::JNI::callObjectMethod(jni_helper, j_context, jmid_getString); - envoy_data native_data = Envoy::JNI::array_to_native_data(jni_helper, j_data); + Envoy::JNI::LocalRefUniquePtr j_data = + jni_helper.callObjectMethod(j_context, jmid_getString); + envoy_data native_data = Envoy::JNI::array_to_native_data(jni_helper, j_data.get()); jni_helper.getEnv()->DeleteLocalRef(jcls_JvmStringAccessorContext); - jni_helper.getEnv()->DeleteLocalRef(j_data); return native_data; } @@ -1482,7 +1481,7 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_callCertificateVerificationFrom Envoy::JNI::JavaArrayOfByteToString(jni_helper, jauthType, &auth_type); Envoy::JNI::JavaArrayOfByteToString(jni_helper, jhost, &host); - return call_jvm_verify_x509_cert_chain(jni_helper, cert_chain, auth_type, host); + return call_jvm_verify_x509_cert_chain(jni_helper, cert_chain, auth_type, host).release(); } extern "C" JNIEXPORT void JNICALL diff --git a/mobile/library/common/jni/jni_utility.cc b/mobile/library/common/jni/jni_utility.cc index ccbf6dee09c2..4592467e46f6 100644 --- a/mobile/library/common/jni/jni_utility.cc +++ b/mobile/library/common/jni/jni_utility.cc @@ -27,11 +27,11 @@ jclass find_class(const char* class_name) { LocalRefUniquePtr class_loader = jni_helper.findClass("java/lang/ClassLoader"); jmethodID find_class_method = jni_helper.getMethodId(class_loader.get(), "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); - Envoy::JNI::Exception::checkAndClear("find_class:GetMethodID"); LocalRefUniquePtr str_class_name = jni_helper.newStringUtf(class_name); - jclass clazz = (jclass)(jni_helper.getEnv()->CallObjectMethod( - get_class_loader(), find_class_method, str_class_name.get())); - Envoy::JNI::Exception::checkAndClear("find_class:CallObjectMethod"); + jclass clazz = + jni_helper + .callObjectMethod(get_class_loader(), find_class_method, str_class_name.get()) + .release(); return clazz; } @@ -139,7 +139,8 @@ jobject native_map_to_map(JniHelper& jni_helper, envoy_map map) { for (envoy_map_size_t i = 0; i < map.length; i++) { LocalRefUniquePtr key = native_data_to_string(jni_helper, map.entries[i].key); LocalRefUniquePtr value = native_data_to_string(jni_helper, map.entries[i].value); - callObjectMethod(jni_helper, j_hashMap, jmid_hashMapPut, key.get(), value.get()); + LocalRefUniquePtr ignored = + jni_helper.callObjectMethod(j_hashMap, jmid_hashMapPut, key.get(), value.get()); } return j_hashMap; } @@ -154,10 +155,9 @@ envoy_data buffer_to_native_data(JniHelper& jni_helper, jobject j_data) { // are supported. We will crash here if this is an invalid buffer, but guards may be // implemented in the JVM layer. jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer.get(), "array", "()[B"); - jbyteArray array = static_cast(callObjectMethod(jni_helper, j_data, jmid_array)); - - envoy_data native_data = array_to_native_data(jni_helper, array); - jni_helper.getEnv()->DeleteLocalRef(array); + LocalRefUniquePtr array = + jni_helper.callObjectMethod(j_data, jmid_array); + envoy_data native_data = array_to_native_data(jni_helper, array.get()); return native_data; } @@ -175,10 +175,9 @@ envoy_data buffer_to_native_data(JniHelper& jni_helper, jobject j_data, size_t d // are supported. We will crash here if this is an invalid buffer, but guards may be // implemented in the JVM layer. jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer.get(), "array", "()[B"); - jbyteArray array = static_cast(callObjectMethod(jni_helper, j_data, jmid_array)); - - envoy_data native_data = array_to_native_data(jni_helper, array, data_length); - jni_helper.getEnv()->DeleteLocalRef(array); + LocalRefUniquePtr array = + jni_helper.callObjectMethod(j_data, jmid_array); + envoy_data native_data = array_to_native_data(jni_helper, array.get(), data_length); return native_data; } @@ -393,33 +392,5 @@ void javaByteArrayToProto(JniHelper& jni_helper, jbyteArray source, RELEASE_ASSERT(success, "Failed to parse protobuf message."); } -#define JNI_UTILITY_DEFINE_CALL_METHOD(JAVA_TYPE, JNI_TYPE) \ - JNI_TYPE call##JAVA_TYPE##Method(JniHelper& jni_helper, jobject object, jmethodID method_id, \ - ...) { \ - va_list args; \ - va_start(args, method_id); \ - JNI_TYPE result = jni_helper.getEnv()->Call##JAVA_TYPE##MethodV(object, method_id, args); \ - va_end(args); \ - Envoy::JNI::Exception::checkAndClear(); \ - return result; \ - } - -// TODO(fredyw): Delete these functions are replaced them with the ones from JniHelper - -JNI_UTILITY_DEFINE_CALL_METHOD(Object, jobject) - -#define JNI_UTILITY_DEFINE_CALL_STATIC_METHOD(JAVA_TYPE, JNI_TYPE) \ - JNI_TYPE callStatic##JAVA_TYPE##Method(JniHelper& jni_helper, jclass clazz, jmethodID method_id, \ - ...) { \ - va_list args; \ - va_start(args, method_id); \ - JNI_TYPE result = jni_helper.getEnv()->CallStatic##JAVA_TYPE##MethodV(clazz, method_id, args); \ - va_end(args); \ - Envoy::JNI::Exception::checkAndClear(); \ - return result; \ - } - -JNI_UTILITY_DEFINE_CALL_STATIC_METHOD(Object, jobject) - } // namespace JNI } // namespace Envoy diff --git a/mobile/library/common/jni/jni_utility.h b/mobile/library/common/jni/jni_utility.h index b60f455e32f8..0e11bfbe751d 100644 --- a/mobile/library/common/jni/jni_utility.h +++ b/mobile/library/common/jni/jni_utility.h @@ -120,26 +120,5 @@ std::vector javaObjectArrayToMatcherData(JniHelper& jni_helper, job void javaByteArrayToProto(JniHelper& jni_helper, jbyteArray source, Envoy::Protobuf::MessageLite* dest); -// TODO(fredyw): Delete these functions are replaced them with the ones from JniHelper - -// Helper functions for JNI's `CallMethod` with proper exception handling in order to satisfy -// -Xcheck:jni. -// See -// https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#calling-instance-methods -#define JNI_UTILITY_DECLARE_CALL_METHOD(JAVA_TYPE, JNI_TYPE) \ - JNI_TYPE call##JAVA_TYPE##Method(JniHelper& jni_helper, jobject object, jmethodID method_id, ...); - -JNI_UTILITY_DECLARE_CALL_METHOD(Object, jobject) - -// Helper functions for JNI's `CallStaticMethod` with proper exception handling in order to -// satisfy -Xcheck:jni. -// See -// https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#calling-static-methods -#define JNI_UTILITY_DECLARE_CALL_STATIC_METHOD(JAVA_TYPE, JNI_TYPE) \ - JNI_TYPE callStatic##JAVA_TYPE##Method(JniHelper& jni_helper, jclass clazz, jmethodID method_id, \ - ...); - -JNI_UTILITY_DECLARE_CALL_STATIC_METHOD(Object, jobject) - } // namespace JNI } // namespace Envoy