From 270e41459e185f2e0b948ca4ae407c7e35a1aa1b Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sun, 19 Nov 2023 10:20:04 +0200 Subject: [PATCH] Refine interop member search with readable/writable requirement (#1687) --- Jint.Tests/Runtime/InteropTests.cs | 88 +++++++++++++++++++ .../Specialized/ReflectionDescriptor.cs | 5 +- Jint/Runtime/Interop/ObjectWrapper.cs | 28 ++++-- Jint/Runtime/Interop/TypeResolver.cs | 13 +-- 4 files changed, 122 insertions(+), 12 deletions(-) diff --git a/Jint.Tests/Runtime/InteropTests.cs b/Jint.Tests/Runtime/InteropTests.cs index e054e0bf42..3fb9c50ab9 100644 --- a/Jint.Tests/Runtime/InteropTests.cs +++ b/Jint.Tests/Runtime/InteropTests.cs @@ -3333,5 +3333,93 @@ public void CanDestructureInteropTargetMethod() var result = engine.Evaluate("const { getStrings } = test; getStrings().Count;"); Assert.Equal(3, result); } + + private class MetadataWrapper : IDictionary + { + public IEnumerator> GetEnumerator() => throw new NotImplementedException(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + public void Add(KeyValuePair item) => throw new NotImplementedException(); + public void Clear() => throw new NotImplementedException(); + public bool Contains(KeyValuePair item) => throw new NotImplementedException(); + public void CopyTo(KeyValuePair[] array, int arrayIndex) => throw new NotImplementedException(); + public bool Remove(KeyValuePair item) => throw new NotImplementedException(); + public int Count { get; set; } + public bool IsReadOnly { get; set; } + public bool ContainsKey(string key) => throw new NotImplementedException(); + public void Add(string key, object value) => throw new NotImplementedException(); + public bool Remove(string key) => throw new NotImplementedException(); + public bool TryGetValue(string key, out object value) + { + value = "from-wrapper"; + return true; + } + + public object this[string key] + { + get => "from-wrapper"; + set + { + } + } + + public ICollection Keys { get; set; } + public ICollection Values { get; set; } + } + + private class ShadowedGetter : IReadOnlyDictionary + { + private Dictionary _dictionary = new(); + + public void SetInitial(object? value, string? key) + { + _dictionary[key] = value; + } + + public IEnumerator> GetEnumerator() => throw new NotImplementedException(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + public int Count { get; } + public bool ContainsKey(string key) => _dictionary.ContainsKey(key); + + public bool TryGetValue(string key, out object value) => _dictionary.TryGetValue(key, out value); + + public object this[string key] + { + get + { + _dictionary.TryGetValue(key, out var value); + return value; + } + } + + public IEnumerable Keys { get; set; } + public IEnumerable Values { get; set; } + } + + private class ShadowingSetter : ShadowedGetter + { + public Dictionary Metadata + { + set + { + SetInitial(new MetadataWrapper(), "metadata"); + } + } + } + + [Fact] + public void CanSelectShadowedPropertiesBasedOnReadableAndWritable() + { + var engine = new Engine(); + engine.SetValue("test", new ShadowingSetter + { + Metadata = null + }); + + engine.Evaluate("test.metadata['abc'] = 123"); + var result = engine.Evaluate("test.metadata['abc']"); + Assert.Equal("from-wrapper", result); + } } } diff --git a/Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs b/Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs index abfe7b4bb5..bec4af0ce7 100644 --- a/Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs +++ b/Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs @@ -43,13 +43,14 @@ protected internal override JsValue? CustomValue set => DoSet(null, value); } - JsValue DoGet(JsValue? thisObj) + private JsValue DoGet(JsValue? thisObj) { var value = _reflectionAccessor.GetValue(_engine, _target); var type = _reflectionAccessor.MemberType; return JsValue.FromObjectWithType(_engine, value, type); } - void DoSet(JsValue? thisObj, JsValue? v) + + private void DoSet(JsValue? thisObj, JsValue? v) { try { diff --git a/Jint/Runtime/Interop/ObjectWrapper.cs b/Jint/Runtime/Interop/ObjectWrapper.cs index ec7a127a5e..7703613325 100644 --- a/Jint/Runtime/Interop/ObjectWrapper.cs +++ b/Jint/Runtime/Interop/ObjectWrapper.cs @@ -50,7 +50,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) if (_properties is null || !_properties.ContainsKey(member)) { // can try utilize fast path - var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, forWrite: true); + var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, mustBeReadable: false, mustBeWritable: true); if (ReferenceEquals(accessor, ConstantValueAccessor.NullAccessor)) { @@ -116,7 +116,13 @@ public override JsValue Get(JsValue property, JsValue receiver) return (uint) index < list.Count ? FromObject(_engine, list[index]) : Undefined; } - return base.Get(property, receiver); + var desc = GetOwnProperty(property, mustBeReadable: true, mustBeWritable: false); + if (desc != PropertyDescriptor.Undefined) + { + return UnwrapJsValue(desc, receiver); + } + + return Prototype?.Get(property, receiver) ?? Undefined; } public override List GetOwnPropertyKeys(Types types = Types.None | Types.String | Types.Symbol) @@ -182,6 +188,12 @@ private IEnumerable EnumerateOwnPropertyKeys(Types types) } public override PropertyDescriptor GetOwnProperty(JsValue property) + { + // we do not know if we need to read or write + return GetOwnProperty(property, mustBeReadable: false, mustBeWritable: false); + } + + private PropertyDescriptor GetOwnProperty(JsValue property, bool mustBeReadable, bool mustBeWritable) { if (TryGetProperty(property, out var x)) { @@ -234,13 +246,17 @@ public override PropertyDescriptor GetOwnProperty(JsValue property) return new PropertyDescriptor(result, PropertyFlag.OnlyEnumerable); } - var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member); + var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, mustBeReadable, mustBeWritable); var descriptor = accessor.CreatePropertyDescriptor(_engine, Target, enumerable: !isDictionary); - if (!isDictionary && !ReferenceEquals(descriptor, PropertyDescriptor.Undefined)) + if (!isDictionary + && !ReferenceEquals(descriptor, PropertyDescriptor.Undefined) + && (!mustBeReadable || accessor.Readable) + && (!mustBeWritable || accessor.Writable)) { // cache the accessor for faster subsequent accesses SetProperty(member, descriptor); } + return descriptor; } @@ -258,7 +274,9 @@ public static PropertyDescriptor GetPropertyDescriptor(Engine engine, object tar _ => null }; } - return engine.Options.Interop.TypeResolver.GetAccessor(engine, target.GetType(), member.Name, Factory).CreatePropertyDescriptor(engine, target); + + var accessor = engine.Options.Interop.TypeResolver.GetAccessor(engine, target.GetType(), member.Name, mustBeReadable: false, mustBeWritable: false, Factory); + return accessor.CreatePropertyDescriptor(engine, target); } internal static Type GetClrType(object obj, Type? type) diff --git a/Jint/Runtime/Interop/TypeResolver.cs b/Jint/Runtime/Interop/TypeResolver.cs index b4fcb7f6b2..eb212b5bf0 100644 --- a/Jint/Runtime/Interop/TypeResolver.cs +++ b/Jint/Runtime/Interop/TypeResolver.cs @@ -47,8 +47,9 @@ internal ReflectionAccessor GetAccessor( Engine engine, Type type, string member, - Func? accessorFactory = null, - bool forWrite = false) + bool mustBeReadable, + bool mustBeWritable, + Func? accessorFactory = null) { var key = new Engine.ClrPropertyDescriptorFactoriesKey(type, member); @@ -58,7 +59,7 @@ internal ReflectionAccessor GetAccessor( return accessor; } - accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member, forWrite); + accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member, mustBeReadable, mustBeWritable); // racy, we don't care, worst case we'll catch up later Interlocked.CompareExchange(ref engine._reflectionAccessors, @@ -74,7 +75,8 @@ private ReflectionAccessor ResolvePropertyDescriptorFactory( Engine engine, Type type, string memberName, - bool forWrite) + bool mustBeReadable, + bool mustBeWritable) { var isInteger = long.TryParse(memberName, NumberStyles.Integer, CultureInfo.InvariantCulture, out _); @@ -86,7 +88,8 @@ private ReflectionAccessor ResolvePropertyDescriptorFactory( // properties and fields cannot be numbers if (!isInteger && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp) - && (!forWrite || temp.Writable)) + && (!mustBeReadable || temp.Readable) + && (!mustBeWritable || temp.Writable)) { return temp; }