fix: restore v1.14.2 behavior for every breaking change on main - #134
Merged
Merged
Conversation
v1.14.2 parsed a filter of any length and any nesting depth. The default limits of 5000 characters and 32 levels rejected filters that v1.14.2 accepted. Set both defaults to int.MaxValue, so the limits are an opt-in through MaxInputLength and MaxNestingDepth. A later major version can turn them on by default again.
v1.14.2 applied HasMaxDepth to every path that starts with the property name, so HasMaxDepth on Address also applied to AddressBackup.State. Match by prefix again, like v1.14.2. A later major version can apply the depth only to the property and the paths below it.
v1.14.2 read one identifier on the right side of a comparison, so Title == Author.Name and Title == foo.bar threw ParsingException. Read one identifier again, like v1.14.2. A later minor version can accept a nested property path on the right side.
v1.14.2 built a TimeOnly value with the constructor that takes five ints. On net6.0 this constructor does not exist, so a TimeOnly filter threw ArgumentNullException. Main fell back to a constant. Remove the fallback, so a v1.14.2 consumer sees the same exception. The opt-in parameter path keeps its behavior. A later version can build TimeOnly values on net6.0.
…metic again v1.14.2 threw ArgumentException for an unknown property in arithmetic, also with AllowUnknownProperties. Main threw UnknownFilterPropertyException, or removed the clause when unknown properties were allowed. Throw ArgumentException again, like v1.14.2.
v1.14.2 checked PreventFilter only for a left-side member, looked up by its name in the exact case after the query-name rewrite. It did not check arithmetic, the right side, another case in a property list, derived properties, or custom operations. PreventSort was looked up by the typed path in the exact case. The parser now does the same checks as v1.14.2 again, so that a v1.14.2 consumer sees no difference. This reopens the six bypasses I1 to I6 of the breaking-change audit. A later major release closes them again.
…ery name again In v1.14.2, a property with PreventFilter and PreventSort threw InvalidOperationException when the filter used its query name before an operator. The alias rewrite pass did this check before the parser ran, so the exception was not wrapped in ParsingException. ParseFilter runs the same logical alias, comparison alias, and query name passes on a copy of the input again. The result of the passes is not used, because the grammar resolves query names. Only the check has an effect.
In v1.14.2, the HasConversion lookups searched by query name after the query name was already replaced with the property path. A property with both HasConversion and HasQueryName did not use its conversion. The lookups search by query name again. A null literal on a converted property builds a value from the text null again. A converted Nullable<T> struct, Guid string operators, and lower-case property lists do not use the conversion again.
After a collection, only properties match again. The first segment must match in the exact case, and a later segment matches in any case. A segment that does not match throws NullReferenceException, like v1.14.2.
ParseFilter replaces each query name in front of a comparison operator with its property path again, like v1.14.2. The pass also changes a query name inside a quoted value, as v1.14.2 did. The pass also matches a query name in front of a comparison alias. The parser still reads the aliases, so the aliases inside quoted values stay as they are.
The property resolver no longer maps a query name to its property path, and the grammar reads only identifier paths again. A query name works through the rewrite before the parse, like v1.14.2. A query name in a property list throws UnknownFilterPropertyException again, and a query name in arithmetic throws ArgumentException again. A query name with a hyphen, a leading underscore, or a space still works in front of an operator.
Arithmetic builds its property paths from the filter text again, like v1.14.2. A property path inside arithmetic does not go through the property resolver, so MaxPropertyDepth does not apply to it. Security consequence: arithmetic can go deeper than MaxPropertyDepth again, as in v1.14.2.
v1.14.2 replaced each operator alias that stands between whitespace with a regex before the parse. This also changed alias text inside a quoted value, so Title eq "salt and pepper" compares with "salt && pepper". Main read the aliases only in the grammar, so the value stayed unchanged and the result was different. The rewrite runs again, in the v1.14.2 order: logical aliases, comparison aliases, then query names. The query-name rewrite goes back to the v1.14.2 pattern, because the aliases are already replaced when it runs. The grammar still reads an alias that the rewrite did not replace, for example (Age)eq 3, which v1.14.2 rejected. The grammar tries the canonical operator first, like v1.14.2. Restores 39312a1.
v1.14.2 split the value of the in and not-in operators on every comma, also on a comma inside a quoted item. Serving ^^ ["Warm, with syrup"] reads the two items Warm and with syrup. Main kept the quoted item as one item, so the same filter gave different rows. Restores c90861c.
v1.14.2 kept the offset of a DateTimeOffset filter value, so 2022-07-01T00:00:03+01:00 gave new DateTimeOffset(..., 01:00:00). Main converted every value to UTC, also when ParameterizeFilterValues is off, so the expression text and the value changed. The value goes to UTC only when ParameterizeFilterValues is on, because Npgsql accepts a DateTimeOffset parameter only with offset 0. Without parameters the value keeps its offset, like v1.14.2. Restores ea5cc66 for the default path.
can_filter_enumerable filtered two fake recipes by the title of the first one. AutoBogus fills the title with one random word, so both recipes sometimes had the same title and the test found two rows.
…d time fraction again Restores the break parts of 5c84ef6. A date time value with the zone before the fraction, for example 2024-01-15T08:00:00Z.5, parses again. A quoted time keeps the v1.14.2 fraction rule: milliseconds need 3 digits and microseconds need 6. The parts that v1.14.2 rejected stay: a fraction before the zone, 7 fraction digits, an unquoted time fraction, and a time list.
…ators again Restores aff9638. The case-sensitive @=, _=, _-= operators and their negations give the v1.14.2 expression text again. In memory, a null property throws NullReferenceException, like v1.14.2. On Postgres, the results do not change. A string operator on a collection property throws ArgumentException again, like v1.14.2, and not ParsingException.
Restores c948532. The public ComparisonOperator factories accept usesAll but do not give it to the constructor, like v1.14.2. An operator from a factory matches any item of a collection.
…n again Restores the v1.14.2 list read that 5db8c4b (PR 110) removed. InOperator(true) and NotInOperator(true) threw NullReferenceException when a caller passed the list as a ConstantExpression. They read the constant list again, like v1.14.2.
…Symbol Completes the C restore. d507c87 added FromSymbol back with an Obsolete attribute. A v1.14.2 consumer that builds with warnings as errors got CS0618. FromSymbol has no attribute again, like v1.14.2.
Restores the message that de350f7 (PR 109) changed. The has operator on a property that is not a collection throws 'DoesNotHaveType is only supported for collections' again, like v1.14.2. The exception type does not change.
…e with a hyphen or a space again The grammar reads the identifier path first, like v1.14.2. If the path is not a property and unknown properties are not allowed, the grammar tries the query names of derived properties and custom operations, longest first. v1.14.2 threw for these inputs, so no filter that v1.14.2 accepted gives a different result.
The Property selector returns object, so a selector for a nullable property gave warning CS8603. The 40 selectors now use the null-forgiving operator, and the build has no warnings.
This was referenced Oct 1, 2026
This was referenced Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Main must have no breaking change against v1.14.2. This PR adds restore commits on top of
c345c53. It does not rewrite history.The rule for a break: an input that v1.14.2 accepted gives a different result (expression text, SQL, value, or exception type, or a throw compared with no throw on in-memory evaluation). A change is not a break if it only makes an input work that v1.14.2 rejected with an exception.
Each restored item has one commit. A later per-change PR can re-apply each item by itself: revert the restore commit and bring back the tests in the test ledger below.
The captain accepted that main gets the old security gaps back (B, I, M, P). The next major release closes them again.
The merge needs the captain's word.
Restored items from the audit
B: parse limits are off by default (
42866b9)de350f7(fix(parser): limit filter nesting depth and length, plus small fixes and doc cleanups #109).DefaultMaxInputLengthandDefaultMaxNestingDepthareint.MaxValuenow.MaxInputLengthandMaxNestingDepthstay as an opt-in.catchcannot stop it. At depth 1000, one parse takes more than 20 seconds of CPU. Every app that sends user input to QueryKit and does not set the limits is exposed again.I: PreventFilter and PreventSort bypasses are open again (
2f33f60)988fdff,2781423,630d080,1f40773,b350bf8(fix(filter): resolve every property reference with one resolver #113).PreventFilteronly for a left-side member, by its name in the exact case after the query-name rewrite. It did not check arithmetic, the right side, another case in a property list, derived properties, or custom operations.PreventSortwas checked by the typed path in the exact case.(Salary + 0) > 50000, then> 75000. A sort bypass shows the order of the hidden values.M: a per-property max depth matches by path prefix (
a947ebb)73523a5(fix(filter): resolve every property reference with one resolver #113).HasMaxDepthonAddressalso applies toAddressBackup.State, like v1.14.2.MaxPropertyDepthagain if its name starts with a property that has a looserHasMaxDepth.P: MaxPropertyDepth does not apply to arithmetic (
8a09c25)988fdff(fix(filter): resolve every property reference with one resolver #113).MaxPropertyDepthagain. For example,(Recipe.Rating + 0) > 1passes a limit of 0.F: HasConversion lookup by query name (
76d3a7d)b817eb3(fix: honor HasConversion when a property has a HasQueryName alias #107).HasConversionandHasQueryNamedoes not use its conversion, like v1.14.2.G: child collection member in the exact case (
0f1d864)b562d00(fix(filter): resolve every property reference with one resolver #113).NullReferenceException, like v1.14.2.J5: query names are replaced before the parse (
f0f48be)3c85253(fix(filter): resolve every property reference with one resolver #113).J-bis: query names only in front of an operator (
5e7d918)3c85253(fix(filter): resolve every property reference with one resolver #113).UnknownFilterPropertyException. A query name in arithmetic throwsArgumentException.K: fully prevented query name throws (
c0e9914)d54de85(fix(filter): resolve every property reference with one resolver #113).PreventFilterandPreventSortthrowsInvalidOperationExceptionwhen a filter uses its query name before an operator.L: one word on the right side (
2b1c252)71b4c40(fix(filter): resolve every property reference with one resolver #113).Title == Author.NamethrowsParsingException, like v1.14.2.O: unknown property in arithmetic (
a3a4d54)af5c3f7(fix(filter): resolve every property reference with one resolver #113).ArgumentException, also withAllowUnknownProperties.T: TimeOnly literals on net6.0 (
7653edb)5db8c4b(perf: parameterize filter values and cache parsers and regexes #110).ArgumentNullExceptionagain, like v1.14.2. The opt-in parameter path does not change.Restored items found after the audit
The audit compared v1.14.2 with main at
d54de85. These commits came later, or the main-verify scout found them. Each one restores only the break part. The throw-to-works part stays and is listed below.f5209e539312a1Title eq "salt and pepper"compares with"salt && pepper"again.01e8b50c90861c7198ac2ea5cc66ParameterizeFilterValuesis on.b49ad2b5c84ef6(break parts)2024-01-15T08:00:00Z.5) parses again. A quoted time keeps the v1.14.2 fraction rule: milliseconds need 3 digits, microseconds need 6.0abd1f8aff9638@=,_=,_-=and their negations. In memory, a null property throwsNullReferenceException. Postgres results do not change.0761e53c948532ComparisonOperatorfactories ignoreusesAll.e6d55b35db8c4b(#110) removedInOperator(true)andNotInOperator(true)read a list that a caller passes as aConstantExpression.19125d1Obsoletemark ofd507c87ArithmeticOperator.FromSymbolhas noObsoleteattribute. A consumer with warnings as errors does not get CS0618.22798f0de350f7(#109)hason a property that is not a collection throwsDoesNotHaveType is only supported for collectionsagain.Design choice in
b49ad2b: a quoted time uses the v1.14.2 fraction rule. An unquoted time fraction keeps the full fraction, because v1.14.2 rejected an unquoted fraction.QN1: a v1.14.2 behavior that main had changed
Author.name == "Lee"with query namenameonTitle: v1.14.2 rewrites it toAuthor.Titleand throwsUnknownFilterPropertyException: 'Title'. Main gavex => (x.Author.Name == "Lee"). The branch throws like v1.14.2 again, through the rewrite before the parse (f0f48be,f5209e5).Kept: v1.14.2 threw, now works
e032af7a.decimal point in every cultureRating > 4.4:ParsingException, nowx => (x.Rating > 4,4)52556cdint and decimal equalityAge == Rating:ParsingException, nowx => (Convert(x.Age, Decimal) == x.Rating)67bb158sort direction after more than one spaceAge desc:ArgumentException, now sorts4,339312a1an alias that the rewrite does not replace(Age)eq 3with aliaseq:ParsingException, nowx => (x.Age == 3)5c84ef6fraction before the zoneAt == 2024-01-15T08:00:00.500Z:ParsingException, now works5c84ef6fraction before an offsetAt == 2024-01-15T10:00:00.5+02:00:ParsingException, now works5c84ef67 fraction digitsAt > 2024-01-15T07:00:00.1234567:ParsingException, now works5c84ef6unquoted time fractionT == 08:30:00.5:ParsingException, nownew TimeOnly(8, 30, 0, 500, 0)5c84ef6time listT ^^ [08:30:00.5]:ParsingException, now worksbc70b3a)full-name == "Ann Lee":UnknownFilterPropertyException: 'full', nowx => (((x.FirstName + " ") + x.Title) == "Ann Lee")bc70b3a)adult-ish == true:UnknownFilterPropertyException: 'adult', now calls the custom operationbc70b3areads the identifier path first, like v1.14.2. It tries the derived-property and custom-operation query names only when the path is not a property andAllowUnknownPropertiesis off, or when the path does not parse. In both cases v1.14.2 threw. The probe casesFirstNamewith query namefirst,Titlewith query nameTitle,Age!= 20with query nameage!, andfoo!= 20withAllowUnknownPropertiesgive the same result as v1.14.2.Kept known differences
QueryKit.InListValues`1in place ofList`1. The rows do not change. The 3 v1.14.2 unit tests that assert the type name fail:simple_in_operator_for_nullable_int,simple_in_operator_for_guid,can_have_custom_prop_name_with_in_operator.ParsingExceptionmessage addsFailed at Line N, Column M.The captain approved this as not breaking. Example:Title eq"Lee"with aliaseqthrowsParsingExceptionon both. Only the message text is different.IQueryKitFilterBehavior,IQueryKitParseLimits,IgnoredClauseBehavior,ParameterizeFilterValues,MaxInputLength,MaxNestingDepth,QueryKitInputLengthExceededException,QueryKitNestingDepthExceededException. The defaults give the v1.14.2 behavior.(full-name, Title) == "Lee"with a derived query namefull-name. v1.14.2 throwsUnknownFilterPropertyException. The branch throwsParsingException. Both reject the input.Other commits
2fa061atest:can_filter_enumerablegave two fake recipes the same random title sometimes, so the test found two rows. The filtered recipe gets a unique title now. This was a flaky test.01b06c1test: 40 property selectors in the test configurations get the null-forgiving operator. The build has no CS8603 warnings now.Changed or removed tests
A per-change PR can bring back each test on the left side.
Test ledger
B 42866b9
M a947ebb
L 2b1c252
T 7653edb
O a3a4d54
I
K
F
G
J5
J-bis
P
39312a1 (alias pre-pass)
c90861c (comma in a quoted list value)
ea5cc66 (DateTimeOffset to UTC, only with parameters on)
5c84ef6 (fraction: break parts only)
Time == "08:30:00.5"andTime == "08:30:00.50"moved to new pin quoted_time_with_fewer_than_three_fraction_digits_drops_the_fraction (v1.14.2 drops a quoted fraction with fewer than 3 digits). New kept cases:Time == "08:30:00.500",2024-01-15T08:00:00Z.5,2024-01-15T10:00:00+02:00.500(v1.14.2 zone-before-fraction format).500Z, fraction before an offset, 7 fraction digits, unquoted time fraction, time listaff9638 (null guards on case-sensitive string operators, full revert)
!= nullguard on @=, _=, _-=, !@=)c948532 (factories pass usesAll, full revert)
NEW-2 (constant list in case-insensitive In and NotIn, from 5db8c4b PR 110)
C residual (Obsolete on FromSymbol, from d507c87)
D (HasType message, from de350f7 PR 109)
QN3/QN4 (derived property and custom operation query names that are not identifiers, kept throw-to-works)
Proof
Run on this branch at
01b06c1. The v1.14.2 files come from tag v1.14.2. The v1.14.2 suites run on net9.0 withDOTNET_ROLL_FORWARD=Major.IQueryKitConfigurationdoes not change.FromSymbolhas noObsoleteattribute.IQueryKitConfigurationmember, callsFromSymbol, nests 40 levels, and sends 6000 characters. Compiled on v1.14.2 and run on v1.14.2, run with the branch dll swapped in, and compiled on the branch: all three print the same output.