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.

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

Proxy on clr #1645

Merged
merged 3 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
241 changes: 240 additions & 1 deletion Jint.Tests/Runtime/ProxyTests.cs
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,240 @@ public void ConstructHandlerInvariant()

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

[Fact]
public void ProxyHandlerGetDataPropertyShouldNotUseReferenceEquals()
{
// There are two JsString which should be treat as same value,
// but they are not ReferenceEquals.
_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;
""");
}

[Fact]
public void ProxyHandlerGetDataPropertyShouldNotCheckClrType()
{
// There are a JsString and a ConcatenatedString which should be treat as same value,
// but they are different CLR Type.
_engine.Execute("""
let o = Object.defineProperty({}, 'value', {
configurable: false,
value: 'Jint',
});
const handler = {
get(target, property, receiver) {
return 'Ji'.concat('nt');
}
};
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

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;
""");
}
}
15 changes: 14 additions & 1 deletion Jint/Native/JsValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Jint.Native.Object;
using Jint.Native.Symbol;
using Jint.Runtime;
using Jint.Runtime.Interop;

namespace Jint.Native
{
Expand Down Expand Up @@ -465,6 +466,11 @@ internal virtual bool OrdinaryHasInstance(JsValue v)

internal static bool SameValue(JsValue x, JsValue y)
{
if (ReferenceEquals(x, y))
{
return true;
}

var typea = x.Type;
var typeb = y.Type;

Expand Down Expand Up @@ -510,8 +516,15 @@ internal static bool SameValue(JsValue x, JsValue y)
return true;
case Types.Symbol:
return x == y;
case Types.Object:
if (x is ObjectWrapper xo && y is ObjectWrapper yo)
{
return ReferenceEquals(xo.Target, yo.Target)
&& xo.ClrType == yo.ClrType;
}
return false;
default:
return ReferenceEquals(x, y);
return false;
}
}

Expand Down
36 changes: 23 additions & 13 deletions Jint/Native/Proxy/JsProxy.cs
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,13 +135,21 @@ 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 && !SameValue(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}')");
}
}
}

Expand All @@ -152,7 +161,7 @@ public override JsValue Get(JsValue property, JsValue receiver)
/// </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 +330,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 (!SameValue(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 +415,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 +484,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 +506,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 +526,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