Don't use Nan::SetNamedPropertyHandler (#2172)

Nan's wrapper for these functions incorrectly asserts that the property name
argument is a `v8::String`, while it's actually a `v8::Name` (a superclass of
String) and depending on the whim of the optimizer it may be a different
concrete subclass. This resulted in the realm-js-private tests (and some
non-test code) crashing with the error "FATAL ERROR: v8::String::Cast Could not
convert to string" when run with a debug build of the library.

Fortunately the last time this API changed was in node 0.12 so there's no
actual need to use Nan's wrapper.

Author: tgoyne

CHANGELOG.md

@@ -6,6 +6,7 @@ x.x.x Release notes (yyyy-MM-dd)
 ### Fixed
 * <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?)
 * ReactNative for Android no longer uses deprecated methods and can build using Gradle 5.0 and above. ([#1995](https://github.com/realm/realm-js/issues/1995))
+* Fix occasional "FATAL ERROR: v8::String::Cast Could not convert to string" crashes when reading a property from a Realm object. ([#2172](https://github.com/realm/realm-js/pull/2172), since 2.19.0)
 
 ### Compatibility
 * Realm Object Server: 3.11.0 or later.

src/node/node_class.hpp

@@ -83,7 +83,7 @@ class ObjectWrap : public Nan::ObjectWrap {
     static void setup_property(v8::Local<TargetType>, const std::string &, const PropertyType &);
 
     static void get_indexes(const Nan::PropertyCallbackInfo<v8::Array>&);
-    static void set_property(v8::Local<v8::String>, v8::Local<v8::Value>, const Nan::PropertyCallbackInfo<v8::Value>&);
+    static void set_property(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<v8::Value>&);
 
     static void set_readonly_property(v8::Local<v8::String> property, v8::Local<v8::Value> value, const Nan::PropertyCallbackInfo<void>& info) {
         std::string message = std::string("Cannot assign to read only property '") + std::string(String(property)) + "'";
@@ -95,7 +95,7 @@ class ObjectWrap : public Nan::ObjectWrap {
         Nan::ThrowError(message.c_str());
     }
 
-    static void get_nonexistent_property(v8::Local<v8::String>, const Nan::PropertyCallbackInfo<v8::Value>&) {
+    static void get_nonexistent_property(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&) {
         // Do nothing. This function exists only to prevent a crash where it is used.
     }
 };
@@ -188,7 +188,7 @@ inline v8::Local<v8::FunctionTemplate> ObjectWrap<ClassType>::create_template()
     if (s_class.string_accessor.getter || s_class.index_accessor.getter || s_class.index_accessor.setter) {
         // Use our own wrapper for the setter since we want to throw for negative indices.
         auto &string_accessor = s_class.string_accessor;
-        Nan::SetNamedPropertyHandler(instance_tpl, string_accessor.getter ? string_accessor.getter : get_nonexistent_property, set_property, 0, 0, string_accessor.enumerator);
+        instance_tpl->SetNamedPropertyHandler(string_accessor.getter ? string_accessor.getter : get_nonexistent_property, set_property, 0, 0, string_accessor.enumerator);
     }
 
     return scope.Escape(tpl);
@@ -270,7 +270,7 @@ inline void ObjectWrap<ClassType>::get_indexes(const Nan::PropertyCallbackInfo<v
 }
 
 template<typename ClassType>
-inline void ObjectWrap<ClassType>::set_property(v8::Local<v8::String> property, v8::Local<v8::Value> value, const Nan::PropertyCallbackInfo<v8::Value>& info) {
+inline void ObjectWrap<ClassType>::set_property(v8::Local<v8::String> property, v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<v8::Value>& info) {
     if (s_class.index_accessor.getter || s_class.index_accessor.setter) {
         try {
             // Negative indices are passed into this string property interceptor, so check for them here.
@@ -366,7 +366,7 @@ void wrap(uint32_t index, v8::Local<v8::Value> value, const Nan::PropertyCallbac
 }
 
 template<node::StringPropertyType::GetterType F>
-void wrap(v8::Local<v8::String> property, const Nan::PropertyCallbackInfo<v8::Value>& info) {
+void wrap(v8::Local<v8::String> property, const v8::PropertyCallbackInfo<v8::Value>& info) {
     v8::Isolate* isolate = info.GetIsolate();
     node::ReturnValue return_value(info.GetReturnValue());
     try {
@@ -378,7 +378,7 @@ void wrap(v8::Local<v8::String> property, const Nan::PropertyCallbackInfo<v8::Va
 }
 
 template<node::StringPropertyType::SetterType F>
-void wrap(v8::Local<v8::String> property, v8::Local<v8::Value> value, const Nan::PropertyCallbackInfo<v8::Value>& info) {
+void wrap(v8::Local<v8::String> property, v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<v8::Value>& info) {
     v8::Isolate* isolate = info.GetIsolate();
     try {
         if (F(isolate, info.This(), property, value)) {
@@ -392,7 +392,7 @@ void wrap(v8::Local<v8::String> property, v8::Local<v8::Value> value, const Nan:
 }
 
 template<node::StringPropertyType::EnumeratorType F>
-void wrap(const Nan::PropertyCallbackInfo<v8::Array>& info) {
+void wrap(const v8::PropertyCallbackInfo<v8::Array>& info) {
     auto names = F(info.GetIsolate(), info.This());
     int count = (int)names.size();
     v8::Local<v8::Array> array = Nan::New<v8::Array>(count);

src/node/node_return_value.hpp

@@ -29,6 +29,7 @@ class ReturnValue<node::Types> {
 
   public:
     ReturnValue(Nan::ReturnValue<v8::Value> value) : m_value(value) {}
+    ReturnValue(v8::ReturnValue<v8::Value> value) : m_value(value) {}
 
     void set(const v8::Local<v8::Value> &value) {
         m_value.Set(value);

src/node/node_types.hpp

@@ -49,9 +49,9 @@ struct Types {
     using PropertySetterCallback = Nan::SetterCallback;
     using IndexPropertyGetterCallback = Nan::IndexGetterCallback;
     using IndexPropertySetterCallback = Nan::IndexSetterCallback;
-    using StringPropertyGetterCallback = Nan::PropertyGetterCallback;
-    using StringPropertySetterCallback = Nan::PropertySetterCallback;
-    using StringPropertyEnumeratorCallback = Nan::PropertyEnumeratorCallback;
+    using StringPropertyGetterCallback = v8::NamedPropertyGetterCallback;
+    using StringPropertySetterCallback = v8::NamedPropertySetterCallback;
+    using StringPropertyEnumeratorCallback = v8::NamedPropertyEnumeratorCallback;
 };
 
 template<typename ClassType>