From ac8eb111a955bdc0a37664e2a8d28c8b5701e130 Mon Sep 17 00:00:00 2001
From: Sidharth Guglani <sidharthguglani@fb.com>
Date: Mon, 2 Dec 2019 05:21:56 -0800
Subject: [PATCH] Add defensive check for result returned after JNI function
 calls

Summary:
Added method which checks if value(methodId, fieldId, class ...) returned by JNI functions (getMethod, getField, getClass ...) is valid or not and throw exception if they are not valid

##Changelog:
[Internal][Yoga] Add defensive check for result returned after JNI calls

Reviewed By: astreet

Differential Revision: D18745718

fbshipit-source-id: 2af26eda15fbe7834e1c9b274deeed4f106274ab
---
 java/jni/common.cpp        | 14 +++++++-------
 java/jni/corefunctions.cpp | 13 +++++++++++++
 java/jni/corefunctions.h   |  2 ++
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/java/jni/common.cpp b/java/jni/common.cpp
index f69ef94f..0cc37333 100644
--- a/java/jni/common.cpp
+++ b/java/jni/common.cpp
@@ -18,11 +18,11 @@ void registerNatives(
     size_t numMethods) {
   jclass clazz = env->FindClass(className);
 
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, !clazz);
 
-  env->RegisterNatives(clazz, methods, numMethods);
+  auto result = env->RegisterNatives(clazz, methods, numMethods);
 
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, result != JNI_OK);
 }
 
 jmethodID getStaticMethodId(
@@ -32,7 +32,7 @@ jmethodID getStaticMethodId(
     const char* methodDescriptor) {
   jmethodID methodId =
       env->GetStaticMethodID(clazz, methodName, methodDescriptor);
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, !methodId);
   return methodId;
 }
 
@@ -42,7 +42,7 @@ jmethodID getMethodId(
     const char* methodName,
     const char* methodDescriptor) {
   jmethodID methodId = env->GetMethodID(clazz, methodName, methodDescriptor);
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, !methodId);
   return methodId;
 }
 
@@ -52,7 +52,7 @@ jfieldID getFieldId(
     const char* fieldName,
     const char* fieldSignature) {
   jfieldID fieldId = env->GetFieldID(clazz, fieldName, fieldSignature);
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, !fieldId);
   return fieldId;
 }
 
@@ -83,7 +83,7 @@ callStaticObjectMethod(JNIEnv* env, jclass clazz, jmethodID methodId, ...) {
   va_start(args, methodId);
   jobject result = env->CallStaticObjectMethodV(clazz, methodId, args);
   va_end(args);
-  assertNoPendingJniException(env);
+  assertNoPendingJniExceptionIf(env, !result);
   return make_local_ref(env, result);
 }
 
diff --git a/java/jni/corefunctions.cpp b/java/jni/corefunctions.cpp
index 4b03919c..8d522289 100644
--- a/java/jni/corefunctions.cpp
+++ b/java/jni/corefunctions.cpp
@@ -76,6 +76,19 @@ void assertNoPendingJniException(JNIEnv* env) {
   throw YogaJniException(throwable);
 }
 
+void assertNoPendingJniExceptionIf(JNIEnv* env, bool condition) {
+  if (!condition) {
+    return;
+  }
+
+  if (env->ExceptionCheck() == JNI_TRUE) {
+    assertNoPendingJniException(env);
+    return;
+  }
+
+  throw YogaJniException();
+}
+
 } // namespace vanillajni
 } // namespace yoga
 } // namespace facebook
diff --git a/java/jni/corefunctions.h b/java/jni/corefunctions.h
index 73d6cce4..18736dbb 100644
--- a/java/jni/corefunctions.h
+++ b/java/jni/corefunctions.h
@@ -47,6 +47,8 @@ void logErrorMessageAndDie(const char* message);
  */
 void assertNoPendingJniException(JNIEnv* env);
 
+void assertNoPendingJniExceptionIf(JNIEnv* env, bool condition);
+
 } // namespace vanillajni
 } // namespace yoga
 } // namespace facebook