Fix some proxy issues (#1353)

Jint.Tests/Runtime/DestructuringTests.cs

@@ -56,4 +56,10 @@ public class DestructuringTests
         _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]);");
+    }
 }

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<object, object>(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 class ProxyTests
             }
             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 class ProxyTests
             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());
     }
 }

Jint/Native/Proxy/ProxyInstance.cs

@@ -43,6 +43,11 @@ namespace Jint.Native.Proxy
         /// </summary>
         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 @@ namespace Jint.Native.Proxy
         /// </summary>
         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 @@ namespace Jint.Native.Proxy
             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 @@ namespace Jint.Native.Proxy
                 {
                    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 @@ namespace Jint.Native.Proxy
 
                 if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable)
                 {
-                    if (targetDesc.Set!.IsUndefined())
+                    if ((targetDesc.Set ?? Undefined).IsUndefined())
                     {
                         ExceptionHelper.ThrowTypeError(_engine.Realm);
                     }

Jint/Runtime/Environments/FunctionEnvironmentRecord.cs

@@ -147,7 +147,7 @@ namespace Jint.Runtime.Environments
         {
             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 @@ namespace Jint.Runtime.Environments
             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 @@ namespace Jint.Runtime.Environments
             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
             {