mobile: Part 5: Update JNI usages with JniHelper (envoyproxy#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 <[email protected]>

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<envoy_cert_verify_status_t>(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<jobjectArray>
+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<jobjectArray>(
-      jni_helper.getEnv()->CallObjectMethod(result, jmid_getCertificateChainEncoded));
-  Envoy::JNI::Exception::checkAndClear("jvm_cert_get_certificate_chain_encoded:CallObjectMethod");
+  Envoy::JNI::LocalRefUniquePtr<jobjectArray> certificate_chain =
+      jni_helper.callObjectMethod<jobjectArray>(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<jobjectArray> 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<std::string>& cert_chain,
-                                        std::string auth_type, absl::string_view hostname) {
+Envoy::JNI::LocalRefUniquePtr<jobject>
+call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper,
+                                const std::vector<std::string>& 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<const uint8_t*>(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<jobject> 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<std::string>& cert_chai
                                        bool* is_issued_by_known_root,
                                        std::vector<std::string>* 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<jobject> 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<std::string>& certs,

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<std::string>& cert_chain,
-                                        std::string auth_type, absl::string_view hostname);
+Envoy::JNI::LocalRefUniquePtr<jobject>
+call_jvm_verify_x509_cert_chain(Envoy::JNI::JniHelper& jni_helper,
+                                const std::vector<std::string>& cert_chain, std::string auth_type,
+                                absl::string_view hostname);
 
 envoy_cert_validation_result verify_x509_cert_chain(const std::vector<std::string>& certs,
                                                     absl::string_view hostname);

mobile/library/common/jni/jni_helper.cc

@@ -171,16 +171,6 @@ void JniHelper::callVoidMethod(jobject object, jmethodID method_id, ...) {
   rethrowException();
 }
 
-LocalRefUniquePtr<jobject> JniHelper::callObjectMethod(jobject object, jmethodID method_id, ...) {
-  va_list args;
-  va_start(args, method_id);
-  LocalRefUniquePtr<jobject> 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<jobject> JniHelper::callStaticObjectMethod(jclass clazz, jmethodID method_id,
-                                                             ...) {
-  va_list args;
-  va_start(args, method_id);
-  LocalRefUniquePtr<jobject> 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.");

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<jclass> findClass(const char* class_name);
+  [[nodiscard]] LocalRefUniquePtr<jclass> 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<jclass> getObjectClass(jobject object);
+  [[nodiscard]] LocalRefUniquePtr<jclass> 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<jthrowable> exceptionOccurred();
+  [[nodiscard]] LocalRefUniquePtr<jthrowable> 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<jobject> newGlobalRef(jobject object);
+  [[nodiscard]] GlobalRefUniquePtr<jobject> 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<jobject> newObject(jclass clazz, jmethodID method_id, ...);
+  [[nodiscard]] LocalRefUniquePtr<jobject> 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<jstring> newStringUtf(const char* str);
+  [[nodiscard]] LocalRefUniquePtr<jstring> 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 `New<Type>Array`. helper function. */
 #define DECLARE_NEW_ARRAY(JAVA_TYPE, JNI_TYPE)                                                     \
-  LocalRefUniquePtr<JNI_TYPE> new##JAVA_TYPE##Array(jsize length);
+  [[nodiscard]] LocalRefUniquePtr<JNI_TYPE> new##JAVA_TYPE##Array(jsize length);
 
   /**
    * Helper functions for `New<Type>Array`.
@@ -224,13 +224,13 @@ class JniHelper {
   DECLARE_NEW_ARRAY(Float, jfloatArray)
   DECLARE_NEW_ARRAY(Double, jdoubleArray)
   DECLARE_NEW_ARRAY(Boolean, jbooleanArray)
-  LocalRefUniquePtr<jobjectArray> newObjectArray(jsize length, jclass element_class,
-                                                 jobject initial_element = nullptr);
+  [[nodiscard]] LocalRefUniquePtr<jobjectArray> newObjectArray(jsize length, jclass element_class,
+                                                               jobject initial_element = nullptr);
 
 /** A macro to create `Get<JavaType>ArrayElement` function. */
 #define DECLARE_GET_ARRAY_ELEMENTS(JAVA_TYPE, JNI_ARRAY_TYPE, JNI_ELEMENT_TYPE)                    \
-  ArrayElementsUniquePtr<JNI_ARRAY_TYPE, JNI_ELEMENT_TYPE> get##JAVA_TYPE##ArrayElements(          \
-      JNI_ARRAY_TYPE array, jboolean* is_copy);
+  [[nodiscard]] ArrayElementsUniquePtr<JNI_ARRAY_TYPE, JNI_ELEMENT_TYPE>                           \
+      get##JAVA_TYPE##ArrayElements(JNI_ARRAY_TYPE array, jboolean* is_copy);
 
   /**
    * Helper functions for `Get<JavaType>ArrayElements`.
@@ -252,7 +252,7 @@ class JniHelper {
    * https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#getobjectarrayelement
    */
   template <typename T = jobject>
-  LocalRefUniquePtr<T> getObjectArrayElement(jobjectArray array, jsize index) {
+  [[nodiscard]] LocalRefUniquePtr<T> getObjectArrayElement(jobjectArray array, jsize index) {
     LocalRefUniquePtr<T> result(static_cast<T>(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<jobject> callObjectMethod(jobject object, jmethodID method_id, ...);
+
+  template <typename T = jobject>
+  [[nodiscard]] LocalRefUniquePtr<T> callObjectMethod(jobject object, jmethodID method_id, ...) {
+    va_list args;
+    va_start(args, method_id);
+    LocalRefUniquePtr<T> result(static_cast<T>(env_->CallObjectMethodV(object, method_id, args)),
+                                LocalRefDeleter(env_));
+    va_end(args);
+    rethrowException();
+    return result;
+  }
 
 /** A macro to create `CallStatic<Type>Method` 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<jobject> callStaticObjectMethod(jclass clazz, jmethodID method_id, ...);
+
+  template <typename T = jobject>
+  [[nodiscard]] LocalRefUniquePtr<T> callStaticObjectMethod(jclass clazz, jmethodID method_id,
+                                                            ...) {
+    va_list args;
+    va_start(args, method_id);
+    LocalRefUniquePtr<T> result(
+        static_cast<T>(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.