JsonLayout - Unwind after invalid property value to avoid invalid Json

NLog · Jan 16, 2021 · cb8d8fc · cb8d8fc
1 parent e1d8639
commit cb8d8fc
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 15 deletions.
diff --git a/src/NLog/Layouts/JsonLayout.cs b/src/NLog/Layouts/JsonLayout.cs
@@ -81,9 +81,9 @@ public bool SerializeObject(object value, StringBuilder builder)
                     return _converter.SerializeObject(value, builder);
             }
 
-            public void SerializeObjectNoLimit(object value, StringBuilder builder)
+            public bool SerializeObjectNoLimit(object value, StringBuilder builder)
             {
-                _converter.SerializeObject(value, builder);
+                return _converter.SerializeObject(value, builder);
             }
         }
 
@@ -371,7 +371,11 @@ private void AppendJsonPropertyValue(string propName, object propertyValue, stri
             if (MaxRecursionLimit <= 1 && captureType == MessageTemplates.CaptureType.Serialize)
             {
                 // Overrides MaxRecursionLimit as message-template tells us it is safe
-                JsonConverter.SerializeObjectNoLimit(propertyValue, sb);
+                if (!JsonConverter.SerializeObjectNoLimit(propertyValue, sb))
+                {
+                    sb.Length = initialLength;
+                    return;
+                }
             }
             else if (captureType == MessageTemplates.CaptureType.Stringify)
             {
@@ -382,7 +386,11 @@ private void AppendJsonPropertyValue(string propName, object propertyValue, stri
             }
             else
             {
-                JsonConverter.SerializeObject(propertyValue, sb);
+                if (!JsonConverter.SerializeObject(propertyValue, sb))
+                {
+                    sb.Length = initialLength;
+                    return;
+                }
             }
 
             if (ExcludeEmptyProperties && (sb[sb.Length-1] == '"' && sb[sb.Length-2] == '"'))

diff --git a/src/NLog/Targets/DefaultJsonSerializer.cs b/src/NLog/Targets/DefaultJsonSerializer.cs
@@ -257,13 +257,20 @@ private bool SerializeSimpleObjectValue(object value, StringBuilder destination,
                 SerializeSimpleTypeCodeValue(convertibleValue, objTypeCode, destination, options, forceToString);
                 return true;
             }
-
+            
             if (value is DateTimeOffset dateTimeOffset)
             {
                 QuoteValue(destination, dateTimeOffset.ToString("yyyy-MM-dd HH:mm:ss zzz", CultureInfo.InvariantCulture));
                 return true;
             }
 
+            if (value is IFormattable formattable)
+            {
+                var hasFormat = !StringHelpers.IsNullOrWhiteSpace(options.Format);
+                SerializeWithFormatProvider(formattable, true, destination, options, hasFormat);
+                return true;
+            }
+
             return false;   // Not simple
         }
 

diff --git a/tests/NLog.UnitTests/Layouts/JsonLayoutTests.cs b/tests/NLog.UnitTests/Layouts/JsonLayoutTests.cs
@@ -103,19 +103,15 @@ public void JsonLayoutRenderingEscapeUnicode()
                     {
                         new JsonAttribute("logger", "${logger}") { EscapeUnicode = true },
                         new JsonAttribute("level", "${level}"),
-                        new JsonAttribute("message", "${message}") { EscapeUnicode = false },
+                        new JsonAttribute("message", "${event-properties:msg}") { EscapeUnicode = false },
                     },
-                SuppressSpaces = true
-            };
-
-            var logEventInfo = new LogEventInfo
-            {
-                LoggerName = "\u00a9",
-                Level = LogLevel.Info,
-                Message = "\u00a9",
+                SuppressSpaces = true,
+                IncludeAllProperties = true,
             };
 
-            Assert.Equal("{\"logger\":\"\\u00a9\",\"level\":\"Info\",\"message\":\"\u00a9\"}", jsonLayout.Render(logEventInfo));
+            var logEventInfo = LogEventInfo.Create(LogLevel.Info, "\u00a9", null, "{$a}", new object[] { "\\" });
+            logEventInfo.Properties["msg"] = "\u00a9";
+            Assert.Equal("{\"logger\":\"\\u00a9\",\"level\":\"Info\",\"message\":\"\u00a9\",\"a\":\"\\\\\",\"msg\":\"\u00a9\"}", jsonLayout.Render(logEventInfo));
         }
 
         [Fact]
@@ -1042,6 +1038,40 @@ public void EscapeForwardSlashDefaultTest()
             AssertDebugLastMessage("debug", "{ \"myurl1\": \"http://hello.world.com/\", \"myurl2\": \"http:\\/\\/hello.world.com\\/\" }");
         }
 
+        [Fact]
+        public void SkipInvalidJsonPropertyValues()
+        {
+            var jsonLayout = new JsonLayout() { IncludeAllProperties = true };
+
+            var logEventInfo = new LogEventInfo
+            {
+                TimeStamp = new DateTime(2010, 01, 01, 12, 34, 56),
+                Level = LogLevel.Info,
+                Message = new System.Text.StringBuilder().Append('x', 1024 * 1024).ToString(),
+            };
+
+            var expectedValue = Guid.NewGuid();
+            logEventInfo.Properties["BadObject"] = new BadObject();
+            logEventInfo.Properties["RequestId"] = expectedValue;
+
+            Assert.Equal($"{{ \"RequestId\": \"{expectedValue}\" }}", jsonLayout.Render(logEventInfo));
+        }
+
+
+
+        class BadObject : IFormattable
+        {
+            public string ToString(string format, IFormatProvider formatProvider)
+            {
+                throw new ApplicationException("BadObject");
+            }
+
+            public override string ToString()
+            {
+                return ToString(null, null);
+            }
+        }
+
         private static LogEventInfo CreateLogEventWithExcluded()
         {
             var logEventInfo = new LogEventInfo