Type-check JS args when converting them to Java args
Summary: When you call a TurboModule method with JS arguments, we necessarily convert these JS args to Java objects/primitives before passing them to the Java implementation of the method. The problem is that we let the type of the JS arg dictate the type of the Java object/primitive to convert said arg to. This means that if a JS developer passes in an `number` to a function that expects an `Array`, we'll convert the number to a `double` and try to call the Java method with that `double`, when it actually expects a `ReadableArray`. This will trigger a JNI error, and crash the program. Ideally, we should be able to catch these type mismatches early on. In this diff, on every TurboModule method call, I parse the method signature to determine the Java type of each JS argument. Then, for any argument, if the JS arg and the Java arg types aren't compatible, I raise an exception, which gets displayed as a RedBox in development. This diff also implements support for `?number` and `?boolean` argument and return types in TurboModules. Reviewed By: mdvacca Differential Revision: D16214814 fbshipit-source-id: 4399bb88c5344cff50aa8fe8d54eb2000990a852
This commit is contained in:
Ramanpreet Nara
Facebook Github Bot
Родитель
3b6f6ca4d5
Коммит
ff9323a0b0
|
|
@ -6,6 +6,7 @@
|
|||
*/
|
||||
|
||||
#include <memory>
|
||||
#include <sstream>
|
||||
#include <string>
|
||||
|
||||
#include <fb/fbjni.h>
|
||||
|
|
@ -56,50 +57,267 @@ jni::local_ref<JCxxCallbackImpl::JavaPart> createJavaCallbackFromJSIFunction(
|
|||
return JCxxCallbackImpl::newObjectCxxArgs(fn);
|
||||
}
|
||||
|
||||
// fnjni already does this conversion, but since we are using plain JNI, this needs to be done again
|
||||
// TODO (axe) Reuse existing implementation as needed - the exist in MethodInvoker.cpp
|
||||
// TODO (axe) If at runtime, JS sends incorrect arguments and this is not typechecked, conversion here will fail. Check for that case (OSS)
|
||||
namespace {
|
||||
|
||||
template <typename T>
|
||||
std::string to_string(T v) {
|
||||
std::ostringstream stream;
|
||||
stream << v;
|
||||
return stream.str();
|
||||
}
|
||||
|
||||
// This is used for generating short exception strings.
|
||||
std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) {
|
||||
if (v.isUndefined()) {
|
||||
return "undefined";
|
||||
}
|
||||
|
||||
if (v.isNull()) {
|
||||
return "null";
|
||||
}
|
||||
|
||||
if (v.isBool()) {
|
||||
return std::string("a boolean (") + (v.getBool() ? "true" : "false") + ")";
|
||||
}
|
||||
|
||||
if (v.isNumber()) {
|
||||
return "a number (" + to_string(v.getNumber()) + ")";
|
||||
}
|
||||
|
||||
if (v.isString()) {
|
||||
return "a string (\"" + v.getString(*rt).utf8(*rt) + "\")";
|
||||
}
|
||||
|
||||
assert(v.isObject() && "Expecting object.");
|
||||
return rt != nullptr && v.getObject(*rt).isFunction(*rt) ? "a function"
|
||||
: "an object";
|
||||
}
|
||||
|
||||
class JavaTurboModuleArgumentConversionException : public std::runtime_error {
|
||||
public:
|
||||
JavaTurboModuleArgumentConversionException(
|
||||
const std::string &expectedType,
|
||||
int index,
|
||||
const std::string &methodName,
|
||||
const jsi::Value *arg,
|
||||
jsi::Runtime *rt)
|
||||
: std::runtime_error(
|
||||
"Expected argument " + to_string(index) + " of method \"" +
|
||||
methodName + "\" to be a " + expectedType + ", but got " +
|
||||
stringifyJSIValue(*arg, rt)) {}
|
||||
};
|
||||
|
||||
class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error {
|
||||
public:
|
||||
JavaTurboModuleInvalidArgumentTypeException(
|
||||
const std::string &actualType,
|
||||
int argIndex,
|
||||
const std::string &methodName)
|
||||
: std::runtime_error(
|
||||
"Called method \"" + methodName + "\" with unsupported type " +
|
||||
actualType + " at argument " + to_string(argIndex)) {}
|
||||
};
|
||||
|
||||
class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error {
|
||||
public:
|
||||
JavaTurboModuleInvalidArgumentCountException(
|
||||
const std::string &methodName,
|
||||
int actualArgCount,
|
||||
int expectedArgCount)
|
||||
: std::runtime_error(
|
||||
"TurboModule method \"" + methodName + "\" called with " +
|
||||
to_string(actualArgCount) +
|
||||
" arguments (expected argument count: " +
|
||||
to_string(expectedArgCount) + ").") {}
|
||||
};
|
||||
|
||||
/**
|
||||
* See
|
||||
* https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html
|
||||
* for a description of Java method signature structure.
|
||||
*/
|
||||
std::vector<std::string> getMethodArgTypesFromSignature(
|
||||
const std::string &methodSignature) {
|
||||
std::vector<std::string> methodArgs;
|
||||
|
||||
for (auto it = methodSignature.begin(); it != methodSignature.end();
|
||||
it += 1) {
|
||||
if (*it == '(') {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (*it == ')') {
|
||||
break;
|
||||
}
|
||||
|
||||
std::string type;
|
||||
|
||||
if (*it == '[') {
|
||||
type += *it;
|
||||
it += 1;
|
||||
}
|
||||
|
||||
if (*it == 'L') {
|
||||
for (; it != methodSignature.end(); it += 1) {
|
||||
type += *it;
|
||||
|
||||
if (*it == ';') {
|
||||
break;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
type += *it;
|
||||
}
|
||||
|
||||
methodArgs.push_back(type);
|
||||
}
|
||||
|
||||
return methodArgs;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
// fnjni already does this conversion, but since we are using plain JNI, this
|
||||
// needs to be done again
|
||||
// TODO (axe) Reuse existing implementation as needed - the exist in
|
||||
// MethodInvoker.cpp
|
||||
std::vector<jvalue> convertJSIArgsToJNIArgs(
|
||||
JNIEnv *env,
|
||||
jsi::Runtime &rt,
|
||||
std::string methodName,
|
||||
std::vector<std::string> methodArgTypes,
|
||||
const jsi::Value *args,
|
||||
size_t count,
|
||||
std::shared_ptr<JSCallInvoker> jsInvoker,
|
||||
TurboModuleMethodValueKind valueKind) {
|
||||
unsigned int expectedArgumentCount = valueKind == PromiseKind
|
||||
? methodArgTypes.size() - 1
|
||||
: methodArgTypes.size();
|
||||
|
||||
if (expectedArgumentCount != count) {
|
||||
throw JavaTurboModuleInvalidArgumentCountException(
|
||||
methodName, count, expectedArgumentCount);
|
||||
}
|
||||
|
||||
auto jargs =
|
||||
std::vector<jvalue>(valueKind == PromiseKind ? count + 1 : count);
|
||||
for (size_t i = 0; i < count; i++) {
|
||||
const jsi::Value *arg = &args[i];
|
||||
if (arg->isBool()) {
|
||||
jargs[i].z = arg->getBool();
|
||||
} else if (arg->isNumber()) {
|
||||
jargs[i].d = arg->getNumber();
|
||||
} else if (arg->isNull() || arg->isUndefined()) {
|
||||
// What happens if Java is expecting a bool or a number, and JS sends a null or undefined?
|
||||
jargs[i].l = nullptr;
|
||||
} else if (arg->isString()) {
|
||||
// We are basically creating a whole new string here
|
||||
// TODO (axe) Is there a way to copy this instead of creating a whole new string ?
|
||||
jargs[i].l = env->NewStringUTF(arg->getString(rt).utf8(rt).c_str());
|
||||
} else if (arg->isObject()) {
|
||||
auto objectArg = arg->getObject(rt);
|
||||
// We are currently using folly:dynamic to convert JSON to Writable Map
|
||||
// TODO (axe) Don't use folly:dynamic, instead construct Java map directly
|
||||
if (objectArg.isArray(rt)) {
|
||||
auto dynamicFromValue = jsi::dynamicFromValue(rt, args[i]);
|
||||
auto jParams = ReadableNativeArray::newObjectCxxArgs(std::move(dynamicFromValue));
|
||||
jargs[i].l = jParams.release();
|
||||
} else if (objectArg.isFunction(rt)) {
|
||||
jsi::Function fn = objectArg.getFunction(rt);
|
||||
jargs[i].l =
|
||||
createJavaCallbackFromJSIFunction(fn, rt, jsInvoker).release();
|
||||
} else {
|
||||
auto dynamicFromValue = jsi::dynamicFromValue(rt, args[i]);
|
||||
auto jParams = ReadableNativeMap::createWithContents(std::move(dynamicFromValue));
|
||||
jargs[i].l = jParams.release();
|
||||
}
|
||||
|
||||
for (unsigned int argIndex = 0; argIndex < count; argIndex += 1) {
|
||||
std::string type = methodArgTypes.at(argIndex);
|
||||
|
||||
const jsi::Value *arg = &args[argIndex];
|
||||
jvalue *jarg = &jargs[argIndex];
|
||||
|
||||
if (type == "D") {
|
||||
if (!arg->isNumber()) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"number", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jarg->d = arg->getNumber();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Z") {
|
||||
if (!arg->isBool()) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"boolean", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jarg->z = (jboolean)arg->getBool();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!(type == "Ljava/lang/Double;" || type == "Ljava/lang/Boolean;" ||
|
||||
type == "Ljava/lang/String;" ||
|
||||
type == "Lcom/facebook/react/bridge/ReadableArray;" ||
|
||||
type == "Lcom/facebook/react/bridge/Callback;" ||
|
||||
type == "Lcom/facebook/react/bridge/ReadableMap;")) {
|
||||
throw JavaTurboModuleInvalidArgumentTypeException(
|
||||
type, argIndex, methodName);
|
||||
}
|
||||
|
||||
if (arg->isNull() || arg->isUndefined()) {
|
||||
jarg->l = nullptr;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Ljava/lang/Double;") {
|
||||
if (!arg->isNumber()) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"number", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jclass doubleClass = env->FindClass("java/lang/Double");
|
||||
jmethodID doubleConstructor =
|
||||
env->GetMethodID(doubleClass, "<init>", "(D)V");
|
||||
jarg->l =
|
||||
env->NewObject(doubleClass, doubleConstructor, arg->getNumber());
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Ljava/lang/Boolean;") {
|
||||
if (!arg->isBool()) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"boolean", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jclass booleanClass = env->FindClass("java/lang/Boolean");
|
||||
jmethodID booleanConstructor =
|
||||
env->GetMethodID(booleanClass, "<init>", "(Z)V");
|
||||
jarg->l =
|
||||
env->NewObject(booleanClass, booleanConstructor, arg->getBool());
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Ljava/lang/String;") {
|
||||
if (!arg->isString()) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"string", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jarg->l = env->NewStringUTF(arg->getString(rt).utf8(rt).c_str());
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Lcom/facebook/react/bridge/ReadableArray;") {
|
||||
if (!(arg->isObject() && arg->getObject(rt).isArray(rt))) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"Array", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
auto dynamicFromValue = jsi::dynamicFromValue(rt, *arg);
|
||||
auto jParams =
|
||||
ReadableNativeArray::newObjectCxxArgs(std::move(dynamicFromValue));
|
||||
jarg->l = jParams.release();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Lcom/facebook/react/bridge/Callback;") {
|
||||
if (!(arg->isObject() && arg->getObject(rt).isFunction(rt))) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"Function", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
jsi::Function fn = arg->getObject(rt).getFunction(rt);
|
||||
jarg->l = createJavaCallbackFromJSIFunction(fn, rt, jsInvoker).release();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type == "Lcom/facebook/react/bridge/ReadableMap;") {
|
||||
if (!(arg->isObject())) {
|
||||
throw JavaTurboModuleArgumentConversionException(
|
||||
"Object", argIndex, methodName, arg, &rt);
|
||||
}
|
||||
|
||||
auto dynamicFromValue = jsi::dynamicFromValue(rt, *arg);
|
||||
auto jParams =
|
||||
ReadableNativeMap::createWithContents(std::move(dynamicFromValue));
|
||||
jarg->l = jParams.release();
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
return jargs;
|
||||
}
|
||||
|
||||
|
|
@ -142,8 +360,17 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
|
|||
return convertFromJMapToValue(env, runtime, constantsMap);
|
||||
}
|
||||
|
||||
std::vector<jvalue> jargs =
|
||||
convertJSIArgsToJNIArgs(env, runtime, args, count, jsInvoker_, valueKind);
|
||||
std::vector<std::string> methodArgTypes =
|
||||
getMethodArgTypesFromSignature(methodSignature);
|
||||
std::vector<jvalue> jargs = convertJSIArgsToJNIArgs(
|
||||
env,
|
||||
runtime,
|
||||
methodName,
|
||||
methodArgTypes,
|
||||
args,
|
||||
count,
|
||||
jsInvoker_,
|
||||
valueKind);
|
||||
|
||||
switch (valueKind) {
|
||||
case VoidKind: {
|
||||
|
|
@ -153,6 +380,27 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
|
|||
return jsi::Value::undefined();
|
||||
}
|
||||
case BooleanKind: {
|
||||
std::string returnType =
|
||||
methodSignature.substr(methodSignature.find_last_of(')') + 1);
|
||||
if (returnType == "Ljava/lang/Boolean;") {
|
||||
auto returnObject =
|
||||
(jobject)env->CallObjectMethodA(instance, methodID, jargs.data());
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
||||
if (returnObject == nullptr) {
|
||||
return jsi::Value::null();
|
||||
}
|
||||
|
||||
jclass booleanClass = env->FindClass("java/lang/Boolean");
|
||||
jmethodID booleanValueMethod =
|
||||
env->GetMethodID(booleanClass, "booleanValue", "()Z");
|
||||
bool returnBoolean =
|
||||
(bool)env->CallBooleanMethod(returnObject, booleanValueMethod);
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
||||
return jsi::Value(returnBoolean);
|
||||
}
|
||||
|
||||
bool returnBoolean =
|
||||
(bool)env->CallBooleanMethodA(instance, methodID, jargs.data());
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
|
@ -160,6 +408,27 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
|
|||
return jsi::Value(returnBoolean);
|
||||
}
|
||||
case NumberKind: {
|
||||
std::string returnType =
|
||||
methodSignature.substr(methodSignature.find_last_of(')') + 1);
|
||||
if (returnType == "Ljava/lang/Double;") {
|
||||
auto returnObject =
|
||||
(jobject)env->CallObjectMethodA(instance, methodID, jargs.data());
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
||||
if (returnObject == nullptr) {
|
||||
return jsi::Value::null();
|
||||
}
|
||||
|
||||
jclass doubleClass = env->FindClass("java/lang/Double");
|
||||
jmethodID doubleValueMethod =
|
||||
env->GetMethodID(doubleClass, "doubleValue", "()D");
|
||||
double returnDouble =
|
||||
(double)env->CallDoubleMethod(returnObject, doubleValueMethod);
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
||||
return jsi::Value(returnDouble);
|
||||
}
|
||||
|
||||
double returnDouble =
|
||||
(double)env->CallDoubleMethodA(instance, methodID, jargs.data());
|
||||
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче