From de51645daeb9d39121c993160ce4a2e3943b00e6 Mon Sep 17 00:00:00 2001 From: Glen Date: Fri, 19 Jun 2026 12:52:15 +0200 Subject: [PATCH 1/2] Make cursor key extraction null-safe for nullable nested fields --- .../src/GreenDonut.Data/Cursors/CursorKey.cs | 145 +++++++++++- .../Cursors/CursorFormatterTests.cs | 208 ++++++++++++++++++ 2 files changed, 352 insertions(+), 1 deletion(-) diff --git a/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs b/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs index 4e0552c48fe..116ef99e256 100644 --- a/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs +++ b/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs @@ -68,7 +68,150 @@ public bool TryFormat(object entity, Span buffer, out int written) private object? GetValue(object entity) { - _compiled ??= Expression.Compile(); + _compiled ??= NullSafeKeySelector.Compile(Expression); return _compiled.DynamicInvoke(entity); } } + +/// +/// Compiles a key-selector expression into a delegate that tolerates null +/// intermediates along a nested member-access path (for example x.Meter.Name +/// when x.Meter is null). Each nullable intermediate is bound to a local and +/// null-checked once, so the result short-circuits to null instead of throwing. +/// This mirrors the C# null-conditional operator, including its single-evaluation +/// guarantee. The serializer layer formats a null key value as the null marker. +/// +file static class NullSafeKeySelector +{ + public static Delegate Compile(LambdaExpression expression) + { + var parameter = expression.Parameters[0]; + + // Drop the outer boxing-to-object conversion (if any); it is re-applied + // at the end so every branch yields object. All other conversions are + // kept so that a cast intermediate (for example (Derived)x.Member) keeps + // rebinding member access against the correct type. + var leaf = StripBoxing(expression.Body); + + // Walk the access spine, made up of member accesses and conversions, + // from the leaf down to its root, stopping at the parameter. + var spine = new List(); + var current = leaf; + while (true) + { + if (current is MemberExpression { Expression: { } inner }) + { + spine.Add(current); + current = inner; + } + else if ( + current is UnaryExpression + { + NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked, + Operand: { } operand + }) + { + spine.Add(current); + current = operand; + } + else + { + break; + } + } + + // Only rewrite a spine that roots in the parameter and has a nullable + // intermediate (any node that gets dereferenced, i.e. all but the leaf). + if (current != parameter || !HasNullableIntermediate(spine)) + { + return expression.Compile(); + } + + spine.Reverse(); + + var variables = new List(); + var statements = new List(); + var done = Expression.Label(typeof(object), "result"); + var nullResult = Expression.Constant(null, typeof(object)); + + Expression expr = parameter; + for (var i = 0; i < spine.Count; i++) + { + var isLeaf = i == spine.Count - 1; + + // Rebuild the node onto the running expression, preserving the + // original member, cast, checked-ness, and conversion operator. + expr = spine[i] is MemberExpression member + ? member.Update(expr) + : ((UnaryExpression)spine[i]).Update(expr); + + if (isLeaf) + { + statements.Add(Expression.Return(done, Expression.Convert(expr, typeof(object)))); + break; + } + + // Bind each intermediate member to a local so it is evaluated once, + // and guard the nullable ones so a null short-circuits instead of + // being dereferenced. Conversions are pure, so they are applied + // inline without a local. + if (spine[i] is MemberExpression) + { + var local = Expression.Variable(expr.Type, "v" + i); + variables.Add(local); + statements.Add(Expression.Assign(local, expr)); + + if (CanBeNull(expr.Type)) + { + statements.Add( + Expression.IfThen( + IsNull(local), + Expression.Return(done, nullResult))); + } + + expr = local; + } + } + + statements.Add(Expression.Label(done, nullResult)); + + var block = Expression.Block(typeof(object), variables, statements); + return Expression.Lambda(block, parameter).Compile(); + } + + // The intermediates are every node in the spine except the leaf (index 0, + // since the spine is collected leaf-first). + private static bool HasNullableIntermediate(List spine) + { + for (var i = 1; i < spine.Count; i++) + { + if (CanBeNull(spine[i].Type)) + { + return true; + } + } + + return false; + } + + // Tests an intermediate for null the same way the C# null-conditional + // operator does: a reference-identity check for reference types (never an + // overloaded operator ==) and a HasValue check for Nullable. + private static Expression IsNull(Expression operand) + => Nullable.GetUnderlyingType(operand.Type) is not null + ? Expression.Not(Expression.Property(operand, "HasValue")) + : Expression.ReferenceEqual(operand, Expression.Constant(null, operand.Type)); + + // Strips only the outer boxing-to-object conversion; other conversions carry + // meaning (casts, widening, user-defined operators) and must be preserved. + private static Expression StripBoxing(Expression expression) + => expression is UnaryExpression + { + NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked + } unary && unary.Type == typeof(object) + ? StripBoxing(unary.Operand) + : expression; + + private static bool CanBeNull(Type type) + => !type.IsValueType || Nullable.GetUnderlyingType(type) is not null; +} diff --git a/src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorFormatterTests.cs b/src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorFormatterTests.cs index 4366686b903..bc8f1c23c54 100644 --- a/src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorFormatterTests.cs +++ b/src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorFormatterTests.cs @@ -113,10 +113,218 @@ public void Format_And_Parse_Two_Keys_With_Colon() Assert.Equal("description:123", parsed.Values[1]); } + [Fact] + public void Format_Nested_Key_With_Null_Intermediate_Yields_Null_Value() + { + // arrange + var entity = new Parent { Child = null }; + Expression> selector = x => x.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}\\null", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_With_NonNull_Intermediate() + { + // arrange + var entity = new Parent { Child = new Child { Name = "test" } }; + Expression> selector = x => x.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}test", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_Reads_Intermediate_Once() + { + // arrange + var entity = new CountingParent { Child = new Child { Name = "test" } }; + Expression> selector = x => x.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}test", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + Assert.Equal(1, entity.ChildReads); + } + + [Fact] + public void Format_Deeply_Nested_Key_With_Null_Inner_Intermediate_Yields_Null_Value() + { + // arrange + var entity = new GrandParent { Parent = new Parent { Child = null } }; + Expression> selector = x => x.Parent!.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}\\null", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Deeply_Nested_Key_With_NonNull_Intermediates() + { + // arrange + var entity = new GrandParent { Parent = new Parent { Child = new Child { Name = "test" } } }; + Expression> selector = x => x.Parent!.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}test", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_Null_Check_Ignores_Overloaded_Equality() + { + // arrange + // The null-safety must rely on a reference null check, not a user-defined + // operator ==, so a null intermediate short-circuits no matter how the + // type defines equality. + var entity = new EqualityOverrideParent { Child = null }; + Expression> selector = x => x.Child!.Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}\\null", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_With_Null_Nullable_Value_Type_Intermediate_Yields_Null_Value() + { + // arrange + var entity = new NullableStructParent { Moment = null }; + Expression> selector = x => x.Moment!.Value.Hour; + var serializer = new IntCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}\\null", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_With_Cast_Intermediate_Extracts_Value() + { + // arrange + // A cast on the access path must be preserved so the leaf member binds + // against the cast type, not the declared (base) type. + var entity = new CastParent { Child = new SpecialChild { Name = "test" } }; + Expression> selector = x => ((SpecialChild)x.Child!).Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}test", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + + [Fact] + public void Format_Nested_Key_With_Cast_Intermediate_And_Null_Yields_Null_Value() + { + // arrange + var entity = new CastParent { Child = null }; + Expression> selector = x => ((SpecialChild)x.Child!).Name; + var serializer = new StringCursorKeySerializer(); + + // act + var result = CursorFormatter.Format(entity, [new CursorKey(selector, serializer)]); + + // assert + Assert.Equal("{}\\null", Encoding.UTF8.GetString(Convert.FromBase64String(result))); + } + public class MyClass { public string Name { get; set; } = null!; public string? Description { get; set; } } + + public class GrandParent + { + public Parent? Parent { get; set; } + } + + public class Parent + { + public Child? Child { get; set; } + } + + public class CountingParent + { + public int ChildReads { get; private set; } + + public Child? Child + { + get + { + ChildReads++; + return field; + } + set; + } + } + + public class Child + { + public string? Name { get; set; } + } + + public class EqualityOverrideParent + { + public EqualityOverrideChild? Child { get; set; } + } + + public class EqualityOverrideChild + { + public string? Name { get; set; } + + // Reports inequality even for two null references, so a guard relying on + // this operator would fail to detect a null intermediate. + public static bool operator ==(EqualityOverrideChild? left, EqualityOverrideChild? right) => false; + + public static bool operator !=(EqualityOverrideChild? left, EqualityOverrideChild? right) => true; + + public override bool Equals(object? obj) => false; + + public override int GetHashCode() => 0; + } + + public class NullableStructParent + { + public DateTime? Moment { get; set; } + } + + public class CastParent + { + public ChildBase? Child { get; set; } + } + + public class ChildBase; + + public class SpecialChild : ChildBase + { + public string? Name { get; set; } + } } From 609c3a51c68cd9fa4aa515fe5f28b5a458f5e346 Mon Sep 17 00:00:00 2001 From: Glen Date: Fri, 19 Jun 2026 13:09:46 +0200 Subject: [PATCH 2/2] Guard cursor key compilation against unexpected selector shapes --- src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs b/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs index 116ef99e256..62e7c7eae9b 100644 --- a/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs +++ b/src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs @@ -85,6 +85,13 @@ file static class NullSafeKeySelector { public static Delegate Compile(LambdaExpression expression) { + // Cursor-key selectors take a single entity parameter; fall back for any + // other shape rather than assuming the parameter count. + if (expression.Parameters.Count != 1) + { + return expression.Compile(); + } + var parameter = expression.Parameters[0]; // Drop the outer boxing-to-object conversion (if any); it is re-applied