Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Proxy on clr #1645

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Proxy on clr #1645

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91d100f
fix #1643
viruscamp Oct 12, 2023
48bd79b
Merge branch 'sebastienros:main' into proxy-on-clr
viruscamp Oct 12, 2023
99be0e2
use `JsValue.SameValue` in `JsProxy`
viruscamp Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
219 changes: 218 additions & 1 deletion Jint.Tests/Runtime/ProxyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
namespace Jint.Tests.Runtime;
using Jint.Native.Error;
using Jint.Runtime;

namespace Jint.Tests.Runtime;

public class ProxyTests
{
Expand Down Expand Up @@ -183,4 +186,218 @@ public void ConstructHandlerInvariant()

Assert.True(_engine.Evaluate(Script).AsBoolean());
}

[Fact]
public void ProxyHandlerGetDataPropertyShouldNotUseReferenceEquals()
{
_engine.Execute("""
let o = Object.defineProperty({}, 'value', {
configurable: false,
value: 'in',
});
const handler = {
get(target, property, receiver) {
return 'Jint'.substring(1,3);
}
};
let p = new Proxy(o, handler);
let pv = p.value;
""");
}

class TestClass
{
public static readonly TestClass Instance = new TestClass();
public string StringValue => "StringValue";
public int IntValue => 42424242; // avoid small numbers cache
public TestClass ObjectWrapper => Instance;
}

[Fact]
public void ProxyClrPropertyPrimitiveString()
{
_engine.SetValue("testClass", TestClass.Instance);
var result = _engine.Evaluate("""
const handler = {
get(target, property, receiver) {
return Reflect.get(target, property, receiver);
}
};
const p = new Proxy(testClass, handler);
return p.StringValue;
""");
Assert.Equal(TestClass.Instance.StringValue, result.AsString());
}

[Fact]
public void ProxyClrPropertyPrimitiveInt()
{
_engine.SetValue("testClass", TestClass.Instance);
var result = _engine.Evaluate("""
const handler = {
get(target, property, receiver) {
return Reflect.get(target, property, receiver);
}
};
const p = new Proxy(testClass, handler);
return p.IntValue;
""");
Assert.Equal(TestClass.Instance.IntValue, result.AsInteger());
}

[Fact]
public void ProxyClrPropertyObjectWrapper()
{
_engine.SetValue("testClass", TestClass.Instance);
var result = _engine.Evaluate("""
const handler = {
get(target, property, receiver) {
return Reflect.get(target, property, receiver);
}
};
const p = new Proxy(testClass, handler);
return p.ObjectWrapper;
""");
}

private static ErrorPrototype TypeErrorPrototype(Engine engine)
=> engine.Realm.Intrinsics.TypeError.PrototypeObject;

private static void AssertJsTypeError(Engine engine, JavaScriptException ex, string msg)
{
Assert.Same(TypeErrorPrototype(engine), ex.Error.AsObject().Prototype);
Assert.Equal(msg, ex.Message);
}

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/get#invariants
// The value reported for a property must be the same as
// the value ofthe corresponding target object property,
// if the target object property is
// a non-writable, non-configurable own data property.
[Fact]
public void ProxyHandlerGetInvariantsDataPropertyReturnsDifferentValue()
{
_engine.Execute("""
let o = Object.defineProperty({}, 'value', {
writable: false,
configurable: false,
value: 42,
});
const handler = {
get(target, property, receiver) {
return 32;
}
};
let p = new Proxy(o, handler);
""");
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("p.value"));
AssertJsTypeError(_engine, ex, "'get' on proxy: property 'value' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '42' but got '32')");
Copy link
Contributor Author

@viruscamp viruscamp Oct 12, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lahma
Should we comment out all AssertJsTypeError?
As it depends internal implementations and unstable error messages.
Just comment out these for develop usage.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you did great work replicating the V8 error messages and they can stay, we can always tweak later if comes a maintenance burden 👍🏻

}

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/get#invariants
// 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.
[Fact]
public void ProxyHandlerGetInvariantsAccessorPropertyWithoutGetButReturnsValue()
{
_engine.Execute("""
let o = Object.defineProperty({}, 'value', {
configurable: false,
set() {},
});
const handler = {
get(target, property, receiver) {
return 32;
}
};
let p = new Proxy(o, handler);
""");
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("p.value"));
AssertJsTypeError(_engine, ex, "'get' on proxy: property 'value' 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 '32')");
}

private const string ScriptProxyHandlerSetInvariantsDataPropertyImmutable = """
let o = Object.defineProperty({}, 'value', {
writable: false,
configurable: false,
value: 42,
});
const handler = {
set(target, property, value, receiver) {
return true;
}
};
let p = new Proxy(o, handler);
""";

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/set#invariants
// Cannot change the value of a property to be different from
// the value of the corresponding target object property,
// if the corresponding target object property is
// a non-writable, non-configurable data property.
[Fact]
public void ProxyHandlerSetInvariantsDataPropertyImmutableChangeValue()
{
_engine.Execute(ScriptProxyHandlerSetInvariantsDataPropertyImmutable);
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("p.value = 32"));
AssertJsTypeError(_engine, ex, "'set' on proxy: trap returned truish for property 'value' which exists in the proxy target as a non-configurable and non-writable data property with a different value");
}

[Fact]
public void ProxyHandlerSetInvariantsDataPropertyImmutableSetSameValue()
{
_engine.Execute(ScriptProxyHandlerSetInvariantsDataPropertyImmutable);
_engine.Evaluate("p.value = 42");
}

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/set#invariants
// Cannot set the value of a property,
// if the corresponding target object property is
// a non-configurable accessor property
// that has undefined as its [[Set]] attribute.
[Fact]
public void ProxyHandlerSetInvariantsAccessorPropertyWithoutSetChange()
{
_engine.Execute("""
let o = Object.defineProperty({}, 'value', {
configurable: false,
get() { return 42; },
});
const handler = {
set(target, property, value, receiver) {
return true;
}
};
let p = new Proxy(o, handler);
""");
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("p.value = 42"));
AssertJsTypeError(_engine, ex, "'set' on proxy: trap returned truish for property 'value' which exists in the proxy target as a non-configurable and non-writable accessor property without a setter");
}

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/set#invariants
// In strict mode, a false return value from the set() handler
// will throw a TypeError exception.
[Fact]
public void ProxyHandlerSetInvariantsReturnsFalseInStrictMode()
{
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("""
'use strict';
let p = new Proxy({}, { set: () => false });
p.value = 42;
"""));
// V8: "'set' on proxy: trap returned falsish for property 'value'",
AssertJsTypeError(_engine, ex, "Cannot assign to read only property 'value' of [object Object]");
}

[Fact]
public void ProxyHandlerSetInvariantsReturnsFalseInNonStrictMode()
{
_engine.Evaluate("""
// 'use strict';
let p = new Proxy({}, { set: () => false });
p.value = 42;
""");
}
}
58 changes: 45 additions & 13 deletions Jint/Native/Proxy/JsProxy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Jint.Native.Object;
using Jint.Runtime;
using Jint.Runtime.Descriptors;
using Jint.Runtime.Interop;

namespace Jint.Native.Proxy
{
Expand Down Expand Up @@ -134,25 +135,55 @@ public override JsValue Get(JsValue property, JsValue receiver)
var targetDesc = target.GetOwnProperty(property);
if (targetDesc != PropertyDescriptor.Undefined)
{
if (targetDesc.IsDataDescriptor() && !targetDesc.Configurable && !targetDesc.Writable && !ReferenceEquals(result, targetDesc._value))
if (targetDesc.IsDataDescriptor())
{
ExceptionHelper.ThrowTypeError(_engine.Realm);
var targetValue = targetDesc.Value;
if (!targetDesc.Configurable && !targetDesc.Writable && !IsSameValue(result, targetValue))
{
ExceptionHelper.ThrowTypeError(_engine.Realm, $"'get' on proxy: property '{property}' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '{targetValue}' but got '{result}')");
}
}
if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable && (targetDesc.Get ?? Undefined).IsUndefined() && !result.IsUndefined())

if (targetDesc.IsAccessorDescriptor())
{
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}')");
if (!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}')");
}
}
}

return result;
}

private static bool IsSameValue(JsValue expected, JsValue actual)
viruscamp marked this conversation as resolved.
Show resolved Hide resolved
{
if (ReferenceEquals(expected, actual))
{
return true;
}
if (expected.GetType() != actual.GetType())
viruscamp marked this conversation as resolved.
Show resolved Hide resolved
{
return false;
}
if (expected.IsPrimitive())
{
return expected == actual;
}
if (expected is ObjectWrapper expectedObj && actual is ObjectWrapper actualObj)
{
return ReferenceEquals(expectedObj.Target, actualObj.Target)
&& expectedObj.ClrType == actualObj.ClrType;
}
return false;
}

/// <summary>
/// https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
/// </summary>
public override List<JsValue> GetOwnPropertyKeys(Types types = Types.None | Types.String | Types.Symbol)
{
if (!TryCallHandler(TrapOwnKeys, new JsValue[] { _target }, out var result))
if (!TryCallHandler(TrapOwnKeys, new[] { _target }, out var result))
{
return _target.GetOwnPropertyKeys(types);
}
Expand Down Expand Up @@ -321,22 +352,23 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver)
return false;
}

var targetDesc = _target.GetOwnProperty(property);
var targetDesc = _target.GetOwnProperty(property);
if (targetDesc != PropertyDescriptor.Undefined)
{
if (targetDesc.IsDataDescriptor() && !targetDesc.Configurable && !targetDesc.Writable)
{
if (targetDesc.Value != value)
var targetValue = targetDesc.Value;
if (!IsSameValue(targetValue, value))
{
ExceptionHelper.ThrowTypeError(_engine.Realm);
ExceptionHelper.ThrowTypeError(_engine.Realm, $"'set' on proxy: trap returned truish for property '{property}' which exists in the proxy target as a non-configurable and non-writable data property with a different value");
}
}

if (targetDesc.IsAccessorDescriptor() && !targetDesc.Configurable)
{
if ((targetDesc.Set ?? Undefined).IsUndefined())
{
ExceptionHelper.ThrowTypeError(_engine.Realm);
ExceptionHelper.ThrowTypeError(_engine.Realm, $"'set' on proxy: trap returned truish for property '{property}' which exists in the proxy target as a non-configurable and non-writable accessor property without a setter");
}
}
}
Expand Down Expand Up @@ -405,7 +437,7 @@ private static bool IsCompatiblePropertyDescriptor(bool extensible, PropertyDesc
/// </summary>
public override bool HasProperty(JsValue property)
{
if (!TryCallHandler(TrapHas, new [] { _target, TypeConverter.ToPropertyKey(property) }, out var jsValue))
if (!TryCallHandler(TrapHas, new[] { _target, TypeConverter.ToPropertyKey(property) }, out var jsValue))
{
return _target.HasProperty(property);
}
Expand Down Expand Up @@ -474,7 +506,7 @@ public override bool Delete(JsValue property)
/// </summary>
public override bool PreventExtensions()
{
if (!TryCallHandler(TrapPreventExtensions, new JsValue[] { _target }, out var result))
if (!TryCallHandler(TrapPreventExtensions, new[] { _target }, out var result))
{
return _target.PreventExtensions();
}
Expand All @@ -496,7 +528,7 @@ public override bool Extensible
{
get
{
if (!TryCallHandler(TrapIsExtensible, new JsValue[] { _target }, out var result))
if (!TryCallHandler(TrapIsExtensible, new[] { _target }, out var result))
{
return _target.Extensible;
}
Expand All @@ -516,7 +548,7 @@ public override bool Extensible
/// </summary>
protected internal override ObjectInstance? GetPrototypeOf()
{
if (!TryCallHandler(TrapGetProtoTypeOf, new JsValue[] { _target }, out var handlerProto ))
if (!TryCallHandler(TrapGetProtoTypeOf, new[] { _target }, out var handlerProto))
{
return _target.Prototype;
}
Expand Down