diff --git a/NativeScript/runtime/AnimationFrame.mm b/NativeScript/runtime/AnimationFrame.mm index 09994f39..43bb0b37 100644 --- a/NativeScript/runtime/AnimationFrame.mm +++ b/NativeScript/runtime/AnimationFrame.mm @@ -225,7 +225,7 @@ void FireFrame(double timestampSeconds) { } entry->scheduled = false; Local cb = entry->callback.Get(isolate); - Local context = cb->GetCreationContextChecked(v8::Isolate::GetCurrent()); + Local context = tns::GetCreationContextOrCurrent(isolate, cb); Context::Scope contextScope(context); if (entry->raf) { Local argv[] = {v8::Number::New(isolate, performanceMillis)}; diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d8674990..cdc251c0 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -452,6 +452,9 @@ return; } + // Runs inside an ffi closure, where a C++ throw cannot propagate; a revoked + // proxy returns nil. + value = tns::UnwrapProxy(value); if (value.IsEmpty() || value->IsNullOrUndefined()) { void* nullPtr = nullptr; *(ffi_arg*)retValue = (unsigned long)nullPtr; @@ -647,7 +650,8 @@ std::vector> initializerArgs; std::string constructorTokens; if (info.Length() == 1 && info[0]->IsObject() && tns::GetValue(isolate, info[0]) == nullptr) { - initializerArgs = GetInitializerArgs(info[0].As(), constructorTokens); + Local initializer = tns::UnwrapProxyOrThrow(isolate, info[0]); + initializerArgs = GetInitializerArgs(initializer.As(), constructorTokens); } std::shared_ptr cache = Caches::Get(isolate); @@ -729,6 +733,8 @@ bool ArgConverter::CanInvoke(Local context, const TypeEncoding* typeEncoding, Local arg) { + // A revoked proxy matches anything so marshalling reports it as such. + arg = tns::UnwrapProxy(arg); if (arg.IsEmpty() || arg->IsNullOrUndefined()) { return true; } @@ -807,10 +813,8 @@ std::string& constructorTokens) { std::vector> args; constructorTokens = ""; - Local context; - bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success); Isolate* isolate = v8::Isolate::GetCurrent(); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); Local properties; if (obj->GetOwnPropertyNames(context).ToLocal(&properties)) { std::stringstream ss; diff --git a/NativeScript/runtime/Helpers.h b/NativeScript/runtime/Helpers.h index 5b48ade5..1a70b1a3 100644 --- a/NativeScript/runtime/Helpers.h +++ b/NativeScript/runtime/Helpers.h @@ -258,7 +258,30 @@ void SetPrivateValue(const v8::Local& obj, const v8::Local GetPrivateValue(const v8::Local& obj, const v8::Local& propName); +// Follows a Proxy chain to its innermost target; empty when any link is +// revoked. Non-proxies come back unchanged. +inline v8::Local UnwrapProxy(v8::Local value) { + while (!value.IsEmpty() && value->IsProxy()) { + v8::Local proxy = value.As(); + if (proxy->IsRevoked()) { + return v8::Local(); + } + value = proxy->GetTarget(); + } + return value; +} + +// UnwrapProxy for values crossing into native code: a revoked proxy throws a +// NativeScriptException carrying a TypeError. +v8::Local UnwrapProxyOrThrow(v8::Isolate* isolate, v8::Local value); + +// The object's creation context, or the isolate's current (else main) context +// for objects that have none, such as proxies. +v8::Local GetCreationContextOrCurrent(v8::Isolate* isolate, + const v8::Local& obj); + void SetValue(v8::Isolate* isolate, const v8::Local& obj, BaseDataWrapper* value); +// Resolves through proxies: a proxied wrapper yields its target's wrapper. BaseDataWrapper* GetValue(v8::Isolate* isolate, const v8::Local& val); // What happens when JS touches a wrapper whose native counterpart has already diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm index ef35ddaa..98d2493d 100644 --- a/NativeScript/runtime/Helpers.mm +++ b/NativeScript/runtime/Helpers.mm @@ -198,24 +198,46 @@ return ok; } +Local tns::UnwrapProxyOrThrow(Isolate* isolate, Local value) { + if (value.IsEmpty() || !value->IsProxy()) { + return value; + } + Local 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 tns::GetCreationContextOrCurrent(Isolate* isolate, const Local& obj) { + Local 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& obj, const Local& propName, const Local& value) { - Local context; - bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success); Isolate* isolate = v8::Isolate::GetCurrent(); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); Local privateKey = Private::ForApi(isolate, propName); + bool success = false; if (!obj->SetPrivate(context, privateKey, value).To(&success) || !success) { tns::Assert(false, isolate); } } Local tns::GetPrivateValue(const Local& obj, const Local& propName) { - Local context; - bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success); Isolate* isolate = v8::Isolate::GetCurrent(); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); Local privateKey = Private::ForApi(isolate, propName); Maybe hasPrivate = obj->HasPrivate(context, privateKey); @@ -263,6 +285,13 @@ } Local obj = val.As(); + if (obj->IsProxy()) { + Local target = tns::UnwrapProxy(obj); + if (target.IsEmpty()) { + return nullptr; + } + obj = target.As(); + } if (obj->InternalFieldCount() > 0) { Local field = obj->GetInternalField(0).As(); if (field.IsEmpty() || field->IsNullOrUndefined() || !field->IsExternal()) { @@ -518,12 +547,10 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { return; } - Local context; - bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success, isolate); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); Local 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 +564,21 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { } bool tns::IsArrayOrArrayLike(Isolate* isolate, const Local& value) { - if (value->IsArray()) { + Local target = tns::UnwrapProxy(value); + if (target.IsEmpty()) { + return false; + } + + if (target->IsArray()) { return true; } - if (!value->IsObject()) { + if (!target->IsObject()) { return false; } - Local obj = value.As(); - Local context; - bool success = obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success, isolate); + Local obj = target.As(); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); return obj->Has(context, ToV8String(isolate, "length")).FromMaybe(false); } diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 304535be..dbc61dcc 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -219,6 +219,7 @@ inline bool isBool() { void Interop::WriteTypeValue(Local context, BaseDataWrapper* typeWrapper, void* dest, Local arg) { Isolate* isolate = v8::Isolate::GetCurrent(); + arg = tns::UnwrapProxyOrThrow(isolate, arg); ValueCache argHelper(arg); bool isEmptyOrUndefined = arg.IsEmpty() || arg->IsNullOrUndefined(); bool success = false; @@ -258,6 +259,9 @@ inline bool isBool() { void Interop::WriteValue(Local context, const TypeEncoding* typeEncoding, void* dest, Local arg) { Isolate* isolate = v8::Isolate::GetCurrent(); + // Every branch below inspects the value's own type and internal fields, + // which a Proxy hides. + arg = tns::UnwrapProxyOrThrow(isolate, arg); ExecuteWriteValueDebugValidationsIfInDebug(context, typeEncoding, dest, arg); ValueCache argHelper(arg); if (arg.IsEmpty() || arg->IsNullOrUndefined()) { @@ -729,6 +733,9 @@ inline bool isBool() { id Interop::ToObject(Local context, v8::Local arg) { Isolate* isolate = v8::Isolate::GetCurrent(); + // Runs inside adapter callbacks invoked by native code, where a C++ throw + // cannot propagate; a revoked proxy reads as nil. + arg = tns::UnwrapProxy(arg); if (arg.IsEmpty() || arg->IsNullOrUndefined()) { return nil; } else if (tns::IsString(arg)) { @@ -1559,14 +1566,13 @@ inline bool isBool() { } Local Interop::ToArray(Local object) { + Isolate* isolate = v8::Isolate::GetCurrent(); + object = tns::UnwrapProxyOrThrow(isolate, object).As(); if (object->IsArray()) { return object.As(); } - Local context; - bool success = object->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success); - Isolate* isolate = v8::Isolate::GetCurrent(); + Local context = tns::GetCreationContextOrCurrent(isolate, object); Local sliceFunc; auto cache = Caches::Get(isolate); @@ -1596,7 +1602,7 @@ inline bool isBool() { Local sliceArgs[1]{object}; Local result; - success = sliceFunc->Call(context, object, 1, sliceArgs).ToLocal(&result); + bool success = sliceFunc->Call(context, object, 1, sliceArgs).ToLocal(&result); tns::Assert(success, isolate); return result.As(); diff --git a/NativeScript/runtime/MetadataBuilder.mm b/NativeScript/runtime/MetadataBuilder.mm index 604e4933..194d895f 100644 --- a/NativeScript/runtime/MetadataBuilder.mm +++ b/NativeScript/runtime/MetadataBuilder.mm @@ -19,6 +19,42 @@ namespace tns { +namespace { + +// Swaps a Proxy receiver for the native object at the end of its chain, so the +// call dispatches as if made on that object directly. On failure a TypeError is +// thrown and false returned. `allowClass` admits a class constructor target. +bool ResolveProxyReceiver(Isolate* isolate, Local& receiver, const char* action, + const char* memberName, bool allowClass) { + if (!receiver->IsProxy()) { + return true; + } + + Local target = tns::UnwrapProxy(receiver); + const char* reason = nullptr; + if (target.IsEmpty()) { + reason = "on a revoked Proxy"; + } else if (!(allowClass && target->IsFunction())) { + BaseDataWrapper* wrapper = + target.As()->InternalFieldCount() > 0 ? tns::GetValue(isolate, target) : nullptr; + if (wrapper == nullptr || (wrapper->Type() != WrapperType::ObjCObject && + wrapper->Type() != WrapperType::ObjCAllocObject)) { + reason = "on a Proxy whose target is not a native object"; + } + } + + if (reason != nullptr) { + std::string message = std::string("Cannot ") + action + " '" + memberName + "' " + reason; + isolate->ThrowException(Exception::TypeError(tns::ToV8String(isolate, message))); + return false; + } + + receiver = target.As(); + return true; +} + +} // namespace + void MetadataBuilder::RegisterConstantsOnGlobalObject(Isolate* isolate, Local globalTemplate, bool isWorkerThread) { @@ -251,14 +287,14 @@ throw NativeScriptException( Local context = isolate->GetCurrentContext(); tns::Assert(info.Length() == 2, isolate); - Local arg1 = info[0].As(); - Local arg2 = info[1].As(); - - if (arg1.IsEmpty() || !arg1->IsObject() || arg1->IsNullOrUndefined() || arg2.IsEmpty() || - !arg2->IsObject() || arg2->IsNullOrUndefined()) { + Local value1 = tns::UnwrapProxy(info[0]); + Local value2 = tns::UnwrapProxy(info[1]); + if (value1.IsEmpty() || !value1->IsObject() || value2.IsEmpty() || !value2->IsObject()) { info.GetReturnValue().Set(false); return; } + Local arg1 = value1.As(); + Local arg2 = value2.As(); BaseDataWrapper* wrapper = tns::GetValue(isolate, info.This()); if (wrapper == nullptr || wrapper->Type() != WrapperType::StructType) { @@ -472,6 +508,9 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta void MetadataBuilder::ToStringFunctionCallback(const FunctionCallbackInfo& info) { Isolate* isolate = info.GetIsolate(); Local thiz = info.This(); + if (!ResolveProxyReceiver(isolate, thiz, "call native method", "toString", false)) { + return; + } BaseDataWrapper* wrapper = tns::GetValue(isolate, thiz); if (wrapper == nullptr || wrapper->Type() != WrapperType::ObjCObject) { @@ -764,7 +803,12 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta CacheItem* item = static_cast*>( info.Data().As()->Value(v8::kExternalPointerTypeTagDefault)); - bool instanceMethod = info.This()->InternalFieldCount() > 0; + Local thiz = info.This(); + if (!ResolveProxyReceiver(isolate, thiz, "call native method", item->meta_->jsName(), true)) { + return; + } + + bool instanceMethod = thiz->InternalFieldCount() > 0; V8FunctionCallbackArgs args(info); // Only the class-side call rewrites the name, so the common path reads @@ -772,7 +816,6 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta const std::string* className = &item->className_; std::string classWrapperName; - Local thiz = info.This(); if (thiz->IsFunction()) { if (BaseDataWrapper* wrapper = tns::GetValue(isolate, thiz)) { ObjCClassWrapper* classWrapper = static_cast(wrapper); @@ -784,7 +827,7 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta Local context = isolate->GetCurrentContext(); Local result = instanceMethod - ? MetadataBuilder::InvokeMethod(context, item->meta_, info.This(), args, *className, true) + ? MetadataBuilder::InvokeMethod(context, item->meta_, thiz, args, *className, true) : MetadataBuilder::InvokeMethod(context, item->meta_, Local(), args, *className, true); @@ -794,15 +837,19 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta } void MetadataBuilder::PropertyGetterCallback(const FunctionCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + CacheItem* item = static_cast*>( + info.Data().As()->Value(v8::kExternalPointerTypeTagDefault)); Local receiver = info.This(); + if (!ResolveProxyReceiver(isolate, receiver, "read native property", item->meta_->jsName(), + false)) { + return; + } if (receiver->InternalFieldCount() < 1) { return; } - Isolate* isolate = info.GetIsolate(); - CacheItem* item = static_cast*>( - info.Data().As()->Value(v8::kExternalPointerTypeTagDefault)); if (!item->meta_->hasGetter()) { Local error = Exception::Error(tns::ToV8String(isolate, "Property is not readable.")); isolate->ThrowException(error); @@ -830,6 +877,10 @@ NamedPropertyHandlerConfiguration config(nullptr, MetadataBuilder::SwizzledInsta } Local receiver = info.This(); + if (!ResolveProxyReceiver(isolate, receiver, "set native property", item->meta_->jsName(), + false)) { + return; + } Local value = info[0]; V8SimpleValueArgs args(value); Local context = isolate->GetCurrentContext(); diff --git a/NativeScript/runtime/Reference.cpp b/NativeScript/runtime/Reference.cpp index 316326c5..d87096d1 100644 --- a/NativeScript/runtime/Reference.cpp +++ b/NativeScript/runtime/Reference.cpp @@ -397,10 +397,6 @@ void Reference::RegisterToStringMethod(Local context, } Reference::DataPair Reference::GetDataPair(Local obj) { - Local context; - bool success = - obj->GetCreationContext(v8::Isolate::GetCurrent()).ToLocal(&context); - tns::Assert(success); Isolate* isolate = v8::Isolate::GetCurrent(); BaseDataWrapper* wrapper = tns::GetValueOrReport(isolate, obj, "Reference indexed access"); diff --git a/NativeScript/runtime/Timers.cpp b/NativeScript/runtime/Timers.cpp index 773746fd..353fd587 100644 --- a/NativeScript/runtime/Timers.cpp +++ b/NativeScript/runtime/Timers.cpp @@ -207,7 +207,7 @@ class TimerState : public OrderedTaskSource { v8::Local cb = task->callback_.Get(isolate); v8::Local context = - cb->GetCreationContextChecked(v8::Isolate::GetCurrent()); + tns::GetCreationContextOrCurrent(isolate, cb); Context::Scope context_scope(context); int argc = task->args_ ? static_cast(task->args_->size()) : 0; if (argc > 0) { diff --git a/TestRunner/app/tests/ProxyReceiverTests.js b/TestRunner/app/tests/ProxyReceiverTests.js new file mode 100644 index 00000000..9a90d02e --- /dev/null +++ b/TestRunner/app/tests/ProxyReceiverTests.js @@ -0,0 +1,162 @@ +describe(module.id, function () { + function findAccessor(proto, name) { + while (proto) { + var descriptor = Object.getOwnPropertyDescriptor(proto, name); + if (descriptor) { + return descriptor; + } + proto = Object.getPrototypeOf(proto); + } + return undefined; + } + + it("calls an instance method on the proxy target", function () { + var target = NSMutableString.alloc().init(); + var proxy = new Proxy(target, {}); + + proxy.appendString("abc"); + + expect(target.toString()).toBe("abc"); + expect(proxy.length).toBe(3); + }); + + it("calls an instance method through nested proxies", function () { + var target = NSMutableString.alloc().init(); + var proxy = new Proxy(new Proxy(target, {}), {}); + + proxy.appendString("ab"); + proxy.appendString("c"); + + expect(target.toString()).toBe("abc"); + }); + + it("reads a native property through the proxy", function () { + var target = NSMutableArray.alloc().init(); + target.addObject(1); + target.addObject(2); + var proxy = new Proxy(target, {}); + + expect(proxy.count).toBe(2); + }); + + it("writes a native property through the proxy", function () { + var target = NSOperation.alloc().init(); + var proxy = new Proxy(target, {}); + + proxy.name = "proxied"; + + expect(target.name).toBe("proxied"); + expect(proxy.name).toBe("proxied"); + }); + + it("converts the proxy to the native description", function () { + var target = NSMutableString.stringWithString("hello"); + var proxy = new Proxy(target, {}); + + expect(String(proxy)).toBe("hello"); + expect("" + proxy).toBe("hello"); + expect(proxy.toString()).toBe("hello"); + }); + + it("calls a class method through a proxied class constructor", function () { + var proxy = new Proxy(NSString, {}); + + var result = proxy.stringWithString("x"); + + expect(result instanceof NSString).toBe(true); + expect(result.toString()).toBe("x"); + }); + + it("passes a proxied native object as a method argument", function () { + var array = NSMutableArray.alloc().init(); + var object = NSObject.alloc().init(); + + array.addObject(new Proxy(object, {})); + + expect(array.count).toBe(1); + expect(array.objectAtIndex(0)).toBe(object); + }); + + it("passes a proxied JS array of native objects as an NSArray", function () { + var a = NSObject.alloc().init(); + var b = NSObject.alloc().init(); + + var array = NSArray.arrayWithArray(new Proxy([a, b], {})); + + expect(array.count).toBe(2); + expect(array.objectAtIndex(0)).toBe(a); + expect(array.objectAtIndex(1)).toBe(b); + }); + + it("passes a proxied plain object as an NSDictionary", function () { + var dictionary = NSDictionary.dictionaryWithDictionary(new Proxy({ key: "value" }, {})); + + expect(dictionary.count).toBe(1); + expect(dictionary.objectForKey("key")).toBe("value"); + }); + + it("passes proxied structs and struct initializers by value", function () { + var fromStruct = NSValue.valueWithRange(new Proxy(NSMakeRange(1, 2), {})); + expect(fromStruct.rangeValue.location).toBe(1); + expect(fromStruct.rangeValue.length).toBe(2); + + var fromObject = NSValue.valueWithRange(new Proxy({ location: 3, length: 4 }, {})); + expect(fromObject.rangeValue.location).toBe(3); + expect(fromObject.rangeValue.length).toBe(4); + }); + + it("throws a TypeError for a revoked proxy receiver", function () { + var revocable = Proxy.revocable(NSMutableString.alloc().init(), {}); + revocable.revoke(); + + expect(function () { + NSMutableString.prototype.appendString.call(revocable.proxy, "x"); + }).toThrowError(TypeError, /revoked Proxy/); + expect(function () { + findAccessor(NSMutableString.prototype, "length").get.call(revocable.proxy); + }).toThrowError(TypeError, /revoked Proxy/); + }); + + it("throws a TypeError for a revoked proxy argument", function () { + var array = NSMutableArray.alloc().init(); + var revocable = Proxy.revocable(NSObject.alloc().init(), {}); + revocable.revoke(); + + expect(function () { + array.addObject(revocable.proxy); + }).toThrowError(TypeError, /revoked Proxy/); + expect(array.count).toBe(0); + }); + + it("throws a TypeError for a proxy whose target is not a native object", function () { + var proxy = new Proxy({}, {}); + + expect(function () { + findAccessor(NSString.prototype, "length").get.call(proxy); + }).toThrowError(TypeError, /not a native object/); + expect(function () { + NSMutableString.prototype.appendString.call(proxy, "x"); + }).toThrowError(TypeError, /not a native object/); + expect(function () { + findAccessor(NSOperation.prototype, "name").set.call(proxy, "x"); + }).toThrowError(TypeError, /not a native object/); + }); + + it("consults proxy traps for the lookup and calls the target natively", function () { + var target = NSMutableString.alloc().init(); + var accessed = []; + var proxy = new Proxy(target, { + get: function (obj, key, receiver) { + accessed.push(key); + return Reflect.get(obj, key, receiver); + } + }); + + proxy.appendString("abc"); + + expect(accessed).toContain("appendString"); + expect(target.toString()).toBe("abc"); + expect(proxy.length).toBe(3); + expect(accessed).toContain("length"); + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 31bfe10a..86a4e26a 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -128,6 +128,7 @@ require("./ObjCConstructors"); require("./MetadataTests"); // require("./ApiTests"); +require("./ProxyReceiverTests"); require("./NsRuntimeTests"); require("./GCFinalizerTests"); require("./WorkerConcurrentStartupTests");