Handle stack overflow in JSError construction gracefully

Summary:
JSError creation can lead to further errors.  Make sure these cases
are handled and don't cause weird crashes or other issues.

This solution has a few parts:
 * include a ScopedNativeDepthTracker in checkStatus
 * If an exception object message or stack property is already a String, don't
   call JS String ctor on it
 * Verify that a jsi::Value is a String before calling getString on it.
 * Add more tests for JSError construction

Changelog: [Internal]

Reviewed By: dulinriley

Differential Revision: D21851645

fbshipit-source-id: 2d10da0e741ad4ede93cd806320f68ad512e5138

ReactCommon/jsi/jsi/jsi.cpp

@@ -401,32 +401,63 @@ JSError::JSError(std::string what, Runtime& rt, Value&& value)
 }
 
 void JSError::setValue(Runtime& rt, Value&& value) {
-  value_ = std::make_shared<jsi::Value>(std::move(value));
+  value_ = std::make_shared<Value>(std::move(value));
 
   try {
     if ((message_.empty() || stack_.empty()) && value_->isObject()) {
       auto obj = value_->getObject(rt);
 
       if (message_.empty()) {
-        jsi::Value message = obj.getProperty(rt, "message");
-        if (!message.isUndefined()) {
-          message_ =
-              callGlobalFunction(rt, "String", message).getString(rt).utf8(rt);
+        try {
+          Value message = obj.getProperty(rt, "message");
+          if (!message.isUndefined() && !message.isString()) {
+            message = callGlobalFunction(rt, "String", message);
+          }
+          if (message.isString()) {
+            message_ = message.getString(rt).utf8(rt);
+          } else if (!message.isUndefined()) {
+            message_ = "String(e.message) is a " + kindToString(message, &rt);
+          }
+        } catch (const std::exception& ex) {
+          message_ = std::string("[Exception while creating message string: ") +
+              ex.what() + "]";
         }
       }
 
       if (stack_.empty()) {
-        jsi::Value stack = obj.getProperty(rt, "stack");
-        if (!stack.isUndefined()) {
-          stack_ =
-              callGlobalFunction(rt, "String", stack).getString(rt).utf8(rt);
+        try {
+          Value stack = obj.getProperty(rt, "stack");
+          if (!stack.isUndefined() && !stack.isString()) {
+            stack = callGlobalFunction(rt, "String", stack);
+          }
+          if (stack.isString()) {
+            stack_ = stack.getString(rt).utf8(rt);
+          } else if (!stack.isUndefined()) {
+            stack_ = "String(e.stack) is a " + kindToString(stack, &rt);
+          }
+        } catch (const std::exception& ex) {
+          message_ = std::string("[Exception while creating stack string: ") +
+              ex.what() + "]";
         }
       }
     }
 
     if (message_.empty()) {
-      message_ =
-          callGlobalFunction(rt, "String", *value_).getString(rt).utf8(rt);
+      try {
+        if (value_->isString()) {
+          message_ = value_->getString(rt).utf8(rt);
+        } else {
+          Value message = callGlobalFunction(rt, "String", *value_);
+          if (message.isString()) {
+            message_ = message.getString(rt).utf8(rt);
+          } else {
+            message_ = "String(e) is a " + kindToString(message, &rt);
+          }
+        }
+      } catch (const std::exception& ex) {
+        message_ = std::string("[Exception while creating message string: ") +
+            ex.what() + "]";
+      }
     }
 
     if (stack_.empty()) {
@@ -436,13 +467,6 @@ void JSError::setValue(Runtime& rt, Value&& value) {
     if (what_.empty()) {
       what_ = message_ + "\n\n" + stack_;
     }
-  } catch (const std::exception& ex) {
-    message_ = std::string("[Exception while creating message string: ") +
-        ex.what() + "]";
-    stack_ = std::string("Exception while creating stack string: ") +
-        ex.what() + "]";
-    what_ =
-        std::string("Exception while getting value fields: ") + ex.what() + "]";
   } catch (...) {
     message_ = "[Exception caught creating message string]";
     stack_ = "[Exception caught creating stack string]";

ReactCommon/jsi/jsi/test/testlib.cpp

@@ -981,19 +981,6 @@ TEST_P(JSITest, JSErrorsCanBeConstructedWithStack) {
 }
 
 TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) {
-  Value globalString = rt.global().getProperty(rt, "String");
-  rt.global().setProperty(rt, "String", Value::undefined());
-  try {
-    eval("throw Error('whoops')");
-    FAIL() << "expected exception";
-  } catch (const JSError& ex) {
-    EXPECT_EQ(
-        ex.getMessage(),
-        "[Exception while creating message string: callGlobalFunction: "
-        "JS global property 'String' is undefined, expected a Function]");
-  }
-  rt.global().setProperty(rt, "String", globalString);
-
   Value globalError = rt.global().getProperty(rt, "Error");
   rt.global().setProperty(rt, "Error", Value::undefined());
   try {
@@ -1272,6 +1259,70 @@ TEST_P(JSITest, SymbolTest) {
   EXPECT_FALSE(Value::strictEquals(rt, eval("Symbol('a')"), eval("'a'")));
 }
 
+TEST_P(JSITest, JSErrorTest) {
+  // JSError creation can lead to further errors.  Make sure these
+  // cases are handled and don't cause weird crashes or other issues.
+  //
+  // Getting message property can throw
+
+  EXPECT_THROW(
+      eval("var GetMessageThrows = {get message() { throw Error('ex'); }};"
+           "throw GetMessageThrows;"),
+      JSIException);
+
+  EXPECT_THROW(
+      eval("var GetMessageThrows = {get message() { throw GetMessageThrows; }};"
+           "throw GetMessageThrows;"),
+      JSIException);
+
+  // Converting exception message to String can throw
+
+  EXPECT_THROW(
+      eval(
+          "Object.defineProperty("
+          "  globalThis, 'String', {configurable:true, get() { var e = Error(); e.message = 23; throw e; }});"
+          "var e = Error();"
+          "e.message = 17;"
+          "throw e;"),
+      JSIException);
+
+  EXPECT_THROW(
+      eval(
+          "var e = Error();"
+          "Object.defineProperty("
+          "  e, 'message', {configurable:true, get() { throw Error('getter'); }});"
+          "throw e;"),
+      JSIException);
+
+  EXPECT_THROW(
+      eval("var e = Error();"
+           "String = function() { throw Error('ctor'); };"
+           "throw e;"),
+      JSIException);
+
+  // Converting an exception message to String can return a non-String
+
+  EXPECT_THROW(
+      eval("String = function() { return 42; };"
+           "var e = Error();"
+           "e.message = 17;"
+           "throw e;"),
+      JSIException);
+
+  // Exception can be non-Object
+
+  EXPECT_THROW(eval("throw 17;"), JSIException);
+
+  EXPECT_THROW(eval("throw undefined;"), JSIException);
+
+  // Converting exception with no message or stack property to String can throw
+
+  EXPECT_THROW(
+      eval("var e = {toString() { throw new Error('errstr'); }};"
+           "throw e;"),
+      JSIException);
+}
+
 //----------------------------------------------------------------------
 // Test that multiple levels of delegation in DecoratedHostObjects works.