jni utility: create utility function from envoy_data to jbyteArray (#…

…1305)

Description: notice that this direction was missing as a utility function.
Risk Level: low - functionality is the same, just dispatched to function.
Testing: builds - successful example app.

Signed-off-by: Jose Nino <[email protected]>

library/common/jni/BUILD

@@ -18,6 +18,7 @@ envoy_cc_library(
     repository = "@envoy",
     deps = [
         "//library/common/types:c_types_lib",
+        "@envoy//source/common/common:assert_lib",
     ],
 )
 

library/common/jni/jni_interface.cc

@@ -145,34 +145,20 @@ static void pass_headers(const char* method, envoy_headers headers, jobject j_co
   jboolean start_headers = JNI_TRUE;
 
   for (envoy_map_size_t i = 0; i < headers.length; i++) {
-    // Note this is just an initial implementation, and we will pass a more optimized structure in
+    // Note: this is just an initial implementation, and we will pass a more optimized structure in
     // the future.
-
-    // Note the JNI function NewStringUTF would appear to be an appealing option here, except it
+    // Note: the JNI function NewStringUTF would appear to be an appealing option here, except it
     // requires a null-terminated *modified* UTF-8 string.
 
     // Create platform byte array for header key
-    jbyteArray key = env->NewByteArray(headers.entries[i].key.length);
-    // TODO: check if copied via isCopy.
-    // TODO: check for NULL.
-    // https://github.com/lyft/envoy-mobile/issues/758
-    void* critical_key = env->GetPrimitiveArrayCritical(key, nullptr);
-    memcpy(critical_key, headers.entries[i].key.bytes, headers.entries[i].key.length);
-    // Here '0' (for which there is no named constant) indicates we want to commit the changes back
-    // to the JVM and free the c array, where applicable.
-    env->ReleasePrimitiveArrayCritical(key, critical_key, 0);
-
+    jbyteArray j_key = native_data_to_array(env, headers.entries[i].key);
     // Create platform byte array for header value
-    jbyteArray value = env->NewByteArray(headers.entries[i].value.length);
-    // TODO: check for NULL.
-    void* critical_value = env->GetPrimitiveArrayCritical(value, nullptr);
-    memcpy(critical_value, headers.entries[i].value.bytes, headers.entries[i].value.length);
-    env->ReleasePrimitiveArrayCritical(value, critical_value, 0);
+    jbyteArray j_value = native_data_to_array(env, headers.entries[i].value);
 
     // Pass this header pair to the platform
-    env->CallVoidMethod(j_context, jmid_passHeader, key, value, start_headers);
-    env->DeleteLocalRef(key);
-    env->DeleteLocalRef(value);
+    env->CallVoidMethod(j_context, jmid_passHeader, j_key, j_value, start_headers);
+    env->DeleteLocalRef(j_key);
+    env->DeleteLocalRef(j_value);
 
     // We don't release local refs currently because we've pushed a large enough frame, but we could
     // consider this and/or periodically popping the frame.
@@ -260,15 +246,7 @@ static void* jvm_on_data(const char* method, envoy_data data, bool end_stream, v
   jmethodID jmid_onData =
       env->GetMethodID(jcls_JvmCallbackContext, method, "([BZ)Ljava/lang/Object;");
 
-  jbyteArray j_data = env->NewByteArray(data.length);
-  // TODO: check if copied via isCopy.
-  // TODO: check for NULL.
-  // https://github.com/lyft/envoy-mobile/issues/758
-  void* critical_data = env->GetPrimitiveArrayCritical(j_data, nullptr);
-  memcpy(critical_data, data.bytes, data.length);
-  // Here '0' (for which there is no named constant) indicates we want to commit the changes back
-  // to the JVM and free the c array, where applicable.
-  env->ReleasePrimitiveArrayCritical(j_data, critical_data, 0);
+  jbyteArray j_data = native_data_to_array(env, data);
   jobject result =
       env->CallObjectMethod(j_context, jmid_onData, j_data, end_stream ? JNI_TRUE : JNI_FALSE);
 
@@ -496,15 +474,7 @@ jvm_http_filter_on_resume(const char* method, envoy_headers* headers, envoy_data
   }
   jbyteArray j_in_data = NULL;
   if (data) {
-    j_in_data = env->NewByteArray(data->length);
-    // TODO: check if copied via isCopy.
-    // TODO: check for NULL.
-    // https://github.com/lyft/envoy-mobile/issues/758
-    void* critical_data = env->GetPrimitiveArrayCritical(j_in_data, nullptr);
-    memcpy(critical_data, data->bytes, data->length);
-    // Here '0' (for which there is no named constant) indicates we want to commit the changes back
-    // to the JVM and free the c array, where applicable.
-    env->ReleasePrimitiveArrayCritical(j_in_data, critical_data, 0);
+    j_in_data = native_data_to_array(env, *data);
   }
   jlong trailers_length = -1;
   if (trailers) {
@@ -572,15 +542,7 @@ static void* call_jvm_on_error(envoy_error error, void* context) {
   jmethodID jmid_onError =
       env->GetMethodID(jcls_JvmObserverContext, "onError", "(I[BI)Ljava/lang/Object;");
 
-  jbyteArray j_error_message = env->NewByteArray(error.message.length);
-  // TODO: check if copied via isCopy.
-  // TODO: check for NULL.
-  // https://github.com/lyft/envoy-mobile/issues/758
-  void* critical_error_message = env->GetPrimitiveArrayCritical(j_error_message, nullptr);
-  memcpy(critical_error_message, error.message.bytes, error.message.length);
-  // Here '0' (for which there is no named constant) indicates we want to commit the changes back
-  // to the JVM and free the c array, where applicable.
-  env->ReleasePrimitiveArrayCritical(j_error_message, critical_error_message, 0);
+  jbyteArray j_error_message = native_data_to_array(env, error.message);
 
   jobject result = env->CallObjectMethod(j_context, jmid_onError, error.error_code, j_error_message,
                                          error.attempt_count);

library/common/jni/jni_utility.cc

@@ -3,6 +3,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "common/common/assert.h"
+
 #include "library/common/jni/jni_version.h"
 
 // NOLINT(namespace-envoy)
@@ -58,6 +60,18 @@ envoy_data array_to_native_data(JNIEnv* env, jbyteArray j_data) {
   return {data_length, native_bytes, free, native_bytes};
 }
 
+jbyteArray native_data_to_array(JNIEnv* env, envoy_data data) {
+  jbyteArray j_data = env->NewByteArray(data.length);
+  void* critical_data = env->GetPrimitiveArrayCritical(j_data, nullptr);
+  RELEASE_ASSERT(critical_data != nullptr, "unable to allocate memory in jni_utility");
+  memcpy(critical_data, data.bytes, data.length);
+  // Here '0' (for which there is no named constant) indicates we want to commit the changes back
+  // to the JVM and free the c array, where applicable.
+  // TODO: potential perf improvement. Check if copied via isCopy, and optimize memory handling.
+  env->ReleasePrimitiveArrayCritical(j_data, critical_data, 0);
+  return j_data;
+}
+
 envoy_data buffer_to_native_data(JNIEnv* env, jobject j_data) {
   uint8_t* direct_address = static_cast<uint8_t*>(env->GetDirectBufferAddress(j_data));
 

library/common/jni/jni_utility.h

@@ -22,6 +22,16 @@ int unbox_integer(JNIEnv* env, jobject boxedInteger);
 
 envoy_data array_to_native_data(JNIEnv* env, jbyteArray j_data);
 
+/**
+ * Utility function that copies envoy_data to jbyteArray.
+ *
+ * @param env, the JNI env pointer.
+ * @param envoy_data, the source to copy from.
+ *
+ * @return jbyteArray, copied data. It is up to the function caller to clean up memory.
+ */
+jbyteArray native_data_to_array(JNIEnv* env, envoy_data data);
+
 envoy_data buffer_to_native_data(JNIEnv* env, jobject j_data);
 
 envoy_data* buffer_to_native_data_ptr(JNIEnv* env, jobject j_data);