diff --git a/src/NLog/Internal/ObjectReflectionCache.cs b/src/NLog/Internal/ObjectReflectionCache.cs index c7e9df6..abb5a34 100644 --- a/src/NLog/Internal/ObjectReflectionCache.cs +++ b/src/NLog/Internal/ObjectReflectionCache.cs @@ -49,6 +49,8 @@ namespace NLog.Internal { private MruCache ObjectTypeCache => _objectTypeCache ?? System.Threading.Interlocked.CompareExchange(ref _objectTypeCache, new MruCache(10000), null) ?? _objectTypeCache; private MruCache? _objectTypeCache; + private MruCache ObjectTypeFieldCache => _objectTypeFieldCache ?? System.Threading.Interlocked.CompareExchange(ref _objectTypeFieldCache, new MruCache(10000), null) ?? _objectTypeFieldCache; + private MruCache? _objectTypeFieldCache; private readonly IServiceProvider _serviceProvider; private IObjectTypeTransformer ObjectTypeTransformation => _objectTypeTransformation ?? (_objectTypeTransformation = _serviceProvider?.GetService() ?? this); private IObjectTypeTransformer? _objectTypeTransformation; @@ -63,9 +65,14 @@ namespace NLog.Internal return null; } - public ObjectPropertyList LookupObjectProperties(object value) + public ObjectPropertyList LookupObjectProperties(object value) => LookupObjectProperties(value, false); + + /// + /// Lookup object members, optionally including public fields besides public properties. + /// + public ObjectPropertyList LookupObjectProperties(object value, bool includeFields) { - if (TryLookupExpandoObject(value, out var propertyValues)) + if (TryLookupExpandoObject(value, includeFields, out var propertyValues)) { return propertyValues; } @@ -90,8 +97,12 @@ namespace NLog.Internal } var objectType = value.GetType(); - var propertyInfos = CheckForObjectProperties(value) ? BuildObjectPropertyInfos(value) : ObjectPropertyInfos.SimpleToString; - ObjectTypeCache.TryAddValue(objectType, propertyInfos); + var typeCache = includeFields ? ObjectTypeFieldCache : ObjectTypeCache; + if (!typeCache.TryGetValue(objectType, out var propertyInfos)) + { + propertyInfos = CheckForObjectProperties(value) ? BuildObjectPropertyInfos(value, includeFields) : ObjectPropertyInfos.SimpleToString; + typeCache.TryAddValue(objectType, propertyInfos); + } return new ObjectPropertyList(value, propertyInfos.Properties, propertyInfos.FastLookup); } @@ -131,6 +142,9 @@ namespace NLog.Internal } public bool TryLookupExpandoObject(object value, out ObjectPropertyList objectPropertyList) + => TryLookupExpandoObject(value, false, out objectPropertyList); + + private bool TryLookupExpandoObject(object value, bool includeFields, out ObjectPropertyList objectPropertyList) { if (value is IDictionary expando) { @@ -148,7 +162,7 @@ namespace NLog.Internal #endif Type objectType = value.GetType(); - if (ObjectTypeCache.TryGetValue(objectType, out var propertyInfos)) + if (!includeFields && ObjectTypeCache.TryGetValue(objectType, out var propertyInfos)) { if (!propertyInfos.HasFastLookup) { @@ -163,9 +177,9 @@ namespace NLog.Internal var dictionaryEnumerator = TryGetDictionaryEnumerator(value); if (dictionaryEnumerator != null) { - propertyInfos = new ObjectPropertyInfos(ArrayHelper.Empty(), new[] { new FastPropertyLookup(string.Empty, TypeCode.Object, (o, p) => dictionaryEnumerator.GetEnumerator(o)) }); - ObjectTypeCache.TryAddValue(objectType, propertyInfos); - objectPropertyList = new ObjectPropertyList(value, propertyInfos.Properties, propertyInfos.FastLookup); + var dictionaryInfos = new ObjectPropertyInfos(ArrayHelper.Empty(), new[] { new FastPropertyLookup(string.Empty, TypeCode.Object, (o, p) => dictionaryEnumerator.GetEnumerator(o)) }); + ObjectTypeCache.TryAddValue(objectType, dictionaryInfos); + objectPropertyList = new ObjectPropertyList(value, dictionaryInfos.Properties, dictionaryInfos.FastLookup); return true; } @@ -174,15 +188,22 @@ namespace NLog.Internal } [UnconditionalSuppressMessage("Trimming - Allow reflection of message args", "IL2072")] - private static ObjectPropertyInfos BuildObjectPropertyInfos(object value) + private static ObjectPropertyInfos BuildObjectPropertyInfos(object value, bool includeFields) { - var properties = GetPublicProperties(value.GetType()); + var objectType = value.GetType(); + var properties = GetPublicProperties(objectType); + var fields = includeFields ? GetPublicFields(objectType) : ArrayHelper.Empty(); if (value is Exception) { // Special handling of Exception (Include Exception-Type as artificial first property) - var fastLookup = BuildFastLookup(properties, true); + var fastLookup = BuildFastLookup(properties, true, fields); return new ObjectPropertyInfos(properties, fastLookup); } + else if (fields.Length > 0) + { + // Fields are only reachable through the fast-lookup, so it cannot be built lazily + return new ObjectPropertyInfos(properties, BuildFastLookup(properties, false, fields)); + } else if (properties.Length == 0) { return ObjectPropertyInfos.SimpleToString; @@ -259,10 +280,61 @@ namespace NLog.Internal return properties ?? ArrayHelper.Empty(); } - private static FastPropertyLookup[] BuildFastLookup(PropertyInfo[] properties, bool includeType) + private static FieldInfo[] ExcludeNamesAlreadyTaken(FieldInfo[] fields, PropertyInfo[] properties, bool includeType) { + var taken = new HashSet(ObjectPropertyList.NameComparer); + if (includeType) + taken.Add("Type"); + foreach (var prop in properties) + taken.Add(prop.Name); + + FieldInfo[]? unique = null; + int count = 0; + for (int i = 0; i < fields.Length; ++i) + { + if (taken.Add(fields[i].Name)) + { + if (unique != null) + unique[count] = fields[i]; + ++count; + } + else if (unique is null) + { + unique = new FieldInfo[fields.Length - 1]; + Array.Copy(fields, unique, count); + } + } + + if (unique is null) + return fields; + + Array.Resize(ref unique, count); + return unique; + } + + private static FieldInfo[] GetPublicFields([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] Type type) + { + try + { + return type.GetFields(BindingFlags.Instance | BindingFlags.Public); + } + catch (Exception ex) + { + InternalLogger.Warn(ex, "Failed to get object fields for type: {0}", type); + return ArrayHelper.Empty(); + } + } + + private static FastPropertyLookup[] BuildFastLookup(PropertyInfo[] properties, bool includeType, FieldInfo[]? fields = null) + { + // A field can share its name with an inherited property, or with the artificial + // Type member. Properties win, so that including fields only adds JSON members. + if (fields?.Length > 0) + fields = ExcludeNamesAlreadyTaken(fields, properties, includeType); + + int fieldCount = fields?.Length ?? 0; int fastAccessIndex = includeType ? 1 : 0; - FastPropertyLookup[] fastLookup = new FastPropertyLookup[properties.Length + fastAccessIndex]; + FastPropertyLookup[] fastLookup = new FastPropertyLookup[properties.Length + fieldCount + fastAccessIndex]; if (includeType) { fastLookup[0] = new FastPropertyLookup("Type", TypeCode.String, (o, p) => o.GetType().ToString()); @@ -284,6 +356,14 @@ namespace NLog.Internal fastLookup[fastAccessIndex++] = new FastPropertyLookup(prop.Name, typeCode, valueLookup); } } + + for (int i = 0; i < fieldCount; ++i) + { + var field = fields![i]; + TypeCode typeCode = Type.GetTypeCode(field.FieldType); + fastLookup[fastAccessIndex++] = new FastPropertyLookup(field.Name, typeCode, (o, p) => field.GetValue(o)); + } + return fastLookup; } @@ -366,9 +446,13 @@ namespace NLog.Internal _fastLookup = CreateIDictionaryEnumerator; } + // The expando enumerator is stored as a single nameless entry, which no real member can be + private static bool IsExpandoLookup(FastPropertyLookup[] fastLookup) + => fastLookup.Length == 1 && fastLookup[0].Name.Length == 0; + public bool TryGetPropertyValue(string name, out PropertyValue propertyValue) { - if (_properties.Length == 0) + if (_properties.Length == 0 && (_fastLookup is null || _fastLookup.Length == 0 || IsExpandoLookup(_fastLookup))) { if (_object is IDictionary expandoObject) { @@ -444,7 +528,7 @@ namespace NLog.Internal public Enumerator GetEnumerator() { - if (_properties.Length > 0 || _fastLookup is null || _fastLookup.Length == 0) + if (_properties.Length > 0 || _fastLookup is null || _fastLookup.Length == 0 || !IsExpandoLookup(_fastLookup)) return new Enumerator(_object, _properties, _fastLookup); var expandoObject = _fastLookup[0].ValueLookup(_object, ArrayHelper.Empty()); diff --git a/src/NLog/Targets/DefaultJsonSerializer.cs b/src/NLog/Targets/DefaultJsonSerializer.cs index a31e523..15ec735 100644 --- a/src/NLog/Targets/DefaultJsonSerializer.cs +++ b/src/NLog/Targets/DefaultJsonSerializer.cs @@ -233,7 +233,7 @@ namespace NLog.Targets } else { - var objectPropertyList = _objectReflectionCache.LookupObjectProperties(value); + var objectPropertyList = _objectReflectionCache.LookupObjectProperties(value, options.IncludePublicFields); return SerializeObjectPropertyList(value, ref objectPropertyList, destination, options, ref objectsInPath, depth); } } @@ -730,6 +730,7 @@ namespace NLog.Targets return true; } + private bool SerializeObjectAsString(object value, StringBuilder destination, JsonSerializeOptions options) { var originalLength = destination.Length; diff --git a/src/NLog/Targets/JsonSerializeOptions.cs b/src/NLog/Targets/JsonSerializeOptions.cs index bff64fe..545b77f 100644 --- a/src/NLog/Targets/JsonSerializeOptions.cs +++ b/src/NLog/Targets/JsonSerializeOptions.cs @@ -104,5 +104,11 @@ namespace NLog.Targets /// /// Default: public int MaxRecursionLimit { get; set; } = 10; + + /// + /// Should public fields be included in the JSON output, in addition to public properties. + /// + /// Default: + public bool IncludePublicFields { get; set; } } } diff --git a/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs b/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs index ffb7b15..286a93a 100644 --- a/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs +++ b/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs @@ -69,6 +69,32 @@ namespace NLog.UnitTests.Targets public IncludedClass Included { get; set; } } + private sealed class ClassWithPublicField + { + public string Name { get; set; } + public int Age; + } + + private sealed class ClassWithOnlyFields + { + public string Name; + public int Age; + } + + private sealed class ClassWithNestedField + { + public IncludedClass Nested; + public string Skipped = null; // A null field must be left out of the JSON + } + + private sealed class ClassWithMixedVisibility + { + public string Public; + internal string Internal; + private string _private = "hidden"; + public string ReadPrivate() => _private; + } + private static ContainerClass BuildSampleObject() { var testObject = new ContainerClass @@ -131,5 +157,146 @@ namespace NLog.UnitTests.Targets @"{""S"":""sample"",""Excluded"":""Skipped"",""Included"":{""IncludedString"":""serialized""}}"; Assert.Equal(expectedValue, sb.ToString()); } + + [Fact] + public void IncludePublicFields_Enabled_SerializesPublicFieldsAlongsideProperties() + { + var testObject = new ClassWithPublicField { Name = "John", Age = 42 }; + + var sb = new StringBuilder(); + var options = new JsonSerializeOptions { IncludePublicFields = true }; + + var jsonSerializer = new DefaultJsonSerializer(null); + jsonSerializer.SerializeObject(testObject, sb, options); + + Assert.Equal(@"{""Name"":""John"",""Age"":42}", sb.ToString()); + } + + [Fact] + public void IncludePublicFields_Disabled_SkipsPublicFields() + { + var testObject = new ClassWithPublicField { Name = "John", Age = 42 }; + + var sb = new StringBuilder(); + var options = new JsonSerializeOptions { IncludePublicFields = false }; + + var jsonSerializer = new DefaultJsonSerializer(null); + jsonSerializer.SerializeObject(testObject, sb, options); + + Assert.Equal(@"{""Name"":""John""}", sb.ToString()); + } + + [Fact] + public void IncludePublicFields_ClassWithoutProperties_SerializesFields() + { + var json = SerializeWithFields(new ClassWithOnlyFields { Name = "John", Age = 42 }); + Assert.Equal(@"{""Name"":""John"",""Age"":42}", json); + } + + [Fact] + public void IncludePublicFields_NullFieldValue_IsSkipped() + { + var json = SerializeWithFields(new ClassWithNestedField { Nested = new IncludedClass { IncludedString = "abc" } }); + Assert.Equal(@"{""Nested"":{""IncludedString"":""abc""}}", json); + } + + [Fact] + public void IncludePublicFields_NonPublicFields_AreNotSerialized() + { + var testObject = new ClassWithMixedVisibility { Public = "yes", Internal = "no" }; + Assert.Equal("hidden", testObject.ReadPrivate()); + Assert.Equal(@"{""Public"":""yes""}", SerializeWithFields(testObject)); + } + + [Fact] + public void IncludePublicFields_WithoutSuppressSpaces_SeparatesWithSpace() + { + var sb = new StringBuilder(); + var options = new JsonSerializeOptions { IncludePublicFields = true, SuppressSpaces = false }; + new DefaultJsonSerializer(null).SerializeObject(new ClassWithOnlyFields { Name = "John", Age = 42 }, sb, options); + Assert.Equal(@"{""Name"":""John"", ""Age"":42}", sb.ToString()); + } + + [Fact] + public void IncludePublicFields_SameTypeWithAndWithoutOption_DoesNotShareCachedMembers() + { + var testObject = new ClassWithPublicField { Name = "John", Age = 42 }; + var jsonSerializer = new DefaultJsonSerializer(null); + + var withFields = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withFields, new JsonSerializeOptions { IncludePublicFields = true }); + var withoutFields = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withoutFields, new JsonSerializeOptions { IncludePublicFields = false }); + var withFieldsAgain = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withFieldsAgain, new JsonSerializeOptions { IncludePublicFields = true }); + + Assert.Equal(@"{""Name"":""John"",""Age"":42}", withFields.ToString()); + Assert.Equal(@"{""Name"":""John""}", withoutFields.ToString()); + Assert.Equal(@"{""Name"":""John"",""Age"":42}", withFieldsAgain.ToString()); + } + + [Fact] + public void IncludePublicFields_ExpandoObject_StillEnumeratesEntries() + { + var testObject = new System.Collections.Generic.Dictionary { { "Name", "John" }, { "Age", 42 } }; + Assert.Equal(@"{""Name"":""John"",""Age"":42}", SerializeWithFields(testObject)); + } + + [Fact] + public void IncludePublicFields_Exception_KeepsArtificialTypeProperty() + { + var json = SerializeWithFields(new System.InvalidOperationException("Oops")); + Assert.Contains(@"""Type"":""System.InvalidOperationException""", json); + Assert.Contains(@"""Message"":""Oops""", json); + } + + [Fact] + public void IncludePublicFields_TypeWithoutFields_IsStableAcrossCalls() + { + var testObject = new IncludedClass { IncludedString = "abc" }; + Assert.Equal(@"{""IncludedString"":""abc""}", SerializeWithFields(testObject)); + Assert.Equal(@"{""IncludedString"":""abc""}", SerializeWithFields(testObject)); + } + + private class BaseWithProperty + { + public string Shadowed { get; set; } + } + + private sealed class DerivedWithField : BaseWithProperty + { + public new string Shadowed; + } + + private sealed class ExceptionWithTypeField : System.Exception + { + public string Type = "collides"; + } + [Fact] + public void IncludePublicFields_FieldShadowingInheritedProperty_EmitsMemberOnce() + { + var testObject = new DerivedWithField { Shadowed = "field" }; + ((BaseWithProperty)testObject).Shadowed = "property"; + + Assert.Equal(@"{""Shadowed"":""property""}", SerializeWithFields(testObject)); + } + + [Fact] + public void IncludePublicFields_ExceptionWithTypeField_KeepsArtificialTypeOnly() + { + var json = SerializeWithFields(new ExceptionWithTypeField()); + + Assert.Contains(@"""Type"":""" + typeof(ExceptionWithTypeField).ToString() + @"""", json); + Assert.DoesNotContain(@"""Type"":""collides""", json); + } + + + private static string SerializeWithFields(object value) + { + var sb = new StringBuilder(); + var options = new JsonSerializeOptions { IncludePublicFields = true, SuppressSpaces = true }; + new DefaultJsonSerializer(null).SerializeObject(value, sb, options); + return sb.ToString(); + } } }