Skip to content

fix(config)!: apply a property max depth only to the property and its paths - #137

Open
pdevito3 wants to merge 1 commit into
mainfrom
fm/qk-breaking-max-depth-prefix
Open

pdevito3 wants to merge 1 commit into
mainfrom
fm/qk-breaking-max-depth-prefix

Conversation

@pdevito3

@pdevito3 pdevito3 commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

For later consideration in a major version. Do not merge now. #134 restored the v1.14.2 prefix match to keep v1.x compatible (restore commit a947ebb). This PR re-applies 73523a5 (#113). The captain decides on this PR separately.

Summary

-            if (mapping.MaxDepth.HasValue &&
-                propertyPath.StartsWith(mapping.Name ?? "", StringComparison.OrdinalIgnoreCase))
+            if (mapping.MaxDepth.HasValue && !string.IsNullOrEmpty(mapping.Name) &&
+                (propertyPath.Equals(mapping.Name, StringComparison.OrdinalIgnoreCase) ||
+                 propertyPath.StartsWith(mapping.Name + ".", StringComparison.OrdinalIgnoreCase)))

HasMaxDepth applies only to the property and the paths below it. The lookup ignores the letter case, as before.

v1.14.2 behavior (and main)

HasMaxDepth applies to every path that starts with the property name as text. HasMaxDepth on Address also applies to AddressBackup.State and to AddressLines.

New behavior

HasMaxDepth on Address applies to Address and to Address.<anything>. A different property such as AddressBackup uses the global MaxPropertyDepth.

Example

var config = new QueryKitConfiguration(settings =>
{
    settings.MaxPropertyDepth = 0;
    settings.Property<Owner>(x => x.Address).HasMaxDepth(1);
});
FilterParser.ParseFilter<Owner>("""AddressBackup.State == "x" """, config);
SortParser.ParseSort<Owner>("AddressBackup.State", config);
  • v1.14.2 and main: both calls work. AddressBackup.State gets the limit of 1 from Address.
  • This PR: both calls throw QueryKitPropertyDepthExceededException ("depth of 1 ... maximum allowed depth of 0").

Security risk on v1.14.2

MaxPropertyDepth limits how deep a caller can go into the object graph. With the prefix match, a path skips the global limit if its name starts with the name of a property that has a looser HasMaxDepth. A caller can then reach navigation properties that the global limit was set to block. This can expose related data and make larger joins than the app permits.

Justification

HasMaxDepth is configured on one property. It must not change the limit of a different property that has a similar name. The new match uses the property path, not the text prefix.

Migration

If a property needs a looser limit, add HasMaxDepth on that property.

README

The Max Property Depth section now tells that the override applies only to the property and the paths below it, with the AddressBackup.State example.

Tests

These tests come back from main before #134, in PropertyDepthTests:

  • filter_per_property_max_depth_applies_to_a_property_that_starts_with_its_name -> filter_per_property_max_depth_does_not_apply_to_a_property_that_starts_with_its_name
  • sort_per_property_max_depth_applies_to_a_property_that_starts_with_its_name -> sort_per_property_max_depth_does_not_apply_to_a_property_that_starts_with_its_name

dotnet test: 470 unit tests and 297 Postgres integration tests (Testcontainers) pass, 0 failures.

Rebase on main

This branch is rebased on current main (#169). The rebase had no conflicts. The breaking change did not change.

… paths

HasMaxDepth matched every path that starts with the property name, so HasMaxDepth on Address also applied to AddressBackup.State. A path could skip the global MaxPropertyDepth if its name started with a property that has a looser limit. Match the property name or the name followed by a dot.

BREAKING CHANGE: HasMaxDepth on a property no longer applies to another property whose name starts with the same text. That property uses the global MaxPropertyDepth.
@pdevito3
pdevito3 force-pushed the fm/qk-breaking-max-depth-prefix branch from 52b1ef0 to 23cde7e Compare October 1, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant