Validate input string passed to LogValuesFormatter (#43238)

Add validation of a format string passed to LogValuesFormatter to throw `ArgumentNullException` if the string is `null` so that all the methods of the `LogMessage.Define` group depending on that class doesn't throw `NullReferenceException` when configuring a logger. Fix #36565
dotnet · Oct 14, 2020 · 2ca0b93 · 2ca0b93
1 parent 7ca6d85
commit 2ca0b93
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 0 deletions.
diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs
@@ -22,6 +22,11 @@ internal class LogValuesFormatter
 
         public LogValuesFormatter(string format)
         {
+            if (format == null)
+            {
+                throw new ArgumentNullException(nameof(format));
+            }
+
             OriginalFormat = format;
 
             var sb = new StringBuilder();

diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs
@@ -322,6 +322,26 @@ public void DefineScope_ThrowsException_WhenExpectedFormatStringParameterCount_N
             Assert.Equal(expectedMessage, exception.Message);
         }
 
+        [Theory]
+        [MemberData(nameof(DefineMethodsData))]
+        public void DefineMessage_ThrowsException_WhenFormatString_IsNull(Func<LogLevel, EventId, string, Delegate> define)
+        {
+            // Act
+            var exception = Assert.Throws<ArgumentNullException>(
+                () => define.Invoke(LogLevel.Error, 0, null));
+        }
+
+        public static IEnumerable<object[]> DefineMethodsData => new[]
+        {
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string> },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string, string> },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string, string, string> },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string, string, string, string> },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string, string, string, string, string> },
+            new object[] { (Func<LogLevel, EventId, string, Delegate>)LoggerMessage.Define<string, string, string, string, string, string> }
+        };
+
         public static IEnumerable<object[]> LogMessagesData => new[]
         {
             new object[] { LoggerMessage.Define(LogLevel.Error, 0, "Log "), 0 },