Skip to content

Commit

Permalink
Fix some proxy issues (#1353)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahma authored Nov 9, 2022
1 parent 6b5f04d commit 410806d
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 37 deletions.
6 changes: 6 additions & 0 deletions Jint.Tests/Runtime/DestructuringTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);");
}
}
159 changes: 135 additions & 24 deletions Jint.Tests/Runtime/ProxyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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());
}
}
16 changes: 13 additions & 3 deletions Jint/Native/Proxy/ProxyInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ public ProxyInstance(
/// </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))
{
Expand All @@ -63,6 +68,11 @@ JsValue ICallable.Call(JsValue thisObject, JsValue[] arguments)
/// </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))
Expand Down Expand Up @@ -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);
}
Expand All @@ -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}')");
}
Expand Down Expand Up @@ -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);
}
Expand Down
17 changes: 7 additions & 10 deletions Jint/Runtime/Environments/FunctionEnvironmentRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
{
Expand Down Expand Up @@ -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
{
Expand Down

0 comments on commit 410806d

Please sign in to comment.