From d5acc04b4694c3f1c41881bf69b0c38c283a30ab Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sat, 10 Oct 2026 00:07:05 +0300 Subject: [PATCH] fix(filter)!: resolve only public members A filter segment now matches only a public property or a public field, ignoring case. An internal, protected, or private member is an unknown property. A query name on a non-public member is also an unknown property. An indexer stays an unknown property. Examples: InternalScore > 30 (internal property) before: x.InternalScore > 30 after: UnknownFilterPropertyException secretRank == 7 (private field) before: x.secretRank == 7 after: UnknownFilterPropertyException score > 30, query name on InternalScore before: x.InternalScore > 30 after: UnknownFilterPropertyException secretRank > 100 with AllowUnknownProperties before: x.secretRank > 100 after: the clause does not filter Title == "x" (public property) before and after: x.Title == "x" BREAKING CHANGE: a filter on an internal, protected, or private member throws UnknownFilterPropertyException. Make the member public to filter on it. --- .../Tests/PropertyResolverTests.cs | 32 +++++--------- QueryKit.UnitTests/PropertyResolverTests.cs | 44 +++++++------------ QueryKit/PropertyResolver.cs | 16 +++---- 3 files changed, 34 insertions(+), 58 deletions(-) diff --git a/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs b/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs index f3b9043..8a0d844 100644 --- a/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs +++ b/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs @@ -2,6 +2,7 @@ namespace QueryKit.IntegrationTests.Tests; using Bogus; using Configuration; +using Exceptions; using FluentAssertions; using Microsoft.EntityFrameworkCore; using SharedTestingHelper.Fakes; @@ -589,42 +590,34 @@ public async Task query_name_that_is_not_a_plain_identifier_filters_by_its_prope } [Fact] - public async Task non_public_mapped_property_filters_in_the_database() + public void non_public_mapped_property_is_an_unknown_property() { // Arrange var testingServiceScope = new TestingServiceScope(); - var nickname = new Faker().Lorem.Sentence(); - var fakePerson = new FakeTestingPersonBuilder().Build(); - fakePerson.Nickname = nickname; - var otherPerson = new FakeTestingPersonBuilder().Build(); - otherPerson.Nickname = new Faker().Lorem.Sentence(); - await testingServiceScope.InsertAsync(fakePerson, otherPerson); - - var input = $"""nickname == "{nickname}" """; + var input = $"""nickname == "{new Faker().Lorem.Sentence()}" """; // Act - var queryable = testingServiceScope.DbContext().People.ApplyQueryKitFilter(input); - var people = await queryable.ToListAsync(); + var act = () => testingServiceScope.DbContext().People.ApplyQueryKitFilter(input); // Assert - queryable.ToQueryString().Should().Contain("""p.nickname = """); - people.Should().ContainSingle(); - people[0].Id.Should().Be(fakePerson.Id); + act.Should().ThrowExactly() + .WithMessage("The filter property 'nickname' was not recognized."); } [Fact] - public async Task non_public_mapped_property_filters_when_unknown_properties_are_allowed() + public async Task non_public_mapped_property_clause_does_not_filter_when_unknown_properties_are_allowed() { // Arrange var testingServiceScope = new TestingServiceScope(); + var title = new Faker().Lorem.Sentence(); var nickname = new Faker().Lorem.Sentence(); - var fakePerson = new FakeTestingPersonBuilder().Build(); + var fakePerson = new FakeTestingPersonBuilder().WithTitle(title).Build(); fakePerson.Nickname = nickname; - var otherPerson = new FakeTestingPersonBuilder().Build(); + var otherPerson = new FakeTestingPersonBuilder().WithTitle(title).Build(); otherPerson.Nickname = new Faker().Lorem.Sentence(); await testingServiceScope.InsertAsync(fakePerson, otherPerson); - var input = $"""Nickname == "{nickname}" """; + var input = $"""Nickname == "{nickname}" && Title == "{title}" """; var config = new QueryKitConfiguration(config => { config.AllowUnknownProperties = true; @@ -636,7 +629,6 @@ public async Task non_public_mapped_property_filters_when_unknown_properties_are .ToListAsync(); // Assert - people.Should().ContainSingle(); - people[0].Id.Should().Be(fakePerson.Id); + people.Should().HaveCount(2); } } diff --git a/QueryKit.UnitTests/PropertyResolverTests.cs b/QueryKit.UnitTests/PropertyResolverTests.cs index 3bd3ad5..711204b 100644 --- a/QueryKit.UnitTests/PropertyResolverTests.cs +++ b/QueryKit.UnitTests/PropertyResolverTests.cs @@ -1005,35 +1005,22 @@ public void unknown_property_in_arithmetic_is_not_recognized() } [Theory] - [InlineData("InternalScore > 30", "x => (x.InternalScore > 30)")] - [InlineData("internalscore > 30", "x => (x.InternalScore > 30)")] - [InlineData("""ProtectedNote == "a" """, """x => (x.ProtectedNote == "a")""")] - [InlineData("secretRank == 7", "x => (x.secretRank == 7)")] - [InlineData("""Owner.InternalAlias == "Ann" """, """x => (x.Owner.InternalAlias == "Ann")""")] - [InlineData("(InternalScore, Rating) > 3", "x => ((x.InternalScore > 3) OrElse (x.Rating > 3))")] - public void non_public_member_filters_like_a_public_member(string input, string expected) + [InlineData("InternalScore > 30", "InternalScore")] + [InlineData("internalscore > 30", "internalscore")] + [InlineData("""ProtectedNote == "a" """, "ProtectedNote")] + [InlineData("secretRank == 7", "secretRank")] + [InlineData("""Owner.InternalAlias == "Ann" """, "InternalAlias")] + [InlineData("(InternalScore, Rating) > 3", "InternalScore")] + public void non_public_member_is_an_unknown_property(string input, string unknownProperty) { - var filterExpression = FilterParser.ParseFilter(input); - - filterExpression.ToDisplayString().Should().Be(expected); - } - - [Fact] - public void non_public_member_filters_the_rows() - { - var models = new List - { - new(internalScore: 50, rank: 7), - new(internalScore: 20, rank: 3), - }; - - var result = models.ApplyQueryKitFilter("InternalScore > 30 && secretRank == 7").ToList(); + var act = () => FilterParser.ParseFilter(input); - result.Should().ContainSingle().Which.Should().BeSameAs(models[0]); + act.Should().ThrowExactly() + .WithMessage($"The filter property '{unknownProperty}' was not recognized."); } [Fact] - public void non_public_member_filters_when_unknown_properties_are_allowed() + public void non_public_member_clause_is_removed_when_unknown_properties_are_allowed() { var input = """secretRank > 100 || Rating == 1"""; var config = new QueryKitConfiguration(config => @@ -1043,11 +1030,11 @@ public void non_public_member_filters_when_unknown_properties_are_allowed() var filterExpression = FilterParser.ParseFilter(input, config); - filterExpression.ToDisplayString().Should().Be("x => ((x.secretRank > 100) OrElse (x.Rating == 1))"); + filterExpression.ToDisplayString().Should().Be("x => (x.Rating == 1)"); } [Fact] - public void query_name_on_a_non_public_member_filters_by_that_member() + public void query_name_on_a_non_public_member_throws_unknown_property() { var input = """score > 30"""; var config = new QueryKitConfiguration(config => @@ -1055,9 +1042,10 @@ public void query_name_on_a_non_public_member_filters_by_that_member() config.Property(x => x.InternalScore).HasQueryName("score"); }); - var filterExpression = FilterParser.ParseFilter(input, config); + var act = () => FilterParser.ParseFilter(input, config); - filterExpression.ToDisplayString().Should().Be("x => (x.InternalScore > 30)"); + act.Should().ThrowExactly() + .WithMessage("The filter property 'InternalScore' was not recognized."); } [Fact] diff --git a/QueryKit/PropertyResolver.cs b/QueryKit/PropertyResolver.cs index 8d60f98..a72fddd 100644 --- a/QueryKit/PropertyResolver.cs +++ b/QueryKit/PropertyResolver.cs @@ -91,8 +91,7 @@ private static PropertyReference Resolve(Type rootType, string reference, string return PropertyReference.NotMember(PropertyReferenceKind.Unknown, reference, null, unknownSegment!); } - // Matches each segment to a member, ignoring case, in the order of Expression.PropertyOrField like v1.14.2: - // a public property, a public field, a non-public property, and then a non-public field. An indexer does not match. + // Matches each segment to a public member, ignoring case: a public property, and then a public field. An indexer does not match. // A segment after a collection resolves on the element type, with the same rules. private static string? ResolveMemberPath(Type rootType, string path, out string? unknownSegment) { @@ -106,15 +105,13 @@ private static PropertyReference Resolve(Type rootType, string reference, string currentType = currentType.GetGenericArguments()[0]; } - var member = (MemberInfo?)currentType.GetProperty(segment, PublicMemberFlags) - ?? (MemberInfo?)currentType.GetField(segment, PublicMemberFlags) - ?? (MemberInfo?)currentType.GetProperty(segment, NonPublicMemberFlags) - ?? currentType.GetField(segment, NonPublicMemberFlags); + var member = (MemberInfo?)currentType.GetProperty(segment, MemberFlags) + ?? currentType.GetField(segment, MemberFlags); if (member == null || member is PropertyInfo indexer && indexer.GetIndexParameters().Length > 0) { - // v1.14.2 named an unknown member by the name of the public property with that name, if there was one. - unknownSegment = currentType.GetProperty(segment, PublicMemberFlags)?.Name ?? segment; + // An indexer is named by its property name, like v1.14.2. + unknownSegment = member?.Name ?? segment; return null; } @@ -126,8 +123,7 @@ private static PropertyReference Resolve(Type rootType, string reference, string return string.Join(".", memberNames); } - private const BindingFlags PublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance; - private const BindingFlags NonPublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.NonPublic | BindingFlags.Instance; + private const BindingFlags MemberFlags = BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance; private static bool IsCollection(Type type) => type != typeof(string) && type.IsGenericType &&