fix(runtime): unwrap Proxy receivers and arguments before native dispatch (#485)

* fix(runtime): unwrap Proxy receivers and arguments before native dispatch

Native wrappers wrapped in a JS Proxy (e.g. by Vue's reactive()) expose no
internal fields, so method calls were dispatched as class methods, property
accessors returned undefined, toString returned an object, and passing a
proxy as an argument aborted the process in GetCreationContext.

- tns::UnwrapProxy walks Proxy::GetTarget chains; tns::GetValue resolves
  through it, so every wrapper lookup sees the target.
- MethodCallback, property getter/setter and toString resolve a proxy
  receiver to its native target (or class constructor for methods) and throw
  a TypeError for revoked proxies or non-native targets.
- WriteValue/WriteTypeValue/ToArray unwrap before marshalling and throw a
  TypeError for revoked proxies; ToObject and callback return values treat
  a revoked proxy as nil.
- GetCreationContext call sites fall back to the current context instead of
  asserting when the object has none.

* fix(runtime): key wrapper state by the Proxy target in SetValue/DeleteValue

GetValue resolves through proxies while SetValue and DeleteValue addressed
the object they were handed, so __releaseNativeCounterpart(proxy) deleted
the target's wrapper and then cleared the slot on the Proxy, leaving the
target's internal field dangling. All three now resolve to the same target.

ResolveProxyReceiver accepts a function target only when it carries an
ObjCClass wrapper; a proxied plain function is a TypeError instead of a
static call on the metadata class.

* fix(runtime): zero the whole return slot for empty callback results

ArgConverter::SetValue wrote a single ffi_arg when a JS callback returned
null, undefined or a revoked Proxy, so struct-valued returns handed native
code stale bytes past the first word. Callers now pass cif->rtype->size
and the empty branch clears the full slot.

* fix(runtime): read JavaScript-backed Proxies through their traps

JS-to-native conversion unwrapped every Proxy and read its target, which
bypasses the get/ownKeys/has traps for ordinary objects and arrays, so
native code saw different data than JS. Vue's reactive() get trap
auto-unwraps refs, so dictionaryWithDictionary(reactive({ a: ref(1) }))
produced { a: <RefImpl> } instead of { a: 1 }.

Rule, applied at every conversion site via tns::ProxyReadSource: the
unwrapped target is still used for classification and native-wrapper
lookups (a revoked Proxy still throws a TypeError). When the target is an
ordinary object or Array, the original Proxy is what gets read. Map, Set,
WeakMap/WeakSet, ArrayBuffer/views, Date, RegExp, boxed primitives,
Promises, functions and objects with internal fields stay on the target:
their prototype methods reject a Proxy receiver, and reactive(new Map())
only works through the raw Map.

Sites:
- Interop::WriteValue / WriteTypeValue: array-like -> ArrayAdapter,
  DictionaryAdapter, constant arrays and all struct-from-object
  initializers read the Proxy.
- Interop::ToObject: plain objects hand the Proxy to DictionaryAdapter;
  proxied arrays are snapshotted through the traps before ArrayAdapter
  (which needs a real v8::Array). A snapshot failure is reported like the
  adapters' other read failures and yields nil.
- Interop::ToArray runs Array.prototype.slice on the Proxy and throws a
  NativeScriptException carrying the trap's error instead of asserting.
- Interop::InitializeStruct and ArgConverter::GetInitializerArgs throw a
  catchable error when a field read throws instead of asserting.
- ArgConverter::FindInitializer passes the Proxy to GetInitializerArgs.
- ArgConverter::SetValue (callback return) hands the Proxy to
  DictionaryAdapter; struct returns stay on the target because a throw
  cannot leave the ffi closure.
- DictionaryAdapter remembers the Proxy target that holds its claim, so
  dealloc still finds and frees the claim after the Proxy is revoked.

Author: edusperoni

NativeScript/runtime/AnimationFrame.mm

@@ -225,7 +225,7 @@ void FireFrame(double timestampSeconds) {
       }
       entry->scheduled = false;
       Local<v8::Function> cb = entry->callback.Get(isolate);
-      Local<Context> context = cb->GetCreationContextChecked(v8::Isolate::GetCurrent());
+      Local<Context> context = tns::GetCreationContextOrCurrent(isolate, cb);
       Context::Scope contextScope(context);
       if (entry->raf) {
         Local<Value> argv[] = {v8::Number::New(isolate, performanceMillis)};

NativeScript/runtime/ArgConverter.h

@@ -71,8 +71,10 @@ class ArgConverter {
   static const ProtocolMeta* FindProtocolMeta(Protocol* protocol);
   static void MethodCallback(ffi_cif* cif, void* retValue, void** argValues,
                              void* userData);
+  // returnSize is the ABI size of the return slot (cif->rtype->size); an
+  // empty, null or undefined value zeroes all of it.
   static void SetValue(v8::Local<v8::Context> context, void* retValue,
-                       v8::Local<v8::Value> value,
+                       size_t returnSize, v8::Local<v8::Value> value,
                        const TypeEncoding* typeEncoding);
   // Returns (lazily creating) the per-isolate interop.escapeException brand
   // private stored in Caches. Empty handle if the isolate cache is invalid.

NativeScript/runtime/ArgConverter.mm

@@ -1,5 +1,6 @@
 #include "ArgConverter.h"
 #include <Foundation/Foundation.h>
+#include <algorithm>
 #include <sstream>
 #include "DictionaryAdapter.h"
 #include "Helpers.h"
@@ -438,7 +439,7 @@
       if (!success) {
         memset(retValue, 0, cif->rtype->size);
       } else {
-        ArgConverter::SetValue(context, retValue, result, data->typeEncoding_);
+        ArgConverter::SetValue(context, retValue, cif->rtype->size, result, data->typeEncoding_);
       }
     } else {
       memset(retValue, 0, cif->rtype->size);
@@ -450,15 +451,19 @@
   }
 }
 
-void ArgConverter::SetValue(Local<Context> context, void* retValue, Local<Value> value,
-                            const TypeEncoding* typeEncoding) {
+void ArgConverter::SetValue(Local<Context> context, void* retValue, size_t returnSize,
+                            Local<Value> value, const TypeEncoding* typeEncoding) {
   if (typeEncoding->type == BinaryTypeEncodingType::VoidEncoding) {
     return;
   }
 
+  // Runs inside an ffi closure, where a C++ throw cannot propagate; a revoked
+  // proxy returns nil. libffi sizes the return slot to at least ffi_arg even
+  // for narrower types, and the full slot must be written.
+  Local<Value> original = value;
+  value = tns::UnwrapProxy(value);
   if (value.IsEmpty() || value->IsNullOrUndefined()) {
-    void* nullPtr = nullptr;
-    *(ffi_arg*)retValue = (unsigned long)nullPtr;
+    memset(retValue, 0, std::max(returnSize, sizeof(ffi_arg)));
     return;
   }
 
@@ -540,8 +545,9 @@
         *static_cast<id __strong*>(retValue) = data;
         return;
       } else {
-        id adapter = [[DictionaryAdapter alloc] initWithJSObject:value.As<Object>()
-                                                         isolate:isolate];
+        id adapter = [[DictionaryAdapter alloc]
+            initWithJSObject:tns::ProxyReadSource(original, value).As<Object>()
+                     isolate:isolate];
         memset(retValue, 0, sizeof(id));
         *static_cast<id __strong*>(retValue) = adapter;
         // CFAutorelease(adapter);
@@ -651,7 +657,9 @@
   std::vector<Local<Value>> initializerArgs;
   std::string constructorTokens;
   if (info.Length() == 1 && info[0]->IsObject() && tns::GetValue(isolate, info[0]) == nullptr) {
-    initializerArgs = GetInitializerArgs(info[0].As<Object>(), constructorTokens);
+    Local<Value> initializer = tns::UnwrapProxyOrThrow(isolate, info[0]);
+    initializerArgs = GetInitializerArgs(tns::ProxyReadSource(info[0], initializer).As<Object>(),
+                                         constructorTokens);
   }
 
   std::shared_ptr<Caches> cache = Caches::Get(isolate);
@@ -733,6 +741,8 @@
 
 bool ArgConverter::CanInvoke(Local<Context> context, const TypeEncoding* typeEncoding,
                              Local<Value> arg) {
+  // A revoked proxy matches anything so marshalling reports it as such.
+  arg = tns::UnwrapProxy(arg);
   if (arg.IsEmpty() || arg->IsNullOrUndefined()) {
     return true;
   }
@@ -811,26 +821,27 @@
                                                            std::string& constructorTokens) {
   std::vector<Local<Value>> args;
   constructorTokens = "";
-  Local<Context> context;
-  bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context);
-  tns::Assert(success);
   Isolate* isolate = v8::Isolate::GetCurrent();
+  Local<Context> context = tns::GetCreationContextOrCurrent(isolate, obj);
   Local<v8::Array> properties;
-  if (obj->GetOwnPropertyNames(context).ToLocal(&properties)) {
-    std::stringstream ss;
-    for (uint32_t i = 0; i < properties->Length(); i++) {
-      Local<Value> propertyName;
-      if (properties->Get(context, i).ToLocal(&propertyName)) {
-        std::string name = tns::ToString(isolate, propertyName);
-        ss << name << ":";
-        Local<Value> propertyValue;
-        bool ok = obj->Get(context, propertyName).ToLocal(&propertyValue);
-        tns::Assert(ok, isolate);
-        args.push_back(propertyValue);
+  TryCatch tc(isolate);
+  if (!obj->GetOwnPropertyNames(context).ToLocal(&properties)) {
+    throw NativeScriptException(isolate, tc, "Cannot read initializer object");
+  }
+  std::stringstream ss;
+  for (uint32_t i = 0; i < properties->Length(); i++) {
+    Local<Value> propertyName;
+    if (properties->Get(context, i).ToLocal(&propertyName)) {
+      std::string name = tns::ToString(isolate, propertyName);
+      ss << name << ":";
+      Local<Value> propertyValue;
+      if (!obj->Get(context, propertyName).ToLocal(&propertyValue)) {
+        throw NativeScriptException(isolate, tc, "Cannot read initializer object");
       }
+      args.push_back(propertyValue);
     }
-    constructorTokens = ss.str();
   }
+  constructorTokens = ss.str();
 
   return args;
 }

NativeScript/runtime/ClassBuilder.mm

@@ -931,7 +931,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
             memset(retValue, 0, cif->rtype->size);
           } else {
             const TypeEncoding* typeEncoding = context->meta_->getter()->encodings()->first();
-            ArgConverter::SetValue(v8Context, retValue, res, typeEncoding);
+            ArgConverter::SetValue(v8Context, retValue, cif->rtype->size, res, typeEncoding);
           }
         }
         if (pendingThrow != nil) {

NativeScript/runtime/DictionaryAdapter.mm

@@ -226,6 +226,9 @@ @implementation DictionaryAdapter {
   // both isolate states; the field compare below guards the isolate-alive
   // path against a slot someone else overwrote.
   ObjCDataWrapper* dataWrapper_;
+  // The Proxy target holding dataWrapper_ when object_ is a Proxy: a Proxy
+  // revoked later no longer leads to it, and the claim must still be found.
+  Persistent<Value>* claimTarget_;
 }
 
 - (instancetype)initWithJSObject:(Local<Object>)jsObject isolate:(Isolate*)isolate {
@@ -240,6 +243,12 @@ - (instancetype)initWithJSObject:(Local<Object>)jsObject isolate:(Isolate*)isola
       self->dataWrapper_ = new ObjCDataWrapper(self);
       self->dataWrapper_->MarkAdapterClaim();
       tns::SetValue(isolate, jsObject, self->dataWrapper_);
+      if (jsObject->IsProxy()) {
+        Local<Value> target = tns::UnwrapProxy(jsObject);
+        if (!target.IsEmpty()) {
+          self->claimTarget_ = new Persistent<Value>(isolate, target);
+        }
+      }
     }
   }
 
@@ -368,7 +377,8 @@ - (void)dealloc {
     // isolate is gone the field can no longer be read, so the claim is dropped
     // rather than freed blind.
     if (dataWrapper_ != nullptr) {
-      Local<Value> value = self->object_->Get(isolate);
+      Local<Value> value =
+          claimTarget_ != nullptr ? claimTarget_->Get(isolate) : self->object_->Get(isolate);
       if (tns::GetValue(isolate, value) == dataWrapper_) {
         tns::DeleteValue(isolate, value);
         delete dataWrapper_;
@@ -379,6 +389,9 @@ - (void)dealloc {
     // vended by -keyEnumerator hold this adapter alive, so nothing can be
     // reading the handle by the time this runs.
     self->object_->Reset();
+    if (claimTarget_ != nullptr) {
+      claimTarget_->Reset();
+    }
   } else if (dataWrapper_ != nullptr) {
     // The isolate is gone, and with it the JS object and every reader of the
     // claim; no other path deletes one (__releaseNativeCounterpart leaves
@@ -389,6 +402,8 @@ - (void)dealloc {
     dataWrapper_ = nullptr;
   }
   self->object_ = nullptr;
+  delete claimTarget_;
+  claimTarget_ = nullptr;
   delete self->wrapper_;
 
   [super dealloc];

NativeScript/runtime/Helpers.h

@@ -258,6 +258,38 @@ void SetPrivateValue(const v8::Local<v8::Object>& obj, const v8::Local<v8::Strin
 v8::Local<v8::Value> GetPrivateValue(const v8::Local<v8::Object>& obj,
                                      const v8::Local<v8::String>& propName);
 
+// Follows a Proxy chain to its innermost target; empty when any link is
+// revoked. Non-proxies come back unchanged.
+inline v8::Local<v8::Value> UnwrapProxy(v8::Local<v8::Value> value) {
+  while (!value.IsEmpty() && value->IsProxy()) {
+    v8::Local<v8::Proxy> proxy = value.As<v8::Proxy>();
+    if (proxy->IsRevoked()) {
+      return v8::Local<v8::Value>();
+    }
+    value = proxy->GetTarget();
+  }
+  return value;
+}
+
+// UnwrapProxy for values crossing into native code: a revoked proxy throws a
+// NativeScriptException carrying a TypeError.
+v8::Local<v8::Value> UnwrapProxyOrThrow(v8::Isolate* isolate, v8::Local<v8::Value> value);
+
+// The value native code should read properties and elements from, given the
+// original value and its unwrapped target. A Proxy over an ordinary object or
+// Array is read through its traps, so native code sees what JS sees. Any other
+// target is read directly: native wrappers keep their state on the target, and
+// objects with internal slots (Map, Set, buffers, Date, RegExp, boxed
+// primitives, functions) reject a Proxy receiver in their own methods.
+v8::Local<v8::Value> ProxyReadSource(v8::Local<v8::Value> original, v8::Local<v8::Value> target);
+
+// The object's creation context, or the isolate's current (else main) context
+// for objects that have none, such as proxies.
+v8::Local<v8::Context> GetCreationContextOrCurrent(v8::Isolate* isolate,
+                                                   const v8::Local<v8::Object>& obj);
+
+// Set/Get/DeleteValue all resolve through proxies: wrapper state is keyed by
+// the Proxy target, so a proxied wrapper yields its target's wrapper.
 void SetValue(v8::Isolate* isolate, const v8::Local<v8::Object>& obj, BaseDataWrapper* value);
 BaseDataWrapper* GetValue(v8::Isolate* isolate, const v8::Local<v8::Value>& val);
 

NativeScript/runtime/Helpers.mm

@@ -198,24 +198,64 @@
   return ok;
 }
 
+Local<Value> tns::UnwrapProxyOrThrow(Isolate* isolate, Local<Value> value) {
+  if (value.IsEmpty() || !value->IsProxy()) {
+    return value;
+  }
+  Local<Value> target = tns::UnwrapProxy(value);
+  if (target.IsEmpty()) {
+    std::string message = "Cannot pass a revoked Proxy to native code";
+    throw NativeScriptException(
+        isolate, v8::Exception::TypeError(tns::ToV8String(isolate, message)), message);
+  }
+  return target;
+}
+
+Local<Value> tns::ProxyReadSource(Local<Value> original, Local<Value> target) {
+  if (original.IsEmpty() || !original->IsProxy() || target.IsEmpty() || !target->IsObject()) {
+    return target;
+  }
+  if (target->IsArray()) {
+    return original;
+  }
+  if (target->IsFunction() || target->IsMap() || target->IsSet() || target->IsWeakMap() ||
+      target->IsWeakSet() || target->IsArrayBuffer() || target->IsArrayBufferView() ||
+      target->IsSharedArrayBuffer() || target->IsDate() || target->IsRegExp() ||
+      target->IsStringObject() || target->IsNumberObject() || target->IsBooleanObject() ||
+      target->IsBigIntObject() || target->IsSymbolObject() || target->IsPromise() ||
+      target.As<Object>()->InternalFieldCount() > 0) {
+    return target;
+  }
+  return original;
+}
+
+Local<Context> tns::GetCreationContextOrCurrent(Isolate* isolate, const Local<Object>& obj) {
+  Local<Context> context;
+  if (obj->GetCreationContext(isolate).ToLocal(&context)) {
+    return context;
+  }
+  context = isolate->GetCurrentContext();
+  if (context.IsEmpty()) {
+    context = Caches::Get(isolate)->GetContext();
+  }
+  return context;
+}
+
 void tns::SetPrivateValue(const Local<Object>& obj, const Local<v8::String>& propName,
                           const Local<Value>& value) {
-  Local<Context> context;
-  bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context);
-  tns::Assert(success);
   Isolate* isolate = v8::Isolate::GetCurrent();
+  Local<Context> context = tns::GetCreationContextOrCurrent(isolate, obj);
   Local<Private> privateKey = Private::ForApi(isolate, propName);
 
+  bool success = false;
   if (!obj->SetPrivate(context, privateKey, value).To(&success) || !success) {
     tns::Assert(false, isolate);
   }
 }
 
 Local<Value> tns::GetPrivateValue(const Local<Object>& obj, const Local<v8::String>& propName) {
-  Local<Context> context;
-  bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context);
-  tns::Assert(success);
   Isolate* isolate = v8::Isolate::GetCurrent();
+  Local<Context> context = tns::GetCreationContextOrCurrent(isolate, obj);
   Local<Private> privateKey = Private::ForApi(isolate, propName);
 
   Maybe<bool> hasPrivate = obj->HasPrivate(context, privateKey);
@@ -243,10 +283,18 @@
   return false;
 }
 
-void tns::SetValue(Isolate* isolate, const Local<Object>& obj, BaseDataWrapper* value) {
-  if (obj.IsEmpty() || obj->IsNullOrUndefined()) {
+void tns::SetValue(Isolate* isolate, const Local<Object>& val, BaseDataWrapper* value) {
+  if (val.IsEmpty() || val->IsNullOrUndefined()) {
+    return;
+  }
+
+  // Wrapper state lives on the Proxy target so that it is found by the same
+  // identity GetValue resolves to.
+  Local<Value> target = tns::UnwrapProxy(val);
+  if (target.IsEmpty()) {
     return;
   }
+  Local<Object> obj = target.As<Object>();
 
   Local<External> ext = External::New(isolate, value, v8::kExternalPointerTypeTagDefault);
 
@@ -263,6 +311,13 @@
   }
 
   Local<Object> obj = val.As<Object>();
+  if (obj->IsProxy()) {
+    Local<Value> target = tns::UnwrapProxy(obj);
+    if (target.IsEmpty()) {
+      return nullptr;
+    }
+    obj = target.As<Object>();
+  }
   if (obj->InternalFieldCount() > 0) {
     Local<Value> field = obj->GetInternalField(0).As<v8::Value>();
     if (field.IsEmpty() || field->IsNullOrUndefined() || !field->IsExternal()) {
@@ -502,11 +557,12 @@ void WriteDebugLine(tns::LogCategory category, const char* message) {
 }
 
 void tns::DeleteValue(Isolate* isolate, const Local<Value>& val) {
-  if (val.IsEmpty() || val->IsNullOrUndefined() || !val->IsObject()) {
+  Local<Value> target = tns::UnwrapProxy(val);
+  if (target.IsEmpty() || target->IsNullOrUndefined() || !target->IsObject()) {
     return;
   }
 
-  Local<Object> obj = val.As<Object>();
+  Local<Object> obj = target.As<Object>();
   if (obj->InternalFieldCount() > 0) {
     obj->SetInternalField(0, v8::Undefined(isolate));
     return;
@@ -518,12 +574,10 @@ void WriteDebugLine(tns::LogCategory category, const char* message) {
     return;
   }
 
-  Local<Context> context;
-  bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context);
-  tns::Assert(success, isolate);
+  Local<Context> context = tns::GetCreationContextOrCurrent(isolate, obj);
   Local<Private> privateKey = Private::ForApi(isolate, metadataKey);
 
-  success = obj->DeletePrivate(context, privateKey).FromMaybe(false);
+  bool success = obj->DeletePrivate(context, privateKey).FromMaybe(false);
   tns::Assert(success, isolate);
 }
 
@@ -537,18 +591,21 @@ void WriteDebugLine(tns::LogCategory category, const char* message) {
 }
 
 bool tns::IsArrayOrArrayLike(Isolate* isolate, const Local<Value>& value) {
-  if (value->IsArray()) {
+  Local<Value> target = tns::UnwrapProxy(value);
+  if (target.IsEmpty()) {
+    return false;
+  }
+
+  if (target->IsArray()) {
     return true;
   }
 
-  if (!value->IsObject()) {
+  if (!target->IsObject()) {
     return false;
   }
 
-  Local<Object> obj = value.As<Object>();
-  Local<Context> context;
-  bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context);
-  tns::Assert(success, isolate);
+  Local<Object> obj = target.As<Object>();
+  Local<Context> context = tns::GetCreationContextOrCurrent(isolate, obj);
   return obj->Has(context, ToV8String(isolate, "length")).FromMaybe(false);
 }