From 145109fc6d7310d844f6c15b74aad29ddb495e79 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 19 Sep 2016 04:43:09 -0700 Subject: [PATCH] Remove additional JSON encoding for native->JS communication Reviewed By: mhorowitz Differential Revision: D3857323 fbshipit-source-id: 4386cc107b8a1425ecb7297b0f659f6c47f01a78 --- Libraries/BatchedBridge/BatchedBridge.js | 7 +- Libraries/Utilities/MessageQueue.js | 16 ++- Libraries/Utilities/PerformanceLogger.js | 4 +- .../src/main/jni/xreact/jni/ProxyExecutor.cpp | 7 +- ReactCommon/cxxreact/BUCK | 3 + ReactCommon/cxxreact/Executor.h | 2 +- ReactCommon/cxxreact/Instance.h | 2 +- ReactCommon/cxxreact/JSCExecutor.cpp | 13 +-- ReactCommon/cxxreact/JSCExecutor.h | 2 +- ReactCommon/cxxreact/MethodCall.cpp | 31 +++--- ReactCommon/cxxreact/MethodCall.h | 2 +- ReactCommon/cxxreact/NativeToJsBridge.cpp | 6 +- ReactCommon/cxxreact/tests/BUCK | 64 +++++++---- ReactCommon/cxxreact/tests/jsarg_helpers.cpp | 101 ++++++++++++++++++ ReactCommon/cxxreact/tests/methodcall.cpp | 57 ++++++---- ReactCommon/cxxreact/tests/value.cpp | 7 +- 16 files changed, 224 insertions(+), 100 deletions(-) create mode 100644 ReactCommon/cxxreact/tests/jsarg_helpers.cpp diff --git a/Libraries/BatchedBridge/BatchedBridge.js b/Libraries/BatchedBridge/BatchedBridge.js index 8e67cf902e3..d06adaa6b14 100644 --- a/Libraries/BatchedBridge/BatchedBridge.js +++ b/Libraries/BatchedBridge/BatchedBridge.js @@ -12,12 +12,7 @@ const MessageQueue = require('MessageQueue'); -const serializeNativeParams = typeof global.__fbBatchedBridgeSerializeNativeParams !== 'undefined'; - -const BatchedBridge = new MessageQueue( - () => global.__fbBatchedBridgeConfig, - serializeNativeParams -); +const BatchedBridge = new MessageQueue(() => global.__fbBatchedBridgeConfig); // TODO: Move these around to solve the cycle in a cleaner way. diff --git a/Libraries/Utilities/MessageQueue.js b/Libraries/Utilities/MessageQueue.js index 0bf8a884079..53db9627030 100644 --- a/Libraries/Utilities/MessageQueue.js +++ b/Libraries/Utilities/MessageQueue.js @@ -19,6 +19,7 @@ const JSTimersExecution = require('JSTimersExecution'); const invariant = require('fbjs/lib/invariant'); const keyMirror = require('fbjs/lib/keyMirror'); +const deepFreezeAndThrowOnMutationInDev = require('deepFreezeAndThrowOnMutationInDev'); const stringifySafe = require('stringifySafe'); const MODULE_IDS = 0; @@ -52,7 +53,7 @@ type Config = { }; class MessageQueue { - constructor(configProvider: () => Config, serializeNativeParams: boolean) { + constructor(configProvider: () => Config) { this._callableModules = {}; this._queue = [[], [], [], 0]; this._callbacks = []; @@ -60,7 +61,6 @@ class MessageQueue { this._callID = 0; this._lastFlush = 0; this._eventLoopStartTime = new Date().getTime(); - this._serializeNativeParams = serializeNativeParams; if (__DEV__) { this._debugInfo = {}; @@ -165,7 +165,7 @@ class MessageQueue { __nativeCall(module, method, params, onFail, onSucc) { if (onFail || onSucc) { if (__DEV__) { - let callId = this._callbackID >> 1; + const callId = this._callbackID >> 1; this._debugInfo[callId] = [module, method]; if (callId > DEBUG_INFO_LIMIT) { delete this._debugInfo[callId - DEBUG_INFO_LIMIT]; @@ -186,8 +186,14 @@ class MessageQueue { this._queue[MODULE_IDS].push(module); this._queue[METHOD_IDS].push(method); - const preparedParams = this._serializeNativeParams ? JSON.stringify(params) : params; - this._queue[PARAMS].push(preparedParams); + if (__DEV__) { + // Any params sent over the bridge should be encodable as JSON + JSON.stringify(params); + + // The params object should not be mutated after being queued + deepFreezeAndThrowOnMutationInDev(params); + } + this._queue[PARAMS].push(params); const now = new Date().getTime(); if (global.nativeFlushQueueImmediate && diff --git a/Libraries/Utilities/PerformanceLogger.js b/Libraries/Utilities/PerformanceLogger.js index d8bd38b93e7..13772ac4dd9 100644 --- a/Libraries/Utilities/PerformanceLogger.js +++ b/Libraries/Utilities/PerformanceLogger.js @@ -11,9 +11,7 @@ 'use strict'; const BatchedBridge = require('BatchedBridge'); -const fbjsPerformanceNow = require('fbjs/lib/performanceNow'); - -const performanceNow = global.nativePerformanceNow || fbjsPerformanceNow; +const performanceNow = global.nativePerformanceNow || require('fbjs/lib/performanceNow'); var timespans = {}; var extras = {}; diff --git a/ReactAndroid/src/main/jni/xreact/jni/ProxyExecutor.cpp b/ReactAndroid/src/main/jni/xreact/jni/ProxyExecutor.cpp index 53cf147fb4e..f23b3f88f18 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/ProxyExecutor.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/ProxyExecutor.cpp @@ -58,9 +58,6 @@ ProxyExecutor::ProxyExecutor(jni::global_ref&& executorInstance, setGlobalVariable( "__fbBatchedBridgeConfig", folly::make_unique(detail::toStdString(folly::toJson(config)))); - setGlobalVariable( - "__fbBatchedBridgeSerializeNativeParams", - folly::make_unique("1")); } ProxyExecutor::~ProxyExecutor() { @@ -95,7 +92,7 @@ void ProxyExecutor::callFunction(const std::string& moduleId, const std::string& std::move(arguments), }; std::string result = executeJSCallWithProxy(m_executor.get(), "callFunctionReturnFlushedQueue", std::move(call)); - m_delegate->callNativeModules(*this, result, true); + m_delegate->callNativeModules(*this, folly::parseJson(result), true); } void ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { @@ -104,7 +101,7 @@ void ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic std::move(arguments) }; std::string result = executeJSCallWithProxy(m_executor.get(), "invokeCallbackAndReturnFlushedQueue", std::move(call)); - m_delegate->callNativeModules(*this, result, true); + m_delegate->callNativeModules(*this, folly::parseJson(result), true); } void ProxyExecutor::setGlobalVariable(std::string propName, diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index 48eece68673..09acf8373db 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/ReactCommon/cxxreact/BUCK @@ -53,6 +53,9 @@ elif THIS_IS_FBOBJC: frameworks = [ '$SDKROOT/System/Library/Frameworks/JavaScriptCore.framework', ], + tests = [ + react_native_xplat_target('cxxreact/tests:tests') + ], **kwargs_add( kwargs, preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS, diff --git a/ReactCommon/cxxreact/Executor.h b/ReactCommon/cxxreact/Executor.h index d1e55ba9714..b69923b754f 100644 --- a/ReactCommon/cxxreact/Executor.h +++ b/ReactCommon/cxxreact/Executor.h @@ -45,7 +45,7 @@ class ExecutorDelegate { virtual std::vector moduleNames() = 0; virtual folly::dynamic getModuleConfig(const std::string& name) = 0; virtual void callNativeModules( - JSExecutor& executor, std::string callJSON, bool isEndOfBatch) = 0; + JSExecutor& executor, folly::dynamic&& calls, bool isEndOfBatch) = 0; virtual MethodCallResult callSerializableNativeHook( JSExecutor& executor, unsigned int moduleId, unsigned int methodId, folly::dynamic&& args) = 0; }; diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index 1e53c81f238..efc1a3a3635 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -56,7 +56,7 @@ class Instance { void handleMemoryPressureCritical(); private: - void callNativeModules(ExecutorToken token, const std::string& calls, bool isEndOfBatch); + void callNativeModules(ExecutorToken token, folly::dynamic&& calls, bool isEndOfBatch); std::shared_ptr callback_; std::unique_ptr nativeToJsBridge_; diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 05ad4a594d7..6cb9f5bf2a6 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -124,9 +124,6 @@ JSCExecutor::JSCExecutor(std::shared_ptr delegate, setGlobalVariable( "__fbBatchedBridgeConfig", folly::make_unique(detail::toStdString(folly::toJson(config)))); - setGlobalVariable( - "__fbBatchedBridgeSerializeNativeParams", - folly::make_unique("")); } JSCExecutor::JSCExecutor( @@ -349,7 +346,7 @@ void JSCExecutor::callNativeModules(Value&& value) { SystraceSection s("JSCExecutor::callNativeModules"); try { auto calls = value.toJSONString(); - m_delegate->callNativeModules(*this, std::move(calls), true); + m_delegate->callNativeModules(*this, folly::parseJson(calls), true); } catch (...) { std::string message = "Error in callNativeModules()"; try { @@ -471,8 +468,9 @@ void JSCExecutor::handleMemoryPressureCritical() { #endif } -void JSCExecutor::flushQueueImmediate(std::string queueJSON) { - m_delegate->callNativeModules(*this, std::move(queueJSON), false); +void JSCExecutor::flushQueueImmediate(Value&& queue) { + auto queueStr = queue.toJSONString(); + m_delegate->callNativeModules(*this, folly::parseJson(queueStr), false); } void JSCExecutor::loadModule(uint32_t moduleId) { @@ -649,8 +647,7 @@ JSValueRef JSCExecutor::nativeFlushQueueImmediate( throw std::invalid_argument("Got wrong number of args"); } - std::string resStr = Value(m_context, arguments[0]).toJSONString(); - flushQueueImmediate(std::move(resStr)); + flushQueueImmediate(Value(m_context, arguments[0])); return JSValueMakeUndefined(m_context); } diff --git a/ReactCommon/cxxreact/JSCExecutor.h b/ReactCommon/cxxreact/JSCExecutor.h index c36e3adc3b1..3d6d366d706 100644 --- a/ReactCommon/cxxreact/JSCExecutor.h +++ b/ReactCommon/cxxreact/JSCExecutor.h @@ -119,7 +119,7 @@ private: void bindBridge() throw(JSException); void callNativeModules(Value&&); void flush(); - void flushQueueImmediate(std::string queueJSON); + void flushQueueImmediate(Value&&); void loadModule(uint32_t moduleId); int addWebWorker(std::string scriptURL, JSValueRef workerRef, JSValueRef globalObjRef); diff --git a/ReactCommon/cxxreact/MethodCall.cpp b/ReactCommon/cxxreact/MethodCall.cpp index aa0757ca52e..17d64c769da 100644 --- a/ReactCommon/cxxreact/MethodCall.cpp +++ b/ReactCommon/cxxreact/MethodCall.cpp @@ -13,42 +13,40 @@ namespace react { #define REQUEST_PARAMSS 2 #define REQUEST_CALLID 3 -std::vector parseMethodCalls(const std::string& json) throw(std::invalid_argument) { - folly::dynamic jsonData = folly::parseJson(json); - +std::vector parseMethodCalls(folly::dynamic&& jsonData) throw(std::invalid_argument) { if (jsonData.isNull()) { return {}; } if (!jsonData.isArray()) { throw std::invalid_argument( - folly::to("Did not get valid calls back from JS: ", jsonData.typeName())); + folly::to("Did not get valid calls back from JS: ", jsonData.typeName())); } if (jsonData.size() < REQUEST_PARAMSS + 1) { throw std::invalid_argument( - folly::to("Did not get valid calls back from JS: size == ", jsonData.size())); + folly::to("Did not get valid calls back from JS: size == ", jsonData.size())); } - auto moduleIds = jsonData[REQUEST_MODULE_IDS]; - auto methodIds = jsonData[REQUEST_METHOD_IDS]; - auto params = jsonData[REQUEST_PARAMSS]; + auto& moduleIds = jsonData[REQUEST_MODULE_IDS]; + auto& methodIds = jsonData[REQUEST_METHOD_IDS]; + auto& params = jsonData[REQUEST_PARAMSS]; int callId = -1; if (!moduleIds.isArray() || !methodIds.isArray() || !params.isArray()) { throw std::invalid_argument( - folly::to("Did not get valid calls back from JS: ", json.c_str())); + folly::to("Did not get valid calls back from JS: ", folly::toJson(jsonData))); } if (moduleIds.size() != methodIds.size() || moduleIds.size() != params.size()) { throw std::invalid_argument( - folly::to("Did not get valid calls back from JS: ", json.c_str())); + folly::to("Did not get valid calls back from JS: ", folly::toJson(jsonData))); } if (jsonData.size() > REQUEST_CALLID) { if (!jsonData[REQUEST_CALLID].isInt()) { throw std::invalid_argument( - folly::to("Did not get valid calls back from JS: %s", json.c_str())); + folly::to("Did not get valid calls back from JS: %s", folly::toJson(jsonData))); } else { callId = jsonData[REQUEST_CALLID].getInt(); } @@ -56,20 +54,15 @@ std::vector parseMethodCalls(const std::string& json) throw(std::inv std::vector methodCalls; for (size_t i = 0; i < moduleIds.size(); i++) { - if (!params[i].isString()) { + if (!params[i].isArray()) { throw std::invalid_argument( - folly::to("Call argument isn't a string")); - } - auto paramsValue = folly::parseJson(params[i].asString()); - if (!paramsValue.isArray()) { - throw std::invalid_argument( - folly::to("Parsed params isn't an array")); + folly::to("Call argument isn't an array")); } methodCalls.emplace_back( moduleIds[i].getInt(), methodIds[i].getInt(), - std::move(paramsValue), + std::move(params[i]), callId); // only incremement callid if contains valid callid as callid is optional diff --git a/ReactCommon/cxxreact/MethodCall.h b/ReactCommon/cxxreact/MethodCall.h index a28bf40e574..bd50b7b2144 100644 --- a/ReactCommon/cxxreact/MethodCall.h +++ b/ReactCommon/cxxreact/MethodCall.h @@ -24,6 +24,6 @@ struct MethodCall { , callId(cid) {} }; -std::vector parseMethodCalls(const std::string& json) throw(std::invalid_argument); +std::vector parseMethodCalls(folly::dynamic&& calls) throw(std::invalid_argument); } } diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index ecddf32e230..130ccd21597 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -53,13 +53,13 @@ public: } void callNativeModules( - JSExecutor& executor, std::string callJSON, bool isEndOfBatch) override { + JSExecutor& executor, folly::dynamic&& calls, bool isEndOfBatch) override { ExecutorToken token = m_nativeToJs->getTokenForExecutor(executor); - m_nativeQueue->runOnQueue([this, token, callJSON=std::move(callJSON), isEndOfBatch] { + m_nativeQueue->runOnQueue([this, token, calls=std::move(calls), isEndOfBatch] () mutable { // An exception anywhere in here stops processing of the batch. This // was the behavior of the Android bridge, and since exception handling // terminates the whole bridge, there's not much point in continuing. - for (auto& call : react::parseMethodCalls(callJSON)) { + for (auto& call : react::parseMethodCalls(std::move(calls))) { m_registry->callNativeMethod( token, call.moduleId, call.methodId, std::move(call.arguments), call.callId); } diff --git a/ReactCommon/cxxreact/tests/BUCK b/ReactCommon/cxxreact/tests/BUCK index 6f30d733a0d..2b3bb2ef2c2 100644 --- a/ReactCommon/cxxreact/tests/BUCK +++ b/ReactCommon/cxxreact/tests/BUCK @@ -1,24 +1,42 @@ -include_defs('//ReactAndroid/DEFS') -include_defs('//ReactAndroid/TEST_DEFS') +TEST_SRCS = [ + 'CxxMessageQueueTest.cpp', + 'jsarg_helpers.cpp', + 'jscexecutor.cpp', + 'jsclogging.cpp', + 'methodcall.cpp', + 'value.cpp', +] -jni_instrumentation_test_lib( - name = 'tests', - class_under_test = 'com/facebook/react/XplatBridgeTest', - soname = 'libxplat-bridge.so', - srcs = [ - 'CxxMessageQueueTest.cpp', - 'value.cpp', - 'methodcall.cpp', - 'jsclogging.cpp', - 'jscexecutor.cpp', - ], - compiler_flags = [ - '-fexceptions', - ], - deps = [ - '//native/third-party/android-ndk:android', - '//xplat/third-party/gmock:gtest', - react_native_xplat_target('cxxreact:bridge'), - ], - visibility = ['//instrumentation_tests/...'], -) +if THIS_IS_FBANDROID: + include_defs('//ReactAndroid/DEFS') + include_defs('//ReactAndroid/TEST_DEFS') + + jni_instrumentation_test_lib( + name = 'tests', + class_under_test = 'com/facebook/react/XplatBridgeTest', + soname = 'libxplat-bridge.so', + srcs = TEST_SRCS, + compiler_flags = [ + '-fexceptions', + ], + deps = [ + '//native/third-party/android-ndk:android', + '//xplat/third-party/gmock:gtest', + react_native_xplat_target('cxxreact:bridge'), + ], + visibility = ['//instrumentation_tests/...'], + ) + +if THIS_IS_FBOBJC: + fb_xplat_cxx_test( + name = 'tests', + srcs = TEST_SRCS, + compiler_flags = [ + '-fexceptions', + ], + deps = [ + '//xplat/third-party/gmock:gtest', + react_native_xplat_target('cxxreact:bridge'), + ], + visibility = [react_native_xplat_target('cxxreact/...')], + ) diff --git a/ReactCommon/cxxreact/tests/jsarg_helpers.cpp b/ReactCommon/cxxreact/tests/jsarg_helpers.cpp new file mode 100644 index 00000000000..0a2268a6a9a --- /dev/null +++ b/ReactCommon/cxxreact/tests/jsarg_helpers.cpp @@ -0,0 +1,101 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include + +#include + +#include +#include +#include + +using namespace std; +using namespace folly; +using namespace facebook::xplat; + +#define EXPECT_JSAE(statement, exstr) do { \ + try { \ + statement; \ + FAIL() << "Expected JsArgumentException(" << (exstr) << ") not thrown"; \ + } catch (const JsArgumentException& ex) { \ + EXPECT_EQ(ex.what(), std::string(exstr)); \ + } \ + } while(0) // let any other exception escape, gtest will deal. + +TEST(JsArgumentHelpersTest, args) { + const bool aBool = true; + const int64_t anInt = 17; + const double aDouble = 3.14; + const string aString = "word"; + const dynamic anArray = dynamic::array("a", "b", "c"); + const dynamic anObject = dynamic::object("k1", "v1")("k2", "v2"); + const string aNumericString = to(anInt); + + folly::dynamic args = dynamic::array(aBool, anInt, aDouble, aString, anArray, anObject, aNumericString); + + EXPECT_EQ(jsArgAsBool(args, 0), aBool); + EXPECT_EQ(jsArgAsInt(args, 1), anInt); + EXPECT_EQ(jsArgAsDouble(args, 2), aDouble); + EXPECT_EQ(jsArgAsString(args, 3), aString); + EXPECT_EQ(jsArgAsArray(args, 4), anArray); + EXPECT_EQ(jsArgAsObject(args, 5), anObject); + + // const args + const folly::dynamic& cargs = args; + const folly::dynamic& a4 = jsArgAsArray(cargs, 4); + EXPECT_EQ(a4, anArray); + EXPECT_EQ(jsArgAsObject(cargs, 5), anObject); + + // helpers returning dynamic should return same object without copying + EXPECT_EQ(&jsArgAsArray(args, 4), &(args[4])); + EXPECT_EQ(&jsArgAsArray(cargs, 4), &(args[4])); + + // dynamics returned for mutable args should be mutable. The test is that + // this compiles. + jsArgAsArray(args, 4)[2] = "d"; + jsArgAsArray(args, 4)[2] = "c"; + // These fail to compile due to constness. + // jsArgAsArray(cargs, 4)[2] = "d"; + // jsArgAsArray(cargs, 4)[2] = "c"; + + // ref-qualified member function tests + EXPECT_EQ(jsArgN(args, 3, &folly::dynamic::getString), aString); + EXPECT_EQ(jsArg(args[3], &folly::dynamic::getString), aString); + + // conversions + EXPECT_EQ(jsArgAsDouble(args, 1), anInt * 1.0); + EXPECT_EQ(jsArgAsString(args, 1), aNumericString); + EXPECT_EQ(jsArgAsInt(args, 6), anInt); + + // Test exception messages. + + // out_of_range + EXPECT_JSAE(jsArgAsBool(args, 7), + "JavaScript provided 7 arguments for C++ method which references at least " + "8 arguments: out of range in dynamic array"); + // Conv range_error (invalid value conversion) + const std::string exhead = "Could not convert argument 3 to required type: "; + const std::string extail = ": Invalid leading character: \"word\""; + try { + jsArgAsInt(args, 3); + FAIL() << "Expected JsArgumentException(" << exhead << "..." << extail << ") not thrown"; + } catch (const JsArgumentException& ex) { + const std::string exwhat = ex.what(); + + EXPECT_GT(exwhat.size(), exhead.size()); + EXPECT_GT(exwhat.size(), extail.size()); + + EXPECT_TRUE(std::equal(exhead.cbegin(), exhead.cend(), exwhat.cbegin())) + << "JsArgumentException('" << exwhat << "') does not begin with '" << exhead << "'"; + EXPECT_TRUE(std::equal(extail.crbegin(), extail.crend(), exwhat.crbegin())) + << "JsArgumentException('" << exwhat << "') does not end with '" << extail << "'"; + } + // inconvertible types + EXPECT_JSAE(jsArgAsArray(args, 2), + "Argument 3 of type double is not required type Array"); + EXPECT_JSAE(jsArgAsInt(args, 4), + "Error converting javascript arg 4 to C++: " + "TypeError: expected dynamic type `int/double/bool/string', but had type `array'"); + // type predicate failure + EXPECT_JSAE(jsArgAsObject(args, 4), + "Argument 5 of type array is not required type Object"); +} diff --git a/ReactCommon/cxxreact/tests/methodcall.cpp b/ReactCommon/cxxreact/tests/methodcall.cpp index 233a5f6724e..ffc649d8996 100644 --- a/ReactCommon/cxxreact/tests/methodcall.cpp +++ b/ReactCommon/cxxreact/tests/methodcall.cpp @@ -2,13 +2,15 @@ #include #include +#include using namespace facebook; using namespace facebook::react; +using namespace folly; TEST(parseMethodCalls, SingleReturnCallNoArgs) { - auto jsText = "[[7],[3],[\"[]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[7],[3],[[]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(0, returnedCall.arguments.size()); @@ -18,31 +20,40 @@ TEST(parseMethodCalls, SingleReturnCallNoArgs) { TEST(parseMethodCalls, InvalidReturnFormat) { try { - parseMethodCalls("{\"foo\":1}"); + auto input = dynamic::object("foo", 1); + parseMethodCalls(std::move(input)); ADD_FAILURE(); } catch (const std::invalid_argument&) { // ignored } try { - parseMethodCalls("[{\"foo\":1}]"); + auto input = dynamic::array(dynamic::object("foo", 1)); + parseMethodCalls(std::move(input)); ADD_FAILURE(); } catch (const std::invalid_argument&) { // ignored } try { - parseMethodCalls("[1,4,{\"foo\":2}]"); + auto input = dynamic::array(1, 4, dynamic::object("foo", 2)); + parseMethodCalls(std::move(input)); ADD_FAILURE(); } catch (const std::invalid_argument&) { // ignored } try { - parseMethodCalls("[[1],[4],{\"foo\":2}]"); + auto input = dynamic::array(dynamic::array(1), + dynamic::array(4), + dynamic::object("foo", 2)); + parseMethodCalls(std::move(input)); ADD_FAILURE(); } catch (const std::invalid_argument&) { // ignored } try { - parseMethodCalls("[[1],[4],[]]"); + auto input = dynamic::array(dynamic::array(1), + dynamic::array(4), + dynamic::array()); + parseMethodCalls(std::move(input)); ADD_FAILURE(); } catch (const std::invalid_argument&) { // ignored @@ -50,8 +61,8 @@ TEST(parseMethodCalls, InvalidReturnFormat) { } TEST(parseMethodCalls, NumberReturn) { - auto jsText = "[[0],[0],[\"[\\\"foobar\\\"]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[\"foobar\"]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -60,8 +71,8 @@ TEST(parseMethodCalls, NumberReturn) { } TEST(parseMethodCalls, StringReturn) { - auto jsText = "[[0],[0],[\"[42.16]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[42.16]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -70,8 +81,8 @@ TEST(parseMethodCalls, StringReturn) { } TEST(parseMethodCalls, BooleanReturn) { - auto jsText = "[[0],[0],[\"[false]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[false]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -80,8 +91,8 @@ TEST(parseMethodCalls, BooleanReturn) { } TEST(parseMethodCalls, NullReturn) { - auto jsText = "[[0],[0],[\"[null]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[null]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -89,8 +100,8 @@ TEST(parseMethodCalls, NullReturn) { } TEST(parseMethodCalls, MapReturn) { - auto jsText = "[[0],[0],[\"[{\\\"foo\\\": \\\"hello\\\", \\\"bar\\\": 4.0, \\\"baz\\\": true}]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[{\"foo\": \"hello\", \"bar\": 4.0, \"baz\": true}]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -105,8 +116,8 @@ TEST(parseMethodCalls, MapReturn) { } TEST(parseMethodCalls, ArrayReturn) { - auto jsText = "[[0],[0],[\"[[\\\"foo\\\", 42.0, false]]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[[\"foo\", 42.0, false]]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(1, returnedCall.arguments.size()); @@ -119,8 +130,8 @@ TEST(parseMethodCalls, ArrayReturn) { } TEST(parseMethodCalls, ReturnMultipleParams) { - auto jsText = "[[0],[0],[\"[\\\"foo\\\", 14, null, false]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0],[0],[[\"foo\", 14, null, false]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(1, returnedCalls.size()); auto returnedCall = returnedCalls[0]; ASSERT_EQ(4, returnedCall.arguments.size()); @@ -131,7 +142,7 @@ TEST(parseMethodCalls, ReturnMultipleParams) { } TEST(parseMethodCalls, ParseTwoCalls) { - auto jsText = "[[0,0],[1,1],[\"[]\",\"[]\"]]"; - auto returnedCalls = parseMethodCalls(jsText); + auto jsText = "[[0,0],[1,1],[[],[]]]"; + auto returnedCalls = parseMethodCalls(folly::parseJson(jsText)); ASSERT_EQ(2, returnedCalls.size()); } diff --git a/ReactCommon/cxxreact/tests/value.cpp b/ReactCommon/cxxreact/tests/value.cpp index b05257f43ef..40a4b72cde1 100644 --- a/ReactCommon/cxxreact/tests/value.cpp +++ b/ReactCommon/cxxreact/tests/value.cpp @@ -2,11 +2,13 @@ #include #include #include +#include +#ifdef WITH_FBJSCEXTENSION #undef ASSERT #include -#include #include "OpaqueJSString.h" +#endif #include @@ -58,6 +60,7 @@ TEST(Value, ToJSONString) { JSGlobalContextRelease(ctx); } +#ifdef WITH_FBJSCEXTENSION // Just test that handling invalid data doesn't crash. TEST(Value, FromBadUtf8) { prepare(); @@ -101,3 +104,5 @@ TEST(Value, BadUtf16) { v.toJSONString(0); JSGlobalContextRelease(ctx); } +#endif +