Prechádzať zdrojové kódy

Exclude static fields and properties from ObjectWrapper by default (#1981)

Marko Lahma 9 mesiacov pred
rodič
commit
8e3e320891

+ 33 - 23
Jint.Tests.PublicInterface/InteropTests.Json.cs

@@ -1,4 +1,5 @@
 using System.Dynamic;
+using FluentAssertions;
 using Jint.Runtime.Interop;
 
 namespace Jint.Tests.PublicInterface;
@@ -102,31 +103,27 @@ public partial class InteropTests
 
         Assert.Equal(expected, value);
     }
-    
+
     [Fact]
     public void CanStringifyUsingSerializeToJson()
     {
         object testObject = new { Foo = "bar", FooBar = new { Foo = 123.45, Foobar = new DateTime(2022, 7, 16, 0, 0, 0, DateTimeKind.Utc) } };
-        
+
         // without interop
-        
-        var engineNoInterop = new Engine();
-        engineNoInterop.SetValue("TimeSpan", TypeReference.CreateTypeReference<TimeSpan>(engineNoInterop));
-        Assert.Throws<Jint.Runtime.JavaScriptException>(
-            () => engineNoInterop.Evaluate("JSON.stringify(TimeSpan.FromSeconds(3))"));
-        
-        engineNoInterop.SetValue("TestObject", testObject);
-        Assert.Equal(
-            "{\"Foo\":\"bar\",\"FooBar\":{\"Foo\":123.45,\"Foobar\":\"2022-07-16T00:00:00.000Z\"}}",
-            engineNoInterop.Evaluate("JSON.stringify(TestObject)"));
-        
+
+        var e = new Engine();
+        e.SetValue("TimeSpan", typeof(TimeSpan));
+#if NETFRAMEWORK
+        e.Evaluate("JSON.stringify(TimeSpan.FromSeconds(3))").AsString().Should().Be("""{"Ticks":30000000,"Days":0,"Hours":0,"Milliseconds":0,"Minutes":0,"Seconds":3,"TotalDays":0.00003472222222222222,"TotalHours":0.0008333333333333333,"TotalMilliseconds":3000,"TotalMinutes":0.05,"TotalSeconds":3}""");
+#else
+        e.Evaluate("JSON.stringify(TimeSpan.FromSeconds(3))").AsString().Should().Be("""{"Ticks":30000000,"Days":0,"Hours":0,"Milliseconds":0,"Microseconds":0,"Nanoseconds":0,"Minutes":0,"Seconds":3,"TotalDays":0.00003472222222222222,"TotalHours":0.0008333333333333334,"TotalMilliseconds":3000,"TotalMicroseconds":3000000,"TotalNanoseconds":3000000000,"TotalMinutes":0.05,"TotalSeconds":3}""");
+#endif
+
+        e.SetValue("TestObject", testObject);
+        e.Evaluate("JSON.stringify(TestObject)").AsString().Should().Be("""{"Foo":"bar","FooBar":{"Foo":123.45,"Foobar":"2022-07-16T00:00:00.000Z"}}""");
+
         // interop using Newtonsoft serializer, for example with snake case naming
-        
-        string Serialize(object o) =>
-            Newtonsoft.Json.JsonConvert.SerializeObject(o,
-                new Newtonsoft.Json.JsonSerializerSettings {
-                    ContractResolver = new Newtonsoft.Json.Serialization.DefaultContractResolver {
-                        NamingStrategy = new Newtonsoft.Json.Serialization.SnakeCaseNamingStrategy() } });
+
         var engine = new Engine(options =>
         {
             options.Interop.SerializeToJson = Serialize;
@@ -136,13 +133,26 @@ public partial class InteropTests
 
         var expected = Serialize(TimeSpan.FromSeconds(3));
         var actual = engine.Evaluate("JSON.stringify(TimeSpan.FromSeconds(3));");
-        Assert.Equal(expected, actual);
-        
+        actual.AsString().Should().Be(expected);
+
         expected = Serialize(testObject);
         actual = engine.Evaluate("JSON.stringify(TestObject)");
-        Assert.Equal(expected, actual);
+        actual.AsString().Should().Be(expected);
 
         actual = engine.Evaluate("JSON.stringify({ nestedValue: TestObject })");
-        Assert.Equal($@"{{""nestedValue"":{expected}}}", actual);
+        actual.AsString().Should().Be($$"""{"nestedValue":{{expected}}}""");
+        return;
+
+        string Serialize(object o)
+        {
+            var settings = new Newtonsoft.Json.JsonSerializerSettings
+            {
+                ContractResolver = new Newtonsoft.Json.Serialization.DefaultContractResolver
+                {
+                    NamingStrategy = new Newtonsoft.Json.Serialization.SnakeCaseNamingStrategy()
+                }
+            };
+            return Newtonsoft.Json.JsonConvert.SerializeObject(o, settings);
+        }
     }
 }

+ 1 - 5
Jint.Tests/Runtime/InteropTests.TypeReference.cs

@@ -184,22 +184,18 @@ public partial class InteropTests
     }
 
     [Fact]
-    public void CanRegisterToStringTag()
+    public void ToStringTagShouldReflectType()
     {
         var reference = TypeReference.CreateTypeReference<Dependency>(_engine);
-        reference.FastSetProperty(GlobalSymbolRegistry.ToStringTag, new PropertyDescriptor(nameof(Dependency), false, false, true));
-        reference.FastSetDataProperty("abc", 123);
 
         _engine.SetValue("MyClass", reference);
         _engine.Execute("var c = new MyClass();");
 
         Assert.Equal("[object Dependency]", _engine.Evaluate("Object.prototype.toString.call(c);"));
-        Assert.Equal(123, _engine.Evaluate("c.abc"));
 
         // engine uses registered type reference
         _engine.SetValue("c2", new Dependency());
         Assert.Equal("[object Dependency]", _engine.Evaluate("Object.prototype.toString.call(c2);"));
-        Assert.Equal(123, _engine.Evaluate("c2.abc"));
     }
 
     private class Injectable

+ 36 - 3
Jint.Tests/Runtime/InteropTests.cs

@@ -4,6 +4,7 @@ using System.Reflection;
 using System.Runtime.CompilerServices;
 using Jint.Native;
 using Jint.Native.Function;
+using Jint.Native.Number;
 using Jint.Runtime;
 using Jint.Runtime.Interop;
 using Jint.Tests.Runtime.Converters;
@@ -55,7 +56,7 @@ public partial class InteropTests : IDisposable
     [Fact]
     public void ShouldStringifyNetObjects()
     {
-        _engine.SetValue("foo", new Foo());
+        _engine.SetValue("foo", typeof(Foo));
         var json = _engine.Evaluate("JSON.stringify(foo.GetBar())").AsString();
         Assert.Equal("{\"Test\":\"123\"}", json);
     }
@@ -2781,20 +2782,32 @@ public partial class InteropTests : IDisposable
             options.SetTypeResolver(customTypeResolver);
             options.AddExtensionMethods(typeof(CustomNamedExtensions));
         });
+
         engine.SetValue("o", new CustomNamed());
         Assert.Equal("StringField", engine.Evaluate("o.jsStringField").AsString());
         Assert.Equal("StringField", engine.Evaluate("o.jsStringField2").AsString());
-        Assert.Equal("StaticStringField", engine.Evaluate("o.jsStaticStringField").AsString());
         Assert.Equal("StringProperty", engine.Evaluate("o.jsStringProperty").AsString());
         Assert.Equal("Method", engine.Evaluate("o.jsMethod()").AsString());
-        Assert.Equal("StaticMethod", engine.Evaluate("o.jsStaticMethod()").AsString());
         Assert.Equal("InterfaceStringProperty", engine.Evaluate("o.jsInterfaceStringProperty").AsString());
         Assert.Equal("InterfaceMethod", engine.Evaluate("o.jsInterfaceMethod()").AsString());
         Assert.Equal("ExtensionMethod", engine.Evaluate("o.jsExtensionMethod()").AsString());
 
+        // static methods are reported by default, unlike properties and fields
+        Assert.Equal("StaticMethod", engine.Evaluate("o.jsStaticMethod()").AsString());
+
+        engine.SetValue("CustomNamed", typeof(CustomNamed));
+        Assert.Equal("StaticStringField", engine.Evaluate("CustomNamed.jsStaticStringField").AsString());
+        Assert.Equal("StaticMethod", engine.Evaluate("CustomNamed.jsStaticMethod()").AsString());
+
         engine.SetValue("XmlHttpRequest", typeof(CustomNamedEnum));
         engine.Evaluate("o.jsEnumProperty = XmlHttpRequest.HEADERS_RECEIVED;");
         Assert.Equal((int) CustomNamedEnum.HeadersReceived, engine.Evaluate("o.jsEnumProperty").AsNumber());
+
+        // can get static members with different configuration
+        var engineWithStaticsReported = new Engine(options => options.Interop.ObjectWrapperReportedFieldBindingFlags |= BindingFlags.Static);
+        engineWithStaticsReported.SetValue("o", new CustomNamed());
+        Assert.Equal("StaticMethod", engineWithStaticsReported.Evaluate("o.staticMethod()").AsString());
+        Assert.Equal("StaticStringField", engineWithStaticsReported.Evaluate("o.staticStringField").AsString());
     }
 
     [Fact]
@@ -3665,4 +3678,24 @@ try {
         var lionManeLength = engine.Evaluate("zoo.animals[0].maneLength");
         Assert.Equal(10, lionManeLength.AsNumber());
     }
+
+    [Fact]
+    public void StaticFieldsShouldFollowJsSemantics()
+    {
+        _engine.Evaluate("Number.MAX_SAFE_INTEGER").AsNumber().Should().Be(NumberConstructor.MaxSafeInteger);
+        _engine.Evaluate("new Number().MAX_SAFE_INTEGER").Should().Be(JsValue.Undefined);
+
+        _engine.Execute("class MyJsClass { static MAX_SAFE_INTEGER = Number.MAX_SAFE_INTEGER; }");
+        _engine.Evaluate("MyJsClass.MAX_SAFE_INTEGER").AsNumber().Should().Be(NumberConstructor.MaxSafeInteger);
+        _engine.Evaluate("new MyJsClass().MAX_SAFE_INTEGER").Should().Be(JsValue.Undefined);
+
+        _engine.SetValue("MyCsClass", typeof(MyClass));
+        _engine.Evaluate("MyCsClass.MAX_SAFE_INTEGER").AsNumber().Should().Be(NumberConstructor.MaxSafeInteger);
+        _engine.Evaluate("new MyCsClass().MAX_SAFE_INTEGER").Should().Be(JsValue.Undefined);
+    }
+
+    private class MyClass
+    {
+        public static JsNumber MAX_SAFE_INTEGER = new JsNumber(NumberConstructor.MaxSafeInteger);
+    }
 }

+ 15 - 0
Jint/Options.cs

@@ -371,6 +371,21 @@ public class Options
         /// All other values are ignored.
         /// </summary>
         public MemberTypes ObjectWrapperReportedMemberTypes { get; set; } = MemberTypes.Field | MemberTypes.Property | MemberTypes.Method;
+
+        /// <summary>
+        /// Reported member binding flags when reflecting, defaults to <see cref="BindingFlags.Instance" /> | <see cref="BindingFlags.Public" />.
+        /// </summary>
+        public BindingFlags ObjectWrapperReportedFieldBindingFlags { get; set; } = BindingFlags.Instance | BindingFlags.Public;
+
+        /// <summary>
+        /// Reported member binding flags when reflecting, defaults to <see cref="BindingFlags.Instance" /> | <see cref="BindingFlags.Public" />.
+        /// </summary>
+        public BindingFlags ObjectWrapperReportedPropertyBindingFlags { get; set; } = BindingFlags.Instance | BindingFlags.Public;
+
+        /// <summary>
+        /// Reported member binding flags when reflecting, defaults to <see cref="BindingFlags.Instance" /> | <see cref="BindingFlags.Public" /> | <see cref="BindingFlags.Static" />.
+        /// </summary>
+        public BindingFlags ObjectWrapperReportedMethodBindingFlags { get; set; } = BindingFlags.Instance | BindingFlags.Public | BindingFlags.Static;
     }
 
     public class ConstraintOptions

+ 1 - 1
Jint/Runtime/Interop/DefaultObjectConverter.cs

@@ -153,7 +153,7 @@ internal static class DefaultObjectConverter
                 }
             }
 
-            // if no known type could be guessed, use the default of wrapping using using ObjectWrapper.
+            // if no known type could be guessed, use the default of wrapping using ObjectWrapper
         }
 
         return result is not null;

+ 4 - 6
Jint/Runtime/Interop/ObjectWrapper.cs

@@ -255,12 +255,10 @@ public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable<ObjectWr
         {
             var interopOptions = _engine.Options.Interop;
 
-            // we take public properties, fields and methods
-            var bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
-
+            // we take properties, fields and methods
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Property) == MemberTypes.Property)
             {
-                foreach (var p in ClrType.GetProperties(bindingFlags))
+                foreach (var p in ClrType.GetProperties(interopOptions.ObjectWrapperReportedPropertyBindingFlags))
                 {
                     if (!interopOptions.TypeResolver.Filter(_engine, ClrType, p))
                     {
@@ -277,7 +275,7 @@ public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable<ObjectWr
 
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Field) == MemberTypes.Field)
             {
-                foreach (var f in ClrType.GetFields(bindingFlags))
+                foreach (var f in ClrType.GetFields(interopOptions.ObjectWrapperReportedFieldBindingFlags))
                 {
                     if (!interopOptions.TypeResolver.Filter(_engine, ClrType, f))
                     {
@@ -290,7 +288,7 @@ public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable<ObjectWr
 
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Method) == MemberTypes.Method)
             {
-                foreach (var m in ClrType.GetMethods(bindingFlags))
+                foreach (var m in ClrType.GetMethods(interopOptions.ObjectWrapperReportedMethodBindingFlags))
                 {
                     // we won't report anything from base object as it would usually not be something to expect from JS perspective
                     if (m.DeclaringType == typeof(object) || m.IsSpecialName || !interopOptions.TypeResolver.Filter(_engine, ClrType, m))

+ 2 - 4
Jint/Runtime/Interop/TypeReference.cs

@@ -27,12 +27,10 @@ public sealed class TypeReference : Constructor, IObjectWrapper
     {
         ReferenceType = type;
 
-        _prototype = engine.Realm.Intrinsics.Function.PrototypeObject;
+        _prototype = new TypeReferencePrototype(engine, this);
+        _prototypeDescriptor = new PropertyDescriptor(_prototype, PropertyFlag.AllForbidden);
         _length = PropertyDescriptor.AllForbiddenDescriptor.NumberZero;
 
-        var proto = new TypeReferencePrototype(engine, this);
-        _prototypeDescriptor = new PropertyDescriptor(proto, PropertyFlag.AllForbidden);
-
         PreventExtensions();
     }
 

+ 11 - 14
Jint/Runtime/Interop/TypeReferencePrototype.cs

@@ -1,26 +1,23 @@
-using Jint.Native;
-using Jint.Native.Object;
+using Jint.Collections;
+using Jint.Native;
+using Jint.Native.Symbol;
 using Jint.Runtime.Descriptors;
 
 namespace Jint.Runtime.Interop;
 
-internal sealed class TypeReferencePrototype : ObjectInstance
+internal sealed class TypeReferencePrototype : Prototype
 {
-    public TypeReferencePrototype(Engine engine, TypeReference typeReference) : base(engine)
+    public TypeReferencePrototype(Engine engine, TypeReference typeReference) : base(engine, engine.Realm)
     {
         TypeReference = typeReference;
         _prototype = engine.Realm.Intrinsics.Object.PrototypeObject;
-    }
-
-    public TypeReference TypeReference { get; }
 
-    public override PropertyDescriptor GetOwnProperty(JsValue property)
-    {
-        var descriptor = TypeReference.GetOwnProperty(property);
-        if (descriptor != PropertyDescriptor.Undefined)
+        var symbols = new SymbolDictionary(1)
         {
-            return descriptor;
-        }
-        return base.GetOwnProperty(property);
+            [GlobalSymbolRegistry.ToStringTag] = new PropertyDescriptor(typeReference.ReferenceType.Name, writable: false, enumerable: false, configurable: true),
+        };
+        SetSymbols(symbols);
     }
+
+    public TypeReference TypeReference { get; }
 }

+ 7 - 9
Jint/Runtime/Interop/TypeResolver.cs

@@ -116,11 +116,9 @@ public sealed class TypeResolver
         // 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;
-
         // properties and fields cannot be numbers
         if (!isInteger
-            && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp)
+            && TryFindMemberAccessor(engine, type, memberName, bindingFlags: null, indexer, out var temp)
             && (!mustBeReadable || temp.Readable)
             && (!mustBeWritable || temp.Writable))
         {
@@ -291,7 +289,7 @@ public sealed class TypeResolver
         Engine engine,
         [DynamicallyAccessedMembers(InteropHelper.DefaultDynamicallyAccessedMemberTypes | DynamicallyAccessedMemberTypes.Interfaces)] Type type,
         string memberName,
-        BindingFlags bindingFlags,
+        BindingFlags? bindingFlags,
         PropertyInfo? indexerToTry,
         [NotNullWhen(true)] out ReflectionAccessor? accessor)
     {
@@ -302,7 +300,7 @@ public sealed class TypeResolver
 
         PropertyInfo? GetProperty([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] Type t)
         {
-            foreach (var p in t.GetProperties(bindingFlags))
+            foreach (var p in t.GetProperties(bindingFlags ?? engine.Options.Interop.ObjectWrapperReportedPropertyBindingFlags))
             {
                 if (!Filter(engine, type, p))
                 {
@@ -358,7 +356,7 @@ public sealed class TypeResolver
 
         // look for a field
         FieldInfo? field = null;
-        foreach (var f in type.GetFields(bindingFlags))
+        foreach (var f in type.GetFields(bindingFlags ?? engine.Options.Interop.ObjectWrapperReportedFieldBindingFlags))
         {
             if (!Filter(engine, type, f))
             {
@@ -400,7 +398,7 @@ public sealed class TypeResolver
             }
         }
 
-        foreach (var m in type.GetMethods(bindingFlags))
+        foreach (var m in type.GetMethods(bindingFlags ?? engine.Options.Interop.ObjectWrapperReportedMethodBindingFlags))
         {
             AddMethod(m);
         }
@@ -425,7 +423,7 @@ public sealed class TypeResolver
         // Add Object methods to interface
         if (type.IsInterface)
         {
-            foreach (var m in typeof(object).GetMethods(bindingFlags))
+            foreach (var m in typeof(object).GetMethods(bindingFlags ?? engine.Options.Interop.ObjectWrapperReportedMethodBindingFlags))
             {
                 AddMethod(m);
             }
@@ -438,7 +436,7 @@ public sealed class TypeResolver
         }
 
         // look for nested type
-        var nestedType = type.GetNestedType(memberName, bindingFlags);
+        var nestedType = type.GetNestedType(memberName, bindingFlags ?? BindingFlags.Instance | BindingFlags.Public | BindingFlags.Static);
         if (nestedType != null)
         {
             var typeReference = TypeReference.CreateTypeReference(engine, nestedType);