From 1bebd2ef9e35a3cf6d51dfe417220d7c092d3d6e Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Thu, 1 Oct 2026 20:28:07 +0300 Subject: [PATCH] fix(filter): throw ParsingException for a '.' number that the culture cannot read In a culture whose decimal separator is not '.', v1.14.2 read only the part of a '.' number before the '.'. Then the grammar failed at the '.'. Thus a '.' number on an integer property gave ParsingException, and main gave FormatException. A number that the culture cannot read in full now builds the clause like main. If that build fails, the parser builds it again with the part that the culture reads, and throws ParsingException if this build succeeds. Lists throw ParsingException, like v1.14.2. en-US is unchanged. A '.' number on a decimal property still filters. --- .../Tests/DotNumberCultureTests.cs | 55 ++++++++++++ QueryKit.UnitTests/DotNumberCultureTests.cs | 66 +++++++++++++++ QueryKit/FilterParser.cs | 84 +++++++++++++++---- 3 files changed, 189 insertions(+), 16 deletions(-) create mode 100644 QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs create mode 100644 QueryKit.UnitTests/DotNumberCultureTests.cs diff --git a/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs b/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs new file mode 100644 index 0000000..22454fc --- /dev/null +++ b/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs @@ -0,0 +1,55 @@ +namespace QueryKit.IntegrationTests.Tests; + +using System.Globalization; +using Exceptions; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using SharedTestingHelper.Fakes; +using WebApiTestProject.Entities; + +// Like v1.14.2, a '.' number on an integer property throws ParsingException in a culture +// whose decimal separator is not '.', and a '.' number on a decimal property still filters. +public class DotNumberCultureTests : TestBase +{ + [Fact] + public async Task dot_number_on_an_integer_property_throws_parsing_exception_in_de_de() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = $"culture {Guid.NewGuid()}"; + var fakePersonOne = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(5) + .WithRating(4.6M) + .Build(); + var fakePersonTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(3) + .WithRating(4.4M) + .Build(); + await testingServiceScope.InsertAsync(fakePersonOne, fakePersonTwo); + + // Act + var originalCulture = CultureInfo.CurrentCulture; + IQueryable appliedQueryable; + try + { + CultureInfo.CurrentCulture = new CultureInfo("de-DE"); + var integerFilter = () => testingServiceScope.DbContext().People + .ApplyQueryKitFilter($"""Title == "{title}" && Age > 4.4"""); + integerFilter.Should().ThrowExactly(); + + appliedQueryable = testingServiceScope.DbContext().People + .ApplyQueryKitFilter($"""Title == "{title}" && Rating > 4.5"""); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(fakePersonOne.Id); + } +} diff --git a/QueryKit.UnitTests/DotNumberCultureTests.cs b/QueryKit.UnitTests/DotNumberCultureTests.cs new file mode 100644 index 0000000..89c99f4 --- /dev/null +++ b/QueryKit.UnitTests/DotNumberCultureTests.cs @@ -0,0 +1,66 @@ +namespace QueryKit.UnitTests; + +using System.Globalization; +using Exceptions; +using FluentAssertions; +using WebApiTestProject.Entities.Recipes; + +// In a culture whose decimal separator is not '.', v1.14.2 read only the part of a '.' number before the '.'. +// A '.' number that does not convert to the property type gives the same exception type as v1.14.2. +public class DotNumberCultureTests +{ + [Theory] + [InlineData("de-DE", "Rating > 4.4")] + [InlineData("fr-FR", "Rating > 4.4")] + [InlineData("de-DE", "Rating == -4.0")] + [InlineData("de-DE", "Rating > 4.4 || Title == \"x\"")] + [InlineData("de-DE", "Rating == .5")] + [InlineData("de-DE", "Rating ^^ [4.0]")] + [InlineData("de-DE", "Rating ^^ [\"x\", 4.0]")] + [InlineData("de-DE", "Ingredients.MinimumQuality == 0.0")] + [InlineData("de-DE", "Ingredients.QualityLevel > 4.5")] + [InlineData("de-DE", "Tags #== 2.0")] + [InlineData("de-DE", "(Rating, Title) == 4.0")] + public void dot_number_on_an_integer_property_throws_parsing_exception(string cultureName, string input) + { + var act = () => WithCulture(cultureName, () => FilterParser.ParseFilter(input)); + + act.Should().ThrowExactly(); + } + + [Theory] + [InlineData("en-US", "Rating > 4.4")] + [InlineData("de-DE", "Rating > 4,4")] + [InlineData("de-DE", "Rating > \"4.4\"")] + [InlineData("de-DE", "Rating ^^ [\"4.0\"]")] + [InlineData("de-DE", "Rating > @4.4")] + [InlineData("de-DE", "HaveMadeItMyself == 4.4")] + public void number_that_v1_14_2_also_converted_throws_format_exception(string cultureName, string input) + { + var act = () => WithCulture(cultureName, () => FilterParser.ParseFilter(input)); + + act.Should().ThrowExactly(); + } + + [Fact] + public void integer_value_still_filters_in_a_comma_culture() + { + var filterExpression = WithCulture("de-DE", () => FilterParser.ParseFilter("Rating > 4")); + + filterExpression.ToString().Should().Be("x => (x.Rating > 4)"); + } + + private static TResult WithCulture(string cultureName, Func action) + { + var originalCulture = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo(cultureName); + return action(); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + } +} diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 06b22bb..2ec3935 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -256,6 +256,51 @@ from sign in Parse.Char('-').Optional().Select(x => x.IsDefined ? "-" : "") from number in Parse.DecimalInvariant select sign + number; + // v1.14.2 read a number only with the decimal separator of the current culture. + private static readonly Parser CultureNumberParser = + from sign in Parse.Char('-').Optional().Select(x => x.IsDefined ? "-" : "") + from number in Parse.Decimal + select sign + number; + + // The part of a number that v1.14.2 read: null when the current culture reads the whole number, + // else the part before the '.' (for example "4" of "4.5" in de-DE), or "" when the culture reads no number. + private static string? CultureNumberPrefix(string number) + { + var result = CultureNumberParser.TryParse(number); + if (!result.WasSuccessful) + { + return ""; + } + + return result.Value.Length == number.Length ? null : result.Value; + } + + // In a culture whose decimal separator is not '.', v1.14.2 read only the prefix of a '.' number (see CultureNumberPrefix), + // built the clause with that prefix, and then failed in the grammar at the '.'. + // If the clause builds with the whole number, the filter is valid. If it does not build, give the v1.14.2 result: + // the exception of the clause for the prefix, else ParsingException. In a '.' culture, nothing changes. + private static Expression BuildClauseLikeV1142(string? cultureNumberPrefix, string right, Func buildClause) + { + if (cultureNumberPrefix == null) + { + return buildClause(right); + } + + try + { + return buildClause(right); + } + catch (Exception exception) + { + if (cultureNumberPrefix.Length > 0) + { + buildClause(cultureNumberPrefix); + } + + throw new ParsingException(exception); + } + } + private static readonly Parser GuidFormatParser = Parse.Regex(@"[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}").Text(); private static readonly Parser RawStringLiteralParser = @@ -268,32 +313,35 @@ from closingQuotes in Parse.Char('"').Repeat(count).Text() // Carries whether the right-hand value was written as a quoted string literal (e.g. "id"). // This is needed to disambiguate a literal from a bare property reference (property-to-property // comparison) once the surrounding quotes have been stripped, since both are otherwise identical strings. - private readonly record struct RightSideValue(string Value, bool IsQuotedLiteral); + // CultureNumberPrefix is set when the value holds a number that only the '.' decimal point reads (see BuildClauseLikeV1142). + private readonly record struct RightSideValue(string Value, bool IsQuotedLiteral, string? CultureNumberPrefix = null); - private static readonly Parser> SquareBracketValuesParser = + private static readonly Parser> SquareBracketValuesParser = Parse.String("null").Text() .Or(GuidFormatParser) .Or(DateTimeFormatParser) .Or(TimeFormatParser) - .Or(ListNumberParser) - .Or(RawStringLiteralParser.Or(DoubleQuoteParser)) - .Or(Identifier) + .Select(v => (v, false)) + .Or(ListNumberParser.Select(v => (v, CultureNumberPrefix(v) != null))) + .Or(RawStringLiteralParser.Or(DoubleQuoteParser).Or(Identifier).Select(v => (v, false))) .DelimitedBy(Parse.Char(',').Token()); - private static readonly Parser SquareBracketParser = + // v1.14.2 failed in the grammar at a '.' list number, before it built the clause, so no part of the list is read. + private static readonly Parser SquareBracketParser = from openingBracket in Parse.Char('[') from content in SquareBracketValuesParser from closingBracket in Parse.Char(']') - select "[" + string.Join(",", content) + "]"; + select new RightSideValue("[" + string.Join(",", content.Select(x => x.Value)) + "]", false, + content.Any(x => x.IsDotOnlyNumber) ? "" : null); private static readonly Parser RightSideValueChoiceParser = Parse.String("null").Text().Select(v => new RightSideValue(v, false)) .Or(GuidFormatParser.Select(v => new RightSideValue(v, false))) .XOr(DateTimeFormatParser.Select(v => new RightSideValue(v, false))) .XOr(TimeFormatParser.Select(v => new RightSideValue(v, false))) - .XOr(NumberParser.Select(v => new RightSideValue(v, false))) + .XOr(NumberParser.Select(v => new RightSideValue(v, false, CultureNumberPrefix(v)))) .XOr((RawStringLiteralParser.Or(DoubleQuoteParser)).Select(v => new RightSideValue(v, true))) - .XOr(SquareBracketParser.Select(v => new RightSideValue(v, false))) + .XOr(SquareBracketParser) .XOr(Identifier.Select(v => new RightSideValue(v, false))); // Keep this last to try property paths only if nothing else matches private static readonly Parser RightSideValueParser = @@ -301,7 +349,9 @@ from atSign in Parse.Char('@').Optional() from leadingSpaces in Parse.WhiteSpace.Many() from value in RightSideValueChoiceParser from trailingSpaces in Parse.WhiteSpace.Many() - select atSign.IsDefined ? value with { Value = "@" + value.Value } : value; + select atSign.IsDefined + ? value with { Value = "@" + value.Value, CultureNumberPrefix = value.CultureNumberPrefix is { Length: > 0 } prefix ? "@" + prefix : value.CultureNumberPrefix } + : value; // Arithmetic expression parsers private static readonly Parser ArithmeticOperatorParser = @@ -770,9 +820,10 @@ private static Parser ComparisonExprParser(ParameterExpression pa var regularComparison = CreateLeftExprParser(parameter.Type, config) .SelectMany(reference => comparisonOperatorParser, (reference, op) => new { reference, op }) - .SelectMany(temp => rightSideValueParser, (temp, rightValue) => new { temp.reference, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral }) - .Select(temp => + .SelectMany(temp => rightSideValueParser, (temp, rightValue) => new { temp.reference, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral, cultureNumberPrefix = rightValue.CultureNumberPrefix }) + .Select(clause => BuildClauseLikeV1142(clause.cultureNumberPrefix, clause.right, right => { + var temp = clause with { right = right }; if (temp.reference.Kind == PropertyReferenceKind.CustomOperation) { return CreateCustomOperationExpression(parameter, temp.reference.Mapping!, temp.op, temp.right); @@ -928,7 +979,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa return temp.op.GetExpression(leftExprForComparison, rightExpr, config?.DbContextType, ResolveCaseMode(propertyPath, config)); - }); + })); return propertyListComparison.Or(arithmeticComparison).Or(regularComparison); } @@ -1136,9 +1187,10 @@ private static Parser PropertyListComparisonExprParser( .SelectMany(properties => comparisonOperatorParser, (properties, op) => new { properties, op }) .SelectMany(temp => rightSideValueParser, - (temp, rightValue) => new { temp.properties, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral }) - .Select(temp => + (temp, rightValue) => new { temp.properties, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral, cultureNumberPrefix = rightValue.CultureNumberPrefix }) + .Select(clause => BuildClauseLikeV1142(clause.cultureNumberPrefix, clause.right, right => { + var temp = clause with { right = right }; if (!temp.properties.Any()) { throw new InvalidOperationException("Property list cannot be empty"); @@ -1196,7 +1248,7 @@ private static Parser PropertyListComparisonExprParser( // If all properties were filtered out, the clause is ignored. v1.14.2 used true here, not true == true. return result ?? (RemovesIgnoredClauses(config) ? RemovedClauseExpression.Instance : Expression.Constant(true)); - }); + })); } private static Type? GetInnerGenericType(Type type)