From 410806d9d7e7adf3298526a6e77708dd138176f5 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Wed, 9 Nov 2022 19:12:49 +0200 Subject: [PATCH] Fix some proxy issues (#1353) --- Jint.Tests/Runtime/DestructuringTests.cs | 6 + Jint.Tests/Runtime/ProxyTests.cs | 159 +++++++++++++++--- Jint/Native/Proxy/ProxyInstance.cs | 16 +- .../Environments/FunctionEnvironmentRecord.cs | 17 +- 4 files changed, 161 insertions(+), 37 deletions(-) diff --git a/Jint.Tests/Runtime/DestructuringTests.cs b/Jint.Tests/Runtime/DestructuringTests.cs index d6429d6c48..4c1ac13cea 100644 --- a/Jint.Tests/Runtime/DestructuringTests.cs +++ b/Jint.Tests/Runtime/DestructuringTests.cs @@ -56,4 +56,10 @@ public void WithParameterFunctionLengthProperty() _engine.Execute("equal(1, ((a, b = 39,) => {}).length);"); _engine.Execute("equal(2, function({a, b}, [c, d]){}.length);"); } + + [Fact] + public void WithNestedRest() + { + _engine.Execute("return function([x, ...[y, ...z]]) { equal(1, x); equal(2, y); equal('3,4', z + ''); }([1, 2, 3, 4]);"); + } } diff --git a/Jint.Tests/Runtime/ProxyTests.cs b/Jint.Tests/Runtime/ProxyTests.cs index 62040113e6..25d71f3bbb 100644 --- a/Jint.Tests/Runtime/ProxyTests.cs +++ b/Jint.Tests/Runtime/ProxyTests.cs @@ -2,39 +2,46 @@ public class ProxyTests { + private readonly Engine _engine; + + public ProxyTests() + { + _engine = new Engine() + .SetValue("equal", new Action(Assert.Equal)); + } + [Fact] public void ProxyCanBeRevokedWithoutContext() { - new Engine() - .Execute(@" - var revocable = Proxy.revocable({}, {}); - var revoke = revocable.revoke; - revoke.call(null); - "); + _engine.Execute(@" + var revocable = Proxy.revocable({}, {}); + var revoke = revocable.revoke; + revoke.call(null); + "); } [Fact] public void ProxyToStringUseTarget() { - var engine = new Engine().Execute(@" - const targetWithToString = {toString: () => 'target'} - "); - Assert.Equal("target", engine.Evaluate("new Proxy(targetWithToString, {}).toString()").AsString()); - Assert.Equal("target", engine.Evaluate("`${new Proxy(targetWithToString, {})}`").AsString()); + _engine.Execute(@" + const targetWithToString = {toString: () => 'target'} + "); + Assert.Equal("target", _engine.Evaluate("new Proxy(targetWithToString, {}).toString()").AsString()); + Assert.Equal("target", _engine.Evaluate("`${new Proxy(targetWithToString, {})}`").AsString()); } [Fact] public void ProxyToStringUseHandler() { - var engine = new Engine().Execute(@" - const handler = { get: (target, prop, receiver) => prop === 'toString' ? () => 'handler' : Reflect.get(target, prop, receiver) } - const targetWithToString = {toString: () => 'target'} - "); - - Assert.Equal("handler", engine.Evaluate("new Proxy({}, handler).toString()").AsString()); - Assert.Equal("handler", engine.Evaluate("new Proxy(targetWithToString, handler).toString()").AsString()); - Assert.Equal("handler", engine.Evaluate("`${new Proxy({}, handler)}`").AsString()); - Assert.Equal("handler", engine.Evaluate("`${new Proxy(targetWithToString, handler)}`").AsString()); + _engine.Execute(@" + const handler = { get: (target, prop, receiver) => prop === 'toString' ? () => 'handler' : Reflect.get(target, prop, receiver) } + const targetWithToString = {toString: () => 'target'} + "); + + Assert.Equal("handler", _engine.Evaluate("new Proxy({}, handler).toString()").AsString()); + Assert.Equal("handler", _engine.Evaluate("new Proxy(targetWithToString, handler).toString()").AsString()); + Assert.Equal("handler", _engine.Evaluate("`${new Proxy({}, handler)}`").AsString()); + Assert.Equal("handler", _engine.Evaluate("`${new Proxy(targetWithToString, handler)}`").AsString()); } [Fact] @@ -55,8 +62,7 @@ public void ToPropertyDescriptor() } return 'did not fail as expected'"; - var engine = new Engine(); - Assert.Equal("enumerable,configurable,value,writable,get,set", engine.Evaluate(Script)); + Assert.Equal("enumerable,configurable,value,writable,get,set", _engine.Evaluate(Script)); } [Fact] @@ -69,7 +75,112 @@ public void DefineProperties() Object.defineProperties({}, p); return get + '';"; - var engine = new Engine(); - Assert.Equal("foo,bar", engine.Evaluate(Script)); + Assert.Equal("foo,bar", _engine.Evaluate(Script)); + } + + [Fact] + public void GetHandlerInstancesOfProxies() + { + const string Script = @" + var proxied = { }; + var proxy = Object.create(new Proxy(proxied, { + get: function (t, k, r) { + equal(t, proxied); equal('foo', k); equal(proxy, r); + return t === proxied && k === 'foo' && r === proxy && 5; + } + })); + equal(5, proxy.foo);"; + + _engine.Execute(Script); + } + + [Fact] + public void SetHandlerInvariants() + { + const string Script = @" + var passed = false; + var proxied = { }; + var proxy = new Proxy(proxied, { + get: function () { + passed = true; + return 4; + } + }); + // The value reported for a property must be the same as the value of the corresponding + // target object property if the target object property is a non-writable, + // non-configurable own data property. + Object.defineProperty(proxied, ""foo"", { value: 5, enumerable: true }); + try { + proxy.foo; + return false; + } + catch(e) {} + // The value reported for a property must be undefined if the corresponding target + // object property is a non-configurable own accessor property that has undefined + // as its [[Get]] attribute. + Object.defineProperty(proxied, ""bar"", + { set: function(){}, enumerable: true }); + try { + proxy.bar; + return false; + } + catch(e) {} + return passed;"; + + Assert.True(_engine.Evaluate(Script).AsBoolean()); + } + + [Fact] + public void ApplyHandlerInvariant() + { + const string Script = @" + var passed = false; + new Proxy(function(){}, { + apply: function () { passed = true; } + })(); + // A Proxy exotic object only has a [[Call]] internal method if the + // initial value of its [[ProxyTarget]] internal slot is an object + // that has a [[Call]] internal method. + try { + new Proxy({}, { + apply: function () {} + })(); + return false; + } catch(e) {} + return passed;"; + + Assert.True(_engine.Evaluate(Script).AsBoolean()); + } + + [Fact] + public void ConstructHandlerInvariant() + { + const string Script = @" + var passed = false; + new Proxy({},{}); + // A Proxy exotic object only has a [[Construct]] internal method if the + // initial value of its [[ProxyTarget]] internal slot is an object + // that has a [[Construct]] internal method. + try { + new new Proxy({}, { + construct: function (t, args) { + return {}; + } + })(); + return false; + } catch(e) {} + // The result of [[Construct]] must be an Object. + try { + new new Proxy(function(){}, { + construct: function (t, args) { + passed = true; + return 5; + } + })(); + return false; + } catch(e) {} + return passed;"; + + Assert.True(_engine.Evaluate(Script).AsBoolean()); } } diff --git a/Jint/Native/Proxy/ProxyInstance.cs b/Jint/Native/Proxy/ProxyInstance.cs index e554fa5f5f..42d41528c1 100644 --- a/Jint/Native/Proxy/ProxyInstance.cs +++ b/Jint/Native/Proxy/ProxyInstance.cs @@ -43,6 +43,11 @@ public ProxyInstance( /// JsValue ICallable.Call(JsValue thisObject, JsValue[] arguments) { + if (_target is not ICallable) + { + ExceptionHelper.ThrowTypeError(_engine.Realm, "(intermediate value) is not a function"); + } + var jsValues = new[] { _target, thisObject, _engine.Realm.Intrinsics.Array.ConstructFast(arguments) }; if (TryCallHandler(TrapApply, jsValues, out var result)) { @@ -63,6 +68,11 @@ JsValue ICallable.Call(JsValue thisObject, JsValue[] arguments) /// ObjectInstance IConstructor.Construct(JsValue[] arguments, JsValue newTarget) { + if (_target is not ICallable) + { + ExceptionHelper.ThrowTypeError(_engine.Realm, "(intermediate value) is not a constructor"); + } + var argArray = _engine.Realm.Intrinsics.Array.Construct(arguments, _engine.Realm.Intrinsics.Array); if (!TryCallHandler(TrapConstruct, new[] { _target, argArray, newTarget }, out var result)) @@ -116,7 +126,7 @@ public override JsValue Get(JsValue property, JsValue receiver) AssertTargetNotRevoked(property); var target = _target; - if (property == KeyFunctionRevoke || !TryCallHandler(TrapGet, new[] {target, TypeConverter.ToPropertyKey(property), this }, out var result)) + if (property == KeyFunctionRevoke || !TryCallHandler(TrapGet, new[] { target, TypeConverter.ToPropertyKey(property), receiver }, out var result)) { return target.Get(property, receiver); } @@ -128,7 +138,7 @@ public override JsValue Get(JsValue property, JsValue receiver) { ExceptionHelper.ThrowTypeError(_engine.Realm); } - if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable && targetDesc.Get!.IsUndefined() && !result.IsUndefined()) + if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable && (targetDesc.Get ?? Undefined).IsUndefined() && !result.IsUndefined()) { ExceptionHelper.ThrowTypeError(_engine.Realm, $"'get' on proxy: property '{property}' is a non-configurable accessor property on the proxy target and does not have a getter function, but the trap did not return 'undefined' (got '{result}')"); } @@ -324,7 +334,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable) { - if (targetDesc.Set!.IsUndefined()) + if ((targetDesc.Set ?? Undefined).IsUndefined()) { ExceptionHelper.ThrowTypeError(_engine.Realm); } diff --git a/Jint/Runtime/Environments/FunctionEnvironmentRecord.cs b/Jint/Runtime/Environments/FunctionEnvironmentRecord.cs index b3d650b08c..ca06123ba3 100644 --- a/Jint/Runtime/Environments/FunctionEnvironmentRecord.cs +++ b/Jint/Runtime/Environments/FunctionEnvironmentRecord.cs @@ -147,7 +147,7 @@ private void SetFunctionParameter( { if (parameter is Identifier identifier) { - var argument = (uint) index < (uint) arguments.Length ? arguments[index] : Undefined; + var argument = arguments.At(index); SetItemSafely(identifier.Name, argument, initiallyEmpty); } else @@ -163,7 +163,7 @@ private void SetFunctionParameterUnlikely( int index, bool initiallyEmpty) { - var argument = arguments.Length > index ? arguments[index] : Undefined; + var argument = arguments.At(index); if (parameter is RestElement restElement) { @@ -289,24 +289,21 @@ private void HandleRestElementArray( int restCount = arguments.Length - (index + 1) + 1; uint count = restCount > 0 ? (uint) restCount : 0; - var rest = _engine.Realm.Intrinsics.Array.ArrayCreate(count); - uint targetIndex = 0; + var rest = new object[count]; for (var argIndex = index; argIndex < arguments.Length; ++argIndex) { - rest.SetIndexValue(targetIndex++, arguments[argIndex], updateLength: false); + rest[targetIndex++] = arguments[argIndex]; } + var array = new ArrayInstance(_engine, rest); if (restElement.Argument is Identifier restIdentifier) { - SetItemSafely(restIdentifier.Name, rest, initiallyEmpty); + SetItemSafely(restIdentifier.Name, array, initiallyEmpty); } else if (restElement.Argument is BindingPattern bindingPattern) { - SetFunctionParameter(context, bindingPattern, new JsValue[] - { - rest - }, index, initiallyEmpty); + SetFunctionParameter(context, bindingPattern, new [] { array }, 0, initiallyEmpty); } else {