Throw FormatException when IndexOutOfRangeException caused by invalid…

… parameter list
NLog · Mar 24, 2019 · 8842f2f · 8842f2f
1 parent d75b1fe
commit 8842f2f
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 11 deletions.
diff --git a/src/NLog.Extensions.Logging/Logging/NLogMessageParameterList.cs b/src/NLog.Extensions.Logging/Logging/NLogMessageParameterList.cs
@@ -84,19 +84,27 @@ private static bool IsValidParameterList(IReadOnlyList<KeyValuePair<string, obje
             bool? firstParameterIsPositional = null;
             for (int i = 0; i < parameterList.Count; ++i)
             {
-                var paramPair = parameterList[i];
-                if (string.IsNullOrEmpty(paramPair.Key))
+                string parameterKey;
+                try
+                {
+                    parameterKey = parameterList[i].Key;
+                }
+                catch (IndexOutOfRangeException ex)
+                {
+                    throw new FormatException($"Invalid format string. Expected {parameterList.Count - 1} format parameters, but failed to lookup parameter index {i}", ex);
+                }
+                if (string.IsNullOrEmpty(parameterKey))
                 {
                     originalMessageIndex = null;
                     return false;
                 }
 
-                char firstChar = paramPair.Key[0];
+                char firstChar = parameterKey[0];
                 if (GetCaptureType(firstChar) != CaptureType.Normal)
                 {
                     hasMessageTemplateCapture = true;
                 }
-                else if (paramPair.Key == NLogLogger.OriginalFormatPropertyName)
+                else if (parameterKey == NLogLogger.OriginalFormatPropertyName)
                 {
                     if (originalMessageIndex.HasValue)
                     {

diff --git a/test/NLog.Extensions.Logging.Tests/LoggerTests.cs b/test/NLog.Extensions.Logging.Tests/LoggerTests.cs
@@ -115,6 +115,34 @@ public void TestScopePropertyDictionary()
             Assert.Equal("NLog.Extensions.Logging.Tests.LoggerTests.Runner|DEBUG|message with id and 1 parameters |Hello", target.Logs.LastOrDefault());
         }
 
+        [Fact]
+        public void TestInvalidFormatString()
+        {
+            try
+            {
+                GetRunner().Log(Microsoft.Extensions.Logging.LogLevel.Information, 0, null, "{0}{1}", "Test");
+                Assert.False(true, "Exception must be thrown");
+            }
+            catch (AggregateException ex)
+            {
+                Assert.IsType<FormatException>(ex.InnerException);
+            }
+        }
+
+        [Fact]
+        public void TestInvalidFormatString2()
+        {
+            try
+            {
+                GetRunner<Runner>(new NLogProviderOptions() { CaptureMessageTemplates = false }).Log(Microsoft.Extensions.Logging.LogLevel.Information, 0, null, "{0}{1}", "Test");
+                Assert.False(true, "Exception must be thrown");
+            }
+            catch (AggregateException ex)
+            {
+                Assert.IsType<FormatException>(ex.InnerException);
+            }
+        }
+
         [Theory]
         [InlineData(Microsoft.Extensions.Logging.LogLevel.Critical, "NLog.Extensions.Logging.Tests.LoggerTests.Runner|FATAL|message Exception of type 'System.Exception' was thrown.|20")]
         [InlineData(Microsoft.Extensions.Logging.LogLevel.Debug, "NLog.Extensions.Logging.Tests.LoggerTests.Runner|DEBUG|message Exception of type 'System.Exception' was thrown.|20")]
@@ -231,27 +259,27 @@ public void LogDebugWithId()
                 _logger.LogDebug(20, "message with id");
             }
 
-            public void Log(Microsoft.Extensions.Logging.LogLevel logLevel, int eventId, Exception exception, string message)
+            public void Log(Microsoft.Extensions.Logging.LogLevel logLevel, int eventId, Exception exception, string message, params object[] args)
             {
                 switch (logLevel)
                 {
                     case Microsoft.Extensions.Logging.LogLevel.Trace:
-                        _logger.LogTrace(eventId, exception, message);
+                        _logger.LogTrace(eventId, exception, message, args);
                         break;
                     case Microsoft.Extensions.Logging.LogLevel.Debug:
-                        _logger.LogDebug(eventId, exception, message);
+                        _logger.LogDebug(eventId, exception, message, args);
                         break;
                     case Microsoft.Extensions.Logging.LogLevel.Information:
-                        _logger.LogInformation(eventId, exception, message);
+                        _logger.LogInformation(eventId, exception, message, args);
                         break;
                     case Microsoft.Extensions.Logging.LogLevel.Warning:
-                        _logger.LogWarning(eventId, exception, message);
+                        _logger.LogWarning(eventId, exception, message, args);
                         break;
                     case Microsoft.Extensions.Logging.LogLevel.Error:
-                        _logger.LogError(eventId, exception, message);
+                        _logger.LogError(eventId, exception, message, args);
                         break;
                     case Microsoft.Extensions.Logging.LogLevel.Critical:
-                        _logger.LogCritical(eventId, exception, message);
+                        _logger.LogCritical(eventId, exception, message, args);
                         break;
                     default:
                         throw new ArgumentOutOfRangeException(nameof(logLevel), logLevel, null);