Improve property selection for setting value under interop (#1389)

fixes #1375
sebastienros · Dec 25, 2022 · fa9b3d9 · fa9b3d9
1 parent 3c45b8c
commit fa9b3d9
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 14 deletions.
diff --git a/Jint.Tests/Runtime/InteropTests.NewtonsoftJson.cs b/Jint.Tests/Runtime/InteropTests.NewtonsoftJson.cs
@@ -115,5 +115,19 @@ public void DecimalsShouldBeHandledFromJObjects()
             var number = Assert.IsType<JsNumber>(fromInterop);
             Assert.Equal(123.456d, number.AsNumber());
         }
+
+        [Fact]
+        public void ShouldBeAbleToChangePropertyWithNameValue()
+        {
+            var engine = new Engine();
+
+            var input = Newtonsoft.Json.JsonConvert.DeserializeObject(@"{ ""value"": ""ORIGINAL"" }");
+            var result = engine
+                .SetValue("input", input)
+                .Evaluate("input.value = \"CHANGED\"; input.value")
+                .AsString();
+
+            Assert.Equal("CHANGED", result);
+        }
     }
 }
diff --git a/Jint/Native/TypedArray/TypedArrayPrototype.cs b/Jint/Native/TypedArray/TypedArrayPrototype.cs
@@ -1,9 +1,5 @@
 using Jint.Collections;
-using Jint.Native.Array;
-using Jint.Native.Object;
-using Jint.Runtime;
 using Jint.Runtime.Descriptors;
-using Jint.Runtime.Interop;
 
 namespace Jint.Native.TypedArray;
 

diff --git a/Jint/Native/WeakSet/WeakSetInstance.cs b/Jint/Native/WeakSet/WeakSetInstance.cs
@@ -1,7 +1,6 @@
 using System.Runtime.CompilerServices;
 
 using Jint.Native.Object;
-using Jint.Native.Symbol;
 using Jint.Runtime;
 
 namespace Jint.Native.WeakSet;

diff --git a/Jint/Runtime/Debugger/DebugHandler.cs b/Jint/Runtime/Debugger/DebugHandler.cs
@@ -1,4 +1,3 @@
-using System.Runtime.CompilerServices;
 using Esprima;
 using Esprima.Ast;
 using Jint.Native;

diff --git a/Jint/Runtime/Interop/ObjectWrapper.cs b/Jint/Runtime/Interop/ObjectWrapper.cs
@@ -48,7 +48,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, Target.GetType(), member);
+                    var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, Target.GetType(), member, forWrite: true);
 
                     if (ReferenceEquals(accessor, ConstantValueAccessor.NullAccessor))
                     {

diff --git a/Jint/Runtime/Interop/TypeResolver.cs b/Jint/Runtime/Interop/TypeResolver.cs
@@ -45,7 +45,12 @@ private static IEnumerable<string> NameCreator(MemberInfo info)
         /// </summary>
         public StringComparer MemberNameComparer { get; set; } = DefaultMemberNameComparer.Instance;
 
-        internal ReflectionAccessor GetAccessor(Engine engine, Type type, string member, Func<ReflectionAccessor?>? accessorFactory = null)
+        internal ReflectionAccessor GetAccessor(
+            Engine engine,
+            Type type,
+            string member,
+            Func<ReflectionAccessor?>? accessorFactory = null,
+            bool forWrite = false)
         {
             var key = new ClrPropertyDescriptorFactoriesKey(type, member);
 
@@ -55,7 +60,7 @@ internal ReflectionAccessor GetAccessor(Engine engine, Type type, string member,
                 return accessor;
             }
 
-            accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member);
+            accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member, forWrite);
 
             // racy, we don't care, worst case we'll catch up later
             Interlocked.CompareExchange(ref _reflectionAccessors,
@@ -70,17 +75,20 @@ internal ReflectionAccessor GetAccessor(Engine engine, Type type, string member,
         private ReflectionAccessor ResolvePropertyDescriptorFactory(
             Engine engine,
             Type type,
-            string memberName)
+            string memberName,
+            bool forWrite)
         {
             var isNumber = uint.TryParse(memberName, out _);
 
             // we can always check indexer if there's one, and then fall back to properties if indexer returns null
             IndexerAccessor.TryFindIndexer(engine, type, memberName, out var indexerAccessor, out var indexer);
 
-            const BindingFlags bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
+            const BindingFlags BindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
 
             // properties and fields cannot be numbers
-            if (!isNumber && TryFindMemberAccessor(engine, type, memberName, bindingFlags, indexer, out var temp))
+            if (!isNumber
+                && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp)
+                && (!forWrite || temp.Writable))
             {
                 return temp;
             }
@@ -280,8 +288,7 @@ internal ReflectionAccessor GetAccessor(Engine engine, Type type, string member,
             }
 
             // TPC: need to grab the extension methods here - for overloads
-            MethodInfo[] extensionMethods;
-            if (engine._extensionMethods.TryGetExtensionMethods(type, out extensionMethods))
+            if (engine._extensionMethods.TryGetExtensionMethods(type, out var extensionMethods))
             {
                 foreach (var methodInfo in extensionMethods)
                 {