diff --git a/src/Serilog.Expressions/Expressions/Compilation/ExpressionValidationException.cs b/src/Serilog.Expressions/Expressions/Compilation/ExpressionValidationException.cs new file mode 100644 index 0000000..4fb57ea --- /dev/null +++ b/src/Serilog.Expressions/Expressions/Compilation/ExpressionValidationException.cs @@ -0,0 +1,26 @@ +// Copyright © Serilog Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Serilog.Expressions.Compilation; + +class ExpressionValidationException : ArgumentException +{ + public ExpressionValidationException(string message) : base(message) + { + } + + public ExpressionValidationException(string message, Exception innerException) : base(message, innerException) + { + } +} \ No newline at end of file diff --git a/src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs b/src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs index 2c6c99c..43e2b98 100644 --- a/src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs +++ b/src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs @@ -16,6 +16,7 @@ using System.Reflection; using System.Runtime.InteropServices; using System.Text; +using Serilog.Debugging; using Serilog.Events; using Serilog.Expressions.Ast; using Serilog.Expressions.Compilation.Transformations; @@ -99,12 +100,24 @@ ExpressionBody Splice(Expression lambda) protected override ExpressionBody Transform(CallExpression call) { if (!_nameResolver.TryResolveFunctionName(call.OperatorName, out var m)) - throw new ArgumentException($"The function name `{call.OperatorName}` was not recognized."); + throw new ExpressionValidationException($"The function name `{call.OperatorName}` was not recognized."); var methodParameters = m.GetParameters() .Select(info => (pi: info, optional: info.GetCustomAttribute() != null)) .ToList(); + // Log warning for CI modifier usage on functions that don't support it + // Note: We log a warning rather than throwing to maintain backward compatibility + // Previously, invalid CI usage was silently ignored + if (call.IgnoreCase) + { + var supportsStringComparison = methodParameters.Any(p => p.pi.ParameterType == typeof(StringComparison)); + if (!supportsStringComparison) + { + SelfLog.WriteLine($"The function `{call.OperatorName}` does not support case-insensitive operation; the 'ci' modifier will be ignored."); + } + } + var allowedParameters = methodParameters.Where(info => info.pi.ParameterType == typeof(LogEventPropertyValue)).ToList(); var requiredParameterCount = allowedParameters.Count(info => !info.optional); diff --git a/src/Serilog.Expressions/Expressions/Compilation/Text/TextMatchingTransformer.cs b/src/Serilog.Expressions/Expressions/Compilation/Text/TextMatchingTransformer.cs index 9fe3ecb..7392e1b 100644 --- a/src/Serilog.Expressions/Expressions/Compilation/Text/TextMatchingTransformer.cs +++ b/src/Serilog.Expressions/Expressions/Compilation/Text/TextMatchingTransformer.cs @@ -51,14 +51,21 @@ Expression TryCompileIndexOfMatch(bool ignoreCase, Expression corpus, Expression { if (regex is ConstantExpression { Constant: ScalarValue { Value: string s } }) { - var opts = RegexOptions.Compiled | RegexOptions.ExplicitCapture; - if (ignoreCase) - opts |= RegexOptions.IgnoreCase; - var compiled = new Regex(s, opts, TimeSpan.FromMilliseconds(100)); - return new IndexOfMatchExpression(Transform(corpus), compiled); + try + { + var opts = RegexOptions.Compiled | RegexOptions.ExplicitCapture; + if (ignoreCase) + opts |= RegexOptions.IgnoreCase; + var compiled = new Regex(s, opts, TimeSpan.FromMilliseconds(100)); + return new IndexOfMatchExpression(Transform(corpus), compiled); + } + catch (ArgumentException ex) + { + throw new ExpressionValidationException($"Invalid regular expression in IndexOfMatch: {ex.Message}", ex); + } } - SelfLog.WriteLine($"Serilog.Expressions: `IndexOfMatch()` requires a constant string regular expression argument; found ${regex}."); + SelfLog.WriteLine($"Serilog.Expressions: `IndexOfMatch()` requires a constant string regular expression argument; found {regex}."); return new CallExpression(false, Operators.OpUndefined); } } \ No newline at end of file diff --git a/src/Serilog.Expressions/Expressions/SerilogExpression.cs b/src/Serilog.Expressions/Expressions/SerilogExpression.cs index 9416c11..8233dbe 100644 --- a/src/Serilog.Expressions/Expressions/SerilogExpression.cs +++ b/src/Serilog.Expressions/Expressions/SerilogExpression.cs @@ -53,8 +53,8 @@ public static CompiledExpression Compile(string expression, /// A function that evaluates the expression in the context of a log event. /// The reported error, if compilation was unsuccessful. /// True if the function could be created; otherwise, false. - /// Regular expression syntax errors currently generate exceptions instead of producing friendly - /// errors. + /// Validation errors including invalid regular expressions and unknown function names are returned + /// as friendly error messages. Invalid case-insensitive modifiers are ignored with warnings. public static bool TryCompile( string expression, [MaybeNullWhen(false)] out CompiledExpression result, @@ -75,8 +75,8 @@ public static bool TryCompile( /// A function that evaluates the expression in the context of a log event. /// The reported error, if compilation was unsuccessful. /// True if the function could be created; otherwise, false. - /// Regular expression syntax errors currently generate exceptions instead of producing friendly - /// errors. + /// Validation errors including invalid regular expressions and unknown function names are returned + /// as friendly error messages. Invalid case-insensitive modifiers are ignored with warnings. public static bool TryCompile(string expression, IFormatProvider? formatProvider, NameResolver nameResolver, @@ -101,10 +101,20 @@ static bool TryCompileImpl(string expression, return false; } - var evaluate = ExpressionCompiler.Compile(root, formatProvider, DefaultFunctionNameResolver.Build(nameResolver)); - result = evt => evaluate(new(evt)); - error = null; - return true; + try + { + var evaluate = ExpressionCompiler.Compile(root, formatProvider, DefaultFunctionNameResolver.Build(nameResolver)); + result = evt => evaluate(new(evt)); + error = null; + return true; + } + catch (ExpressionValidationException ex) + { + // Catch validation errors from compilation + result = null; + error = ex.Message; + return false; + } } /// diff --git a/test/Serilog.Expressions.Tests/ExpressionValidationTests.cs b/test/Serilog.Expressions.Tests/ExpressionValidationTests.cs new file mode 100644 index 0000000..b7948dc --- /dev/null +++ b/test/Serilog.Expressions.Tests/ExpressionValidationTests.cs @@ -0,0 +1,184 @@ +using Xunit; + +namespace Serilog.Expressions.Tests; + +public class ExpressionValidationTests +{ + [Theory] + [InlineData("IsMatch(Name, '[invalid')", "Invalid regular expression")] + [InlineData("IndexOfMatch(Text, '(?<')", "Invalid regular expression")] + [InlineData("IsMatch(Name, '(?P)')", "Invalid regular expression")] + [InlineData("IsMatch(Name, '(unclosed')", "Invalid regular expression")] + [InlineData("IndexOfMatch(Text, '*invalid')", "Invalid regular expression")] + public void InvalidRegularExpressionsAreReportedGracefully(string expression, string expectedErrorFragment) + { + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + Assert.False(result); + Assert.Contains(expectedErrorFragment, error); + Assert.Null(compiled); + } + + [Theory] + [InlineData("UnknownFunction()", "The function name `UnknownFunction` was not recognized.")] + [InlineData("foo(1, 2, 3)", "The function name `foo` was not recognized.")] + [InlineData("MyCustomFunc(Name)", "The function name `MyCustomFunc` was not recognized.")] + [InlineData("notAFunction()", "The function name `notAFunction` was not recognized.")] + public void UnknownFunctionNamesAreReportedGracefully(string expression, string expectedError) + { + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + Assert.False(result); + Assert.Equal(expectedError, error); + Assert.Null(compiled); + } + + [Theory] + [InlineData("Length(Name) ci")] + [InlineData("Round(Value, 2) ci")] + [InlineData("Now() ci")] + [InlineData("TypeOf(Value) ci")] + [InlineData("IsDefined(Prop) ci")] + public void InvalidCiModifierUsageCompilesWithWarning(string expression) + { + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + Assert.True(result, $"Failed to compile: {error}"); + Assert.NotNull(compiled); + Assert.Null(error); + } + + [Theory] + [InlineData("Contains(Name, 'test') ci")] + [InlineData("StartsWith(Path, '/api') ci")] + [InlineData("EndsWith(File, '.txt') ci")] + [InlineData("IsMatch(Email, '@example') ci")] + [InlineData("IndexOfMatch(Text, 'pattern') ci")] + [InlineData("IndexOf(Name, 'x') ci")] + [InlineData("Name = 'test' ci")] + [InlineData("Name <> 'test' ci")] + [InlineData("Name like '%test%' ci")] + public void ValidCiModifierUsageCompiles(string expression) + { + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + Assert.True(result, $"Failed to compile: {error}"); + Assert.NotNull(compiled); + Assert.Null(error); + } + + [Fact] + public void FirstErrorIsReportedInComplexExpressions() + { + var expression = "UnknownFunc() and Length(Value) > 5"; + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + + Assert.False(result); + Assert.Null(compiled); + + // Should report the first error encountered + Assert.Contains("UnknownFunc", error); + Assert.NotNull(error); + } + + [Fact] + public void ValidExpressionsStillCompileWithoutErrors() + { + var validExpressions = new[] + { + "IsMatch(Name, '^[A-Z]')", + "IndexOfMatch(Text, '\\d+')", + "Contains(Name, 'test') ci", + "Length(Items) > 5", + "Round(Value, 2)", + "TypeOf(Data) = 'String'", + "Name like '%test%'", + "StartsWith(Path, '/') and EndsWith(Path, '.json')" + }; + + foreach (var expr in validExpressions) + { + var result = SerilogExpression.TryCompile(expr, out var compiled, out var error); + Assert.True(result, $"Failed to compile: {expr}. Error: {error}"); + Assert.NotNull(compiled); + Assert.Null(error); + } + } + + [Fact] + public void CompileMethodStillThrowsForInvalidExpressions() + { + // Ensure Compile() method (not TryCompile) maintains throwing behavior for invalid expressions + Assert.Throws(() => + SerilogExpression.Compile("UnknownFunction()")); + + Assert.Throws(() => + SerilogExpression.Compile("IsMatch(Name, '[invalid')")); + + // CI modifier on non-supporting functions compiles with warning + var compiledWithCi = SerilogExpression.Compile("Length(Name) ci"); + Assert.NotNull(compiledWithCi); + + Assert.Throws(() => + SerilogExpression.Compile("IndexOfMatch(Text, '(?<')")); + } + + [Theory] + [InlineData("IsMatch(Name, Name)")] // Non-constant pattern + [InlineData("IndexOfMatch(Text, Value)")] // Non-constant pattern + public void NonConstantRegexPatternsHandledGracefully(string expression) + { + // These should compile but may log warnings (not errors) + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + + // These compile successfully but return undefined at runtime + Assert.True(result); + Assert.NotNull(compiled); + Assert.Null(error); + } + + [Fact] + public void RegexTimeoutIsRespected() + { + // This regex should compile fine - timeout only matters at runtime + var expression = @"IsMatch(Text, '(a+)+b')"; + + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + + Assert.True(result); + Assert.NotNull(compiled); + Assert.Null(error); + } + + [Fact] + public void ComplexExpressionsReportFirstError() + { + var expression = "UnknownFunc1() or Length(Value) > 5"; + var result = SerilogExpression.TryCompile(expression, out var compiled, out var error); + + Assert.False(result); + Assert.Null(compiled); + Assert.NotNull(error); + + // Should report the first error encountered during compilation + Assert.Contains("UnknownFunc1", error); + } + + [Fact] + public void BackwardCompatibilityPreservedForInvalidCiUsage() + { + // These previously compiled (CI was silently ignored) + // They should still compile with the new changes + var expressions = new[] + { + "undefined() ci", + "null = undefined() ci", + "Length(Name) ci", + "Round(Value, 2) ci" + }; + + foreach (var expr in expressions) + { + var result = SerilogExpression.TryCompile(expr, out var compiled, out var error); + Assert.True(result, $"Breaking change detected: {expr} no longer compiles. Error: {error}"); + Assert.NotNull(compiled); + Assert.Null(error); + } + } +} \ No newline at end of file