From cd55770cbf5197d08a9fd418d3d5d3291efc4a0f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Wed, 8 Jul 2026 14:03:24 +0200 Subject: [PATCH 1/2] Fix DataLoader branch-key collisions for filter values. --- .../Internal/ExpressionHasher.cs | 614 +++++++++++++++++- .../Internal/ExpressionHasherTests.cs | 504 +++++++++++++- .../DataLoaderTests.cs | 130 ++++ .../Data.PostgreSQL.Tests/IntegrationTests.cs | 56 ++ 4 files changed, 1295 insertions(+), 9 deletions(-) diff --git a/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs b/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs index 0fad6a61108..db680c0930f 100644 --- a/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs +++ b/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs @@ -1,7 +1,9 @@ using System.Buffers; using System.Buffers.Text; +using System.Globalization; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text; @@ -18,10 +20,42 @@ internal sealed class ExpressionHasher : ExpressionVisitor private ReadOnlySpan New => "New:"u8; private ReadOnlySpan MemberInit => "MemberInit|"u8; private ReadOnlySpan Binding => "Binding:"u8; + private ReadOnlySpan TypeBinary => "TypeBinary:"u8; + private ReadOnlySpan Conditional => "Conditional|"u8; + private ReadOnlySpan NewArray => "NewArray:"u8; + private ReadOnlySpan Index => "Index:"u8; + private ReadOnlySpan ListInit => "ListInit|"u8; + private ReadOnlySpan Default => "Default:"u8; + private ReadOnlySpan Invocation => "Invocation|"u8; + private ReadOnlySpan Constant => "K:"u8; + private ReadOnlySpan ConstantRoot => "CR:"u8; + private ReadOnlySpan Value => "V:"u8; + private ReadOnlySpan Null => "Z:"u8; + private ReadOnlySpan Str => "S:"u8; + private ReadOnlySpan Bool => "B:"u8; + private ReadOnlySpan Num => "N:"u8; + private ReadOnlySpan EnumTag => "E:"u8; + private ReadOnlySpan Temporal => "T:"u8; + private ReadOnlySpan GuidTag => "G:"u8; + private ReadOnlySpan CollectionStart => "C[:"u8; + private ReadOnlySpan CollectionEnd => "C]:"u8; + private ReadOnlySpan Struct => "W:"u8; + private ReadOnlySpan TypeFallback => "TF:"u8; + private ReadOnlySpan Truncated => "TRUNC:"u8; + private ReadOnlySpan ErrorTag => "ERR:"u8; + + private const int MaxCapturedNodes = 10_000; + private const int MaxCapturedBytes = 64 * 1024; + private const int MaxCapturedDepth = 3; + private const int MaxCapturedStringChars = 2048; + private const byte CharDiscriminator = (byte)TypeCode.Char; + private const byte OtherFormattableDiscriminator = 103; private byte[] _buffer; private readonly int _initialSize; private int _start; + private int _capturedNodeCount; + private int _capturedByteCount; public ExpressionHasher() { @@ -90,6 +124,8 @@ public string Compute() } _start = 0; + _capturedNodeCount = 0; + _capturedByteCount = 0; return hashString; } @@ -119,10 +155,71 @@ protected override Expression VisitMember(MemberExpression node) Append(Member); Append(node.Member); Append('|'); + + if (node.Expression is ConstantExpression constant) + { + Append(ConstantRoot); + AppendType(constant.Type); + Append('|'); + + object? value; + bool evaluated; + + // Reference-type property getters can execute user code. Captured closures use fields, + // and value-type holders cover hoisted ExpressionParameter values. + try + { + switch (node.Member) + { + case FieldInfo field: + value = field.GetValue(constant.Value); + evaluated = true; + break; + + case PropertyInfo property + when property.GetIndexParameters().Length == 0 && constant.Value is ValueType: + value = property.GetValue(constant.Value); + evaluated = true; + break; + + default: + value = null; + evaluated = false; + break; + } + } + catch + { + value = null; + evaluated = false; + } + + if (evaluated) + { + Append(Value); + AppendCapturedValue(value, 0); + } + else + { + AppendMemberTypeFallback(node.Member, constant.Type); + } + + return node; + } + Visit(node.Expression); return node; } + protected override Expression VisitConstant(ConstantExpression node) + { + Append(Constant); + AppendType(node.Type); + Append('|'); + AppendCapturedValue(node.Value, 0); + return node; + } + protected override Expression VisitParameter(ParameterExpression node) { Append(Parameter); @@ -145,6 +242,16 @@ protected override Expression VisitMethodCall(MethodCallExpression node) Append(MethodCall); Append(node.Method); Append('|'); + + if (node.Method.IsGenericMethod) + { + foreach (var typeArgument in node.Method.GetGenericArguments()) + { + AppendType(typeArgument); + Append('|'); + } + } + Visit(node.Object); foreach (var argument in node.Arguments) { @@ -176,6 +283,88 @@ protected override Expression VisitMemberInit(MemberInitExpression node) return node; } + protected override Expression VisitTypeBinary(TypeBinaryExpression node) + { + Append(TypeBinary); + Append((int)node.NodeType); + Append('|'); + AppendType(node.TypeOperand); + Append('|'); + Visit(node.Expression); + return node; + } + + protected override Expression VisitConditional(ConditionalExpression node) + { + Append(Conditional); + Visit(node.Test); + Visit(node.IfTrue); + Visit(node.IfFalse); + return node; + } + + protected override Expression VisitNewArray(NewArrayExpression node) + { + Append(NewArray); + Append((int)node.NodeType); + Append('|'); + AppendType(node.Type); + Append('|'); + foreach (var expression in node.Expressions) + { + Visit(expression); + } + return node; + } + + protected override Expression VisitIndex(IndexExpression node) + { + Append(Index); + Append(node.Indexer); + Append('|'); + Visit(node.Object); + foreach (var argument in node.Arguments) + { + Visit(argument); + } + return node; + } + + protected override Expression VisitListInit(ListInitExpression node) + { + Append(ListInit); + Visit(node.NewExpression); + foreach (var initializer in node.Initializers) + { + Append(initializer.AddMethod); + Append('|'); + foreach (var argument in initializer.Arguments) + { + Visit(argument); + } + } + return node; + } + + protected override Expression VisitDefault(DefaultExpression node) + { + Append(Default); + AppendType(node.Type); + Append('|'); + return node; + } + + protected override Expression VisitInvocation(InvocationExpression node) + { + Append(Invocation); + Visit(node.Expression); + foreach (var argument in node.Arguments) + { + Visit(argument); + } + return node; + } + private void VisitMemberBindingInternal(MemberBinding binding) { Append(Binding); @@ -209,6 +398,416 @@ private void VisitMemberBindingInternal(MemberBinding binding) } } + private void AppendCapturedValue(object? value, int depth) + { + if (++_capturedNodeCount > MaxCapturedNodes + || _capturedByteCount > MaxCapturedBytes + || depth > MaxCapturedDepth) + { + AppendTruncated(value); + return; + } + + switch (value) + { + case null: + Append(Null); + break; + + case string s: + AppendStringValue(s); + break; + + case bool b: + Append(Bool); + Append(b ? (byte)1 : (byte)0); + AddCapturedBytes(1); + break; + + case char c: + AppendFormatted(Num, CharDiscriminator, (int)c, default); + break; + + case Enum e: + AppendEnumValue(e); + break; + + case sbyte i: + AppendFormatted(Num, (byte)TypeCode.SByte, i, default); + break; + + case byte i: + AppendFormatted(Num, (byte)TypeCode.Byte, i, default); + break; + + case short i: + AppendFormatted(Num, (byte)TypeCode.Int16, i, default); + break; + + case ushort i: + AppendFormatted(Num, (byte)TypeCode.UInt16, i, default); + break; + + case int i: + AppendFormatted(Num, (byte)TypeCode.Int32, i, default); + break; + + case uint i: + AppendFormatted(Num, (byte)TypeCode.UInt32, i, default); + break; + + case long i: + AppendFormatted(Num, (byte)TypeCode.Int64, i, default); + break; + + case ulong i: + AppendFormatted(Num, (byte)TypeCode.UInt64, i, default); + break; + + case float f: + AppendFormatted(Num, (byte)TypeCode.Single, f, default); + break; + + case double d: + AppendFormatted(Num, (byte)TypeCode.Double, d, default); + break; + + case decimal d: + AppendFormatted(Num, (byte)TypeCode.Decimal, d, default); + break; + + case DateTime d: + AppendFormatted(Temporal, (byte)TypeCode.DateTime, d, "O"); + break; + + case DateTimeOffset d: + AppendFormatted(Temporal, (byte)'O', d, "O"); + break; + + case TimeSpan t: + AppendFormatted(Temporal, (byte)'S', t, "c"); + break; + + case DateOnly d: + AppendFormatted(Temporal, (byte)'D', d, "O"); + break; + + case TimeOnly t: + AppendFormatted(Temporal, (byte)'Y', t, "O"); + break; + + case Guid g: + AppendFormatted(GuidTag, (byte)'G', g, "D"); + break; + + default: + AppendComplexValue(value, depth); + break; + } + } + + private void AppendComplexValue(object value, int depth) + { + var type = value.GetType(); + + if (type.IsArray) + { + if (value is Array { Rank: 1 } array && array.GetLowerBound(0) == 0) + { + AppendCollection(array, type, array.Length, depth); + } + else + { + AppendTypeFallback(type); + } + return; + } + + if (IsList(type)) + { + var list = (System.Collections.IList)value; + AppendCollection(list, type, list.Count, depth); + return; + } + + if (value is System.Collections.IEnumerable) + { + // Other sequences may be lazy and can hit a database or throw when enumerated. + AppendTypeFallback(type); + return; + } + + if (type.IsValueType) + { + if (value is IFormattable formattable) + { + AppendOtherFormattableValue(formattable, type); + } + else + { + AppendStructValue(value, type, depth); + } + return; + } + + AppendTypeFallback(type); + } + + private void AppendStringValue(string value) + { + Append(Str); + Append(value.Length); + Append(':'); + + // The length prefix keeps string payloads from aliasing element boundaries. Hashing + // the UTF-16 payload directly is lossless for any content, including unpaired + // surrogates, and cannot fail. + var chars = value.Length <= MaxCapturedStringChars + ? value.AsSpan() + : value.AsSpan(0, MaxCapturedStringChars); + + Append(MemoryMarshal.AsBytes(chars)); + AddCapturedBytes(chars.Length * 2); + + if (value.Length > MaxCapturedStringChars) + { + Append(Truncated); + } + } + + private void AppendEnumValue(Enum value) + { + var enumType = value.GetType(); + Append(EnumTag); + AppendType(enumType); + Append('|'); + + try + { + var formatted = value.ToString("D"); + Append(formatted); + AddCapturedBytes(formatted.Length); + } + catch + { + AppendError(enumType); + } + } + + private void AppendFormatted(ReadOnlySpan tag, byte discriminator, T value, ReadOnlySpan format) + where T : IUtf8SpanFormattable + { + Append(tag); + Append(discriminator); + Append(':'); + + try + { + Span utf8 = stackalloc byte[128]; + if (value.TryFormat(utf8, out var written, format, CultureInfo.InvariantCulture)) + { + Append(utf8[..written]); + AddCapturedBytes(written); + return; + } + } + catch + { + // Fall through to the error marker. + } + + AppendError(typeof(T)); + } + + private void AppendCollection(System.Collections.IList items, Type type, int count, int depth) + { + Append(CollectionStart); + AppendType(type); + Append('|'); + Append(count); + Append(':'); + + var position = _start; + try + { + for (var i = 0; i < count; i++) + { + if (_capturedNodeCount > MaxCapturedNodes || _capturedByteCount > MaxCapturedBytes) + { + // A single marker with the remaining count keeps the work bounded. + Append(Truncated); + Append(count - i); + Append('|'); + break; + } + + AppendCapturedValue(items[i], depth + 1); + } + } + catch + { + _start = position; + AppendError(type); + return; + } + + Append(CollectionEnd); + } + + private void AppendStructValue(object value, Type type, int depth) + { + Append(Struct); + AppendType(type); + Append('|'); + + var position = _start; + + // Struct field reads run no user code, so captured value types like record structs, + // tuples, and composite keys can be folded safely. + try + { + var fields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + Array.Sort(fields, static (a, b) => string.CompareOrdinal(a.Name, b.Name)); + + foreach (var field in fields) + { + if (_capturedNodeCount > MaxCapturedNodes || _capturedByteCount > MaxCapturedBytes) + { + Append(Truncated); + break; + } + + Append(field.Name); + Append('='); + AppendCapturedValue(field.GetValue(value), depth + 1); + } + } + catch + { + _start = position; + AppendError(type); + return; + } + + Append('|'); + } + + private void AppendOtherFormattableValue(IFormattable value, Type type) + { + Append(Num); + Append(OtherFormattableDiscriminator); + Append(':'); + AppendType(type); + Append('|'); + + try + { + var formatted = value.ToString(null, CultureInfo.InvariantCulture); + if (formatted is null) + { + Append(Null); + return; + } + + // Length-prefixed like any string so user-defined text cannot alias element boundaries. + AppendStringValue(formatted); + } + catch + { + AppendError(type); + } + } + + private void AppendTruncated(object? value) + { + Append(Truncated); + AddCapturedBytes(32); + + if (value is null) + { + Append(Null); + return; + } + + var type = value.GetType(); + AppendType(type); + + if (TryGetMaterializedCollectionCount(value, type, out var count)) + { + Append('|'); + Append(count); + } + + Append('|'); + } + + private void AppendMemberTypeFallback(MemberInfo member, Type holderType) + { + Append(TypeFallback); + AppendType(holderType); + Append('|'); + + switch (member) + { + case FieldInfo field: + AppendType(field.FieldType); + break; + + case PropertyInfo property: + AppendType(property.PropertyType); + break; + + default: + AppendType(member.DeclaringType ?? holderType); + break; + } + + Append('|'); + } + + private void AppendTypeFallback(Type type) + { + Append(TypeFallback); + AppendType(type); + Append('|'); + } + + private void AppendError(Type type) + { + Append(ErrorTag); + AppendType(type); + Append('|'); + } + + private static bool IsList(Type type) + => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>); + + private static bool TryGetMaterializedCollectionCount(object value, Type type, out int count) + { + if (value is Array { Rank: 1 } array && array.GetLowerBound(0) == 0) + { + count = array.Length; + return true; + } + + if (IsList(type) && value is System.Collections.ICollection collection) + { + count = collection.Count; + return true; + } + + count = 0; + return false; + } + + private void AddCapturedBytes(int count) + { + if (count > 0) + { + _capturedByteCount += count; + } + } + private void Append(int i) { int written; @@ -258,6 +857,9 @@ private void Append(MemberInfo? member) Append(member.Name); } + private void AppendType(Type type) + => Append(type.FullName ?? type.Name); + private void Append(char s) { if (_start == _buffer.Length) @@ -332,7 +934,7 @@ private void ExpandBufferCapacity() { var newBuffer = ArrayPool.Shared.Rent(_buffer.Length * 2); _buffer.AsSpan().CopyTo(newBuffer); - ArrayPool.Shared.Return(_buffer); + ReturnCleared(_buffer); _buffer = newBuffer; } @@ -340,8 +942,16 @@ private Span ExpandRollingBufferCapacity(int bufferIndex) { var newBuffer = ArrayPool.Shared.Rent(_buffer.Length * 2); _buffer.AsSpan()[..bufferIndex].CopyTo(newBuffer); - ArrayPool.Shared.Return(_buffer); + ReturnCleared(_buffer); _buffer = newBuffer; return _buffer.AsSpan()[bufferIndex..]; } + + private static void ReturnCleared(byte[] buffer) + { + // The buffer holds captured predicate values, so scrub it before handing it back + // to the shared pool, mirroring the clear in Compute. + buffer.AsSpan().Clear(); + ArrayPool.Shared.Return(buffer); + } } diff --git a/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs b/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs index 8cc1643fe25..ece71a99694 100644 --- a/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs +++ b/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs @@ -79,7 +79,7 @@ public static void Selector_With_List() var hash = hasher.Add(selector).Compute(); // assert - Assert.Equal("78155299ff3ce114f4f60f97b02a00b9", hash); + Assert.Equal("9d85f008b51c17f727b1ccf836d28d02", hash); } [Fact] @@ -93,7 +93,7 @@ public static void Simple_Predicate() var hash = hasher.Add(selector).Compute(); // assert - Assert.Equal("76892c282824bfdfe59e135a298c926e", hash); + Assert.Equal("a6291badbb36d78585568bddc5fc334f", hash); } [Fact] @@ -150,18 +150,16 @@ public static void BufferResize_Add_Expression() // arrange var hasher = new ExpressionHasher(); var initialBufferSize = hasher.InitialBufferSize; - var expression = Expression.Lambda>(Expression.Constant(new Entity1())); // translated to 8 bytes + var expression = Expression.Lambda>(Expression.Constant(new Entity1())); // act - var length = 0; - while (length < initialBufferSize + 1) + while (hasher.BufferSize == initialBufferSize) { hasher.Add(expression); - length += 8; } // assert - Assert.Equal(initialBufferSize * 2, hasher.BufferSize); + Assert.True(hasher.BufferSize > initialBufferSize); } [Fact] @@ -204,12 +202,504 @@ public static void BufferResize_Add_SortDefinition() Assert.Equal(initialBufferSize * 2, hasher.BufferSize); } + [Fact] + public static void Hoisted_Eq_Different_Value_Should_Produce_Different_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var param = Expression.Parameter(typeof(Entity1), "x"); + var expression1 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror("abc")), + "p")), + param); + var expression2 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror("xyz")), + "p")), + param); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Hoisted_Eq_Same_Value_Should_Produce_Same_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var param = Expression.Parameter(typeof(Entity1), "x"); + var expression1 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror("abc")), + "p")), + param); + var expression2 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror("abc")), + "p")), + param); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.Equal(hash1, hash2); + } + + [Fact] + public static void Closure_Captured_Different_Value_Should_Produce_Different_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var expression1 = Build("abc"); + var expression2 = Build("xyz"); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + + static Expression> Build(string v) => x => x.Name == v; + } + + [Fact] + public static void Bare_Constant_Different_Value_Should_Produce_Different_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var param = Expression.Parameter(typeof(Entity1), "x"); + var expression1 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Constant("abc")), + param); + var expression2 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.Name)), + Expression.Constant("xyz")), + param); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Hoisted_In_List_Different_Elements_Should_Produce_Different_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var param = Expression.Parameter(typeof(Entity1), "x"); + var containsMethod = typeof(Enumerable) + .GetMethods() + .Single(m => m.Name == "Contains" && m.GetParameters().Length == 2) + .MakeGenericMethod(typeof(string)); + var expression1 = Expression.Lambda>( + Expression.Call( + containsMethod, + Expression.Property( + Expression.Constant( + new ExpressionParameterMirror>(new[] { "a", "b" })), + "p"), + Expression.Property(param, nameof(Entity1.Name))), + param); + var expression2 = Expression.Lambda>( + Expression.Call( + containsMethod, + Expression.Property( + Expression.Constant( + new ExpressionParameterMirror>(new[] { "a", "c" })), + "p"), + Expression.Property(param, nameof(Entity1.Name))), + param); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Hoisted_Bool_Different_Value_Should_Produce_Different_Hash() + { + // arrange + var hasher = new ExpressionHasher(); + var param = Expression.Parameter(typeof(Entity1), "x"); + var expression1 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.IsActive)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror(true)), + "p")), + param); + var expression2 = Expression.Lambda>( + Expression.Equal( + Expression.Property(param, nameof(Entity1.IsActive)), + Expression.Property( + Expression.Constant(new ExpressionParameterMirror(false)), + "p")), + param); + + // act + var hash1 = hasher.Add(expression1).Compute(); + var hash2 = hasher.Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Throwing_Property_Getter_On_Reference_Holder_Should_Not_Be_Invoked() + { + // arrange + var holder = new ThrowingPropertyHolder(); + var expression = Expression.Property( + Expression.Constant(holder), + nameof(ThrowingPropertyHolder.Value)); + + // act + var exception = Record.Exception(() => new ExpressionHasher().Add(expression).Compute()); + + // assert + Assert.Null(exception); + Assert.False(holder.WasInvoked); + } + + [Fact] + public static void Lazy_Enumerable_Should_Not_Be_Enumerated() + { + // arrange + var enumerable = new ThrowingEnumerable(); + var expression = Expression.Constant(enumerable); + + // act + var exception = Record.Exception(() => new ExpressionHasher().Add(expression).Compute()); + + // assert + Assert.Null(exception); + Assert.False(enumerable.Enumerated); + } + + [Fact] + public static void Queryable_Should_Not_Be_Enumerated() + { + // arrange + var queryable = new ThrowingQueryable(); + var expression = Expression.Constant(queryable); + + // act + var exception = Record.Exception(() => new ExpressionHasher().Add(expression).Compute()); + + // assert + Assert.Null(exception); + Assert.False(queryable.Enumerated); + } + + [Fact] + public static void Self_Referential_Collection_Should_Terminate_And_Be_Deterministic() + { + // arrange + var self = new object[2]; + self[0] = self; + self[1] = "x"; + var expression = Expression.Constant(self); + + // act + string? hash1 = null; + string? hash2 = null; + var exception1 = Record.Exception(() => hash1 = new ExpressionHasher().Add(expression).Compute()); + var exception2 = Record.Exception(() => hash2 = new ExpressionHasher().Add(expression).Compute()); + + // assert + Assert.Null(exception1); + Assert.Null(exception2); + Assert.Equal(hash1, hash2); + } + + [Fact] + public static void Captured_DateTime_Sub_Second_Difference_Should_Produce_Different_Hash() + { + // arrange + var value1 = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc).AddTicks(1_230_000); + var value2 = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc).AddTicks(4_560_000); + + // act + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value2); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Captured_List_Element_Boundaries_Should_Produce_Different_Hash() + { + // arrange + var value1 = new[] { "a,b" }; + var value2 = new[] { "a", "b" }; + + // act + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value2); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Same_List_Values_Different_Instances_Should_Produce_Same_Hash() + { + // arrange + var value1 = new[] { "a", "b" }; + var value2 = new[] { "a", "b" }; + + // act + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value2); + + // assert + Assert.Equal(hash1, hash2); + } + + [Fact] + public static void Constant_Int_And_String_As_Object_Should_Produce_Different_Hash() + { + // arrange + var expression1 = Expression.Constant(1, typeof(object)); + var expression2 = Expression.Constant("1", typeof(object)); + + // act + var hash1 = new ExpressionHasher().Add(expression1).Compute(); + var hash2 = new ExpressionHasher().Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Long_String_Should_Hash_Deterministically_And_Distinguish_Prefix() + { + // arrange + var longValue = new string('a', 5000); + var prefixed1 = "A" + new string('a', 5000); + var prefixed2 = "B" + new string('a', 5000); + + // act + var hash1 = HashConstant(longValue); + var hash2 = HashConstant(longValue); + var prefixedHash1 = HashConstant(prefixed1); + var prefixedHash2 = HashConstant(prefixed2); + + // assert + Assert.Equal(hash1, hash2); + Assert.NotEqual(prefixedHash1, prefixedHash2); + } + + [Fact] + public static void Bare_Bool_Constants_Should_Produce_Different_Hash() + { + // arrange + var expression1 = Expression.Constant(true); + var expression2 = Expression.Constant(false); + + // act + var hash1 = new ExpressionHasher().Add(expression1).Compute(); + var hash2 = new ExpressionHasher().Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Captured_Record_Struct_Different_Value_Should_Produce_Different_Hash() + { + // arrange + var value1 = new StructKey(1, "a"); + var value2 = new StructKey(2, "a"); + + // act + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value2); + var hash3 = HashConstant(new StructKey(1, "a")); + + // assert + Assert.NotEqual(hash1, hash2); + Assert.Equal(hash1, hash3); + } + + [Fact] + public static void Captured_Tuple_Member_Chain_Should_Produce_Different_Hash() + { + // arrange + var expression1 = BuildTuplePredicate(("a", "b")); + var expression2 = BuildTuplePredicate(("c", "d")); + + // act + var hash1 = new ExpressionHasher().Add(expression1).Compute(); + var hash2 = new ExpressionHasher().Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + + static Expression> BuildTuplePredicate((string A, string B) pair) + => x => x.Name == pair.A; + } + + [Fact] + public static void Formattable_Struct_List_Boundaries_Should_Produce_Different_Hash() + { + // arrange + // A payload that reproduces the per-element header could previously shift element + // boundaries so that two different lists encoded to identical bytes. + var header = "N" + (char)103 + ":" + typeof(AmbiguousFormattable).FullName + "|"; + var value1 = new[] { new AmbiguousFormattable("1" + header + "2"), new AmbiguousFormattable("3") }; + var value2 = new[] { new AmbiguousFormattable("1"), new AmbiguousFormattable("2" + header + "3") }; + + // act + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value2); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Type_Check_Predicate_Different_Type_Should_Produce_Different_Hash() + { + // arrange + Expression> expression1 = x => x.Entity is Entity2; + Expression> expression2 = x => x.Entity is Entity3; + + // act + var hash1 = new ExpressionHasher().Add(expression1).Compute(); + var hash2 = new ExpressionHasher().Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Generic_Method_Argument_Should_Produce_Different_Hash() + { + // arrange + Expression> expression1 = x => x.Entities.OfType().Any(); + Expression> expression2 = x => x.Entities.OfType().Any(); + + // act + var hash1 = new ExpressionHasher().Add(expression1).Compute(); + var hash2 = new ExpressionHasher().Add(expression2).Compute(); + + // assert + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public static void Large_Captured_List_Should_Be_Bounded_And_Deterministic() + { + // arrange + var value = new int[100_000]; + var hasher = new ExpressionHasher(); + + // act + hasher.Add(Expression.Constant(value)); + var bufferSize = hasher.BufferSize; + var hash1 = hasher.Compute(); + var hash2 = HashConstant(value); + + // assert + Assert.True(bufferSize < 1_048_576, $"buffer grew to {bufferSize} bytes"); + Assert.Equal(hash1, hash2); + } + + private static string HashConstant(object value) + => new ExpressionHasher().Add(Expression.Constant(value)).Compute(); + + private readonly record struct ExpressionParameterMirror(T p); + + private readonly record struct StructKey(int Id, string Name); + + private readonly struct AmbiguousFormattable(string text) : IFormattable + { + public string ToString(string? format, IFormatProvider? formatProvider) => text; + } + + private sealed class ThrowingPropertyHolder + { + public bool WasInvoked { get; private set; } + + public string Value + { + get + { + WasInvoked = true; + throw new InvalidOperationException(); + } + } + } + + private sealed class ThrowingEnumerable : IEnumerable + { + public bool Enumerated { get; private set; } + + public IEnumerator GetEnumerator() + { + Enumerated = true; + throw new InvalidOperationException(); + } + + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() + => GetEnumerator(); + } + + private sealed class ThrowingQueryable : IQueryable + { + public bool Enumerated { get; private set; } + + public Type ElementType => typeof(string); + + public Expression Expression => System.Linq.Expressions.Expression.Constant(this); + + public IQueryProvider Provider => throw new InvalidOperationException(); + + public IEnumerator GetEnumerator() + { + Enumerated = true; + throw new InvalidOperationException(); + } + + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() + => GetEnumerator(); + } + public class Entity1 { public string Name { get; set; } = null!; public string? Description { get; set; } + public bool IsActive { get; set; } + public IEntity Entity { get; set; } = null!; public List Entities { get; set; } = null!; diff --git a/src/HotChocolate/Data/test/Data.Filters.SqlServer.Tests/DataLoaderTests.cs b/src/HotChocolate/Data/test/Data.Filters.SqlServer.Tests/DataLoaderTests.cs index 2d7bd95496e..6df64219052 100644 --- a/src/HotChocolate/Data/test/Data.Filters.SqlServer.Tests/DataLoaderTests.cs +++ b/src/HotChocolate/Data/test/Data.Filters.SqlServer.Tests/DataLoaderTests.cs @@ -1,4 +1,5 @@ using System.Linq.Expressions; +using System.Text.Json; using GreenDonut; using GreenDonut.Data; using HotChocolate.Execution; @@ -185,6 +186,94 @@ public async Task Filter_With_Expression_Null() .MatchMarkdownSnapshot(); } + [Fact] + public async Task Filter_With_Aliased_Filtering_Same_Operator_Different_Value() + { + // Arrange + var context = new CatalogContext(); + await context.SeedAsync(); + + // Act + var result = await new ServiceCollection() + .AddTransient(_ => context) + .AddGraphQL() + .AddFiltering() + .AddQueryType() + .AddPagingArguments() + .ModifyRequestOptions(o => o.IncludeExceptionDetails = true) + .ExecuteRequestAsync( + """ + { + a: productsByBrand(brandId: 1, where: { name: { eq: "Product 0-0" } }) { + name + } + b: productsByBrand(brandId: 1, where: { name: { eq: "Product 0-1" } }) { + name + } + } + """, + cancellationToken: Xunit.TestContext.Current.CancellationToken); + using var doc = JsonDocument.Parse(result.ToJson()); + + // Assert + var data = doc.RootElement.GetProperty("data"); + var aNames = data.GetProperty("a") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + var bNames = data.GetProperty("b") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + + Assert.Equal(new[] { "Product 0-0" }, aNames); + Assert.Equal(new[] { "Product 0-1" }, bNames); + } + + [Fact] + public async Task Filter_With_Aliased_Filtering_Same_Operator_In_Different_Value() + { + // Arrange + var context = new CatalogContext(); + await context.SeedAsync(); + + // Act + var result = await new ServiceCollection() + .AddTransient(_ => context) + .AddGraphQL() + .AddFiltering() + .AddQueryType() + .AddPagingArguments() + .ModifyRequestOptions(o => o.IncludeExceptionDetails = true) + .ExecuteRequestAsync( + """ + { + a: productsByBrand(brandId: 1, where: { name: { in: ["Product 0-0"] } }) { + name + } + b: productsByBrand(brandId: 1, where: { name: { in: ["Product 0-1"] } }) { + name + } + } + """, + cancellationToken: Xunit.TestContext.Current.CancellationToken); + using var doc = JsonDocument.Parse(result.ToJson()); + + // Assert + var data = doc.RootElement.GetProperty("data"); + var aNames = data.GetProperty("a") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + var bNames = data.GetProperty("b") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + + Assert.Equal(new[] { "Product 0-0" }, aNames); + Assert.Equal(new[] { "Product 0-1" }, bNames); + } + public class Query { public async Task FilterExpression( @@ -306,6 +395,47 @@ protected override async Task> LoadBatchAsyn return x.GroupBy(t => t.Id).ToDictionary(t => t.Key, t => t.ToArray()); } } + + public class AliasFilterQuery + { + [UseFiltering] + public async Task ProductsByBrand( + int brandId, + IFilterContext context, + AliasProductsByBrandIdDataLoader productsByBrandId, + CancellationToken cancellationToken) + => await productsByBrandId.Where(context).LoadAsync(brandId, cancellationToken); + } + + public class AliasProductsByBrandIdDataLoader : StatefulBatchDataLoader + { + private readonly IServiceProvider _services; + + public AliasProductsByBrandIdDataLoader( + IServiceProvider services, + IBatchScheduler batchScheduler, + DataLoaderOptions options) : base(batchScheduler, options) + { + _services = services; + } + + protected override async Task> LoadBatchAsync( + IReadOnlyList keys, + DataLoaderFetchContext context, + CancellationToken cancellationToken) + { + var catalogContext = _services.GetRequiredService(); + + var query = catalogContext.Products + .Where(t => keys.Contains(t.BrandId)) + .Where(context.GetPredicate()); + var list = await query.ToListAsync(cancellationToken); + + return list + .GroupBy(t => t.BrandId) + .ToDictionary(t => t.Key, t => t.ToArray()); + } + } } file static class Extensions diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs index d3d3687f2a5..a5168eb02fb 100644 --- a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs +++ b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs @@ -14,6 +14,7 @@ using Squadron; using System.Reflection; using System.Runtime.CompilerServices; +using System.Text.Json; using System.Text.RegularExpressions; using static CookieCrumble.TestEnvironment; @@ -731,6 +732,61 @@ public async Task MaxPageSize_Should_Be_Enforced_On_Connection_Without_UseConnec """); } + [Fact] + public async Task Query_Brand_Products_Aliased_Same_Operator_Different_Value() + { + // arrange + // Two distinct queries must be issued, one per predicate value; a single query + // means the aliases collapsed onto one DataLoader branch. + using var interceptor = new TestQueryInterceptor(); + + // act + var result = await ExecuteAsync( + """ + { + brands(where: { name: { eq: "Daybird" } }) { + nodes { + name + a: products(where: { name: { eq: "Wanderer Black Hiking Boots" } }) { + nodes { + name + } + } + b: products(where: { name: { eq: "Trailblaze hiking backpack" } }) { + nodes { + name + } + } + } + } + } + """); + using var doc = JsonDocument.Parse(result.ToJson()); + + // assert + var node = doc.RootElement + .GetProperty("data") + .GetProperty("brands") + .GetProperty("nodes")[0]; + var aNames = node.GetProperty("a") + .GetProperty("nodes") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + var bNames = node.GetProperty("b") + .GetProperty("nodes") + .EnumerateArray() + .Select(e => e.GetProperty("name").GetString()) + .ToArray(); + + Assert.Equal(new[] { "Wanderer Black Hiking Boots" }, aNames); + Assert.Equal(new[] { "Trailblaze hiking backpack" }, bNames); + + // One query for the brands page and one per product predicate value. A count of + // two means the aliases collapsed onto a single DataLoader branch. + Assert.Equal(3, interceptor.Queries.Count); + } + private static ServiceProvider CreateServer(string connectionString, int? maxPageSize = null) { var services = new ServiceCollection(); From 311ee75e930203c70324f2c6877a21f2cde1537d Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Wed, 8 Jul 2026 14:59:19 +0200 Subject: [PATCH 2/2] edits --- .../Internal/ExpressionHasher.cs | 345 +++++------------- .../Internal/ExpressionHasherTests.cs | 38 +- 2 files changed, 107 insertions(+), 276 deletions(-) diff --git a/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs b/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs index db680c0930f..c538b06594a 100644 --- a/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs +++ b/src/GreenDonut/src/GreenDonut.Data/Internal/ExpressionHasher.cs @@ -1,8 +1,8 @@ using System.Buffers; using System.Buffers.Text; -using System.Globalization; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text; @@ -41,21 +41,12 @@ internal sealed class ExpressionHasher : ExpressionVisitor private ReadOnlySpan CollectionEnd => "C]:"u8; private ReadOnlySpan Struct => "W:"u8; private ReadOnlySpan TypeFallback => "TF:"u8; - private ReadOnlySpan Truncated => "TRUNC:"u8; - private ReadOnlySpan ErrorTag => "ERR:"u8; - private const int MaxCapturedNodes = 10_000; - private const int MaxCapturedBytes = 64 * 1024; - private const int MaxCapturedDepth = 3; - private const int MaxCapturedStringChars = 2048; - private const byte CharDiscriminator = (byte)TypeCode.Char; - private const byte OtherFormattableDiscriminator = 103; + private const int MaxCapturedDepth = 8; private byte[] _buffer; private readonly int _initialSize; private int _start; - private int _capturedNodeCount; - private int _capturedByteCount; public ExpressionHasher() { @@ -124,8 +115,6 @@ public string Compute() } _start = 0; - _capturedNodeCount = 0; - _capturedByteCount = 0; return hashString; } @@ -162,46 +151,24 @@ protected override Expression VisitMember(MemberExpression node) AppendType(constant.Type); Append('|'); - object? value; - bool evaluated; - // Reference-type property getters can execute user code. Captured closures use fields, // and value-type holders cover hoisted ExpressionParameter values. - try + switch (node.Member) { - switch (node.Member) - { - case FieldInfo field: - value = field.GetValue(constant.Value); - evaluated = true; - break; - - case PropertyInfo property - when property.GetIndexParameters().Length == 0 && constant.Value is ValueType: - value = property.GetValue(constant.Value); - evaluated = true; - break; - - default: - value = null; - evaluated = false; - break; - } - } - catch - { - value = null; - evaluated = false; - } + case FieldInfo field: + Append(Value); + AppendCapturedValue(field.GetValue(constant.Value), 0); + break; - if (evaluated) - { - Append(Value); - AppendCapturedValue(value, 0); - } - else - { - AppendMemberTypeFallback(node.Member, constant.Type); + case PropertyInfo property + when property.GetIndexParameters().Length == 0 && constant.Value is ValueType: + Append(Value); + AppendCapturedValue(property.GetValue(constant.Value), 0); + break; + + default: + AppendMemberTypeFallback(node.Member, constant.Type); + break; } return node; @@ -400,12 +367,11 @@ private void VisitMemberBindingInternal(MemberBinding binding) private void AppendCapturedValue(object? value, int depth) { - if (++_capturedNodeCount > MaxCapturedNodes - || _capturedByteCount > MaxCapturedBytes - || depth > MaxCapturedDepth) + if (depth > MaxCapturedDepth) { - AppendTruncated(value); - return; + throw new NotSupportedException( + "The captured value graph is too deep or cyclic and cannot be hashed into " + + $"a DataLoader branch key (value type: {value?.GetType().FullName ?? "null"})."); } switch (value) @@ -421,11 +387,10 @@ private void AppendCapturedValue(object? value, int depth) case bool b: Append(Bool); Append(b ? (byte)1 : (byte)0); - AddCapturedBytes(1); break; case char c: - AppendFormatted(Num, CharDiscriminator, (int)c, default); + AppendNumber(ValueKind.Char, c); break; case Enum e: @@ -433,71 +398,74 @@ private void AppendCapturedValue(object? value, int depth) break; case sbyte i: - AppendFormatted(Num, (byte)TypeCode.SByte, i, default); + AppendNumber(ValueKind.SByte, i); break; case byte i: - AppendFormatted(Num, (byte)TypeCode.Byte, i, default); + AppendNumber(ValueKind.Byte, i); break; case short i: - AppendFormatted(Num, (byte)TypeCode.Int16, i, default); + AppendNumber(ValueKind.Int16, i); break; case ushort i: - AppendFormatted(Num, (byte)TypeCode.UInt16, i, default); + AppendNumber(ValueKind.UInt16, i); break; case int i: - AppendFormatted(Num, (byte)TypeCode.Int32, i, default); + AppendNumber(ValueKind.Int32, i); break; case uint i: - AppendFormatted(Num, (byte)TypeCode.UInt32, i, default); + AppendNumber(ValueKind.UInt32, i); break; case long i: - AppendFormatted(Num, (byte)TypeCode.Int64, i, default); + AppendNumber(ValueKind.Int64, i); break; case ulong i: - AppendFormatted(Num, (byte)TypeCode.UInt64, i, default); + AppendNumber(ValueKind.UInt64, i); break; case float f: - AppendFormatted(Num, (byte)TypeCode.Single, f, default); + AppendNumber(ValueKind.Single, f); break; case double d: - AppendFormatted(Num, (byte)TypeCode.Double, d, default); + AppendNumber(ValueKind.Double, d); break; case decimal d: - AppendFormatted(Num, (byte)TypeCode.Decimal, d, default); + AppendNumber(ValueKind.Decimal, d); break; case DateTime d: - AppendFormatted(Temporal, (byte)TypeCode.DateTime, d, "O"); + AppendNumber(ValueKind.DateTime, d.Ticks); + Append((byte)d.Kind); break; case DateTimeOffset d: - AppendFormatted(Temporal, (byte)'O', d, "O"); + AppendNumber(ValueKind.DateTimeOffset, d.Ticks); + AppendRaw(d.Offset.Ticks); break; case TimeSpan t: - AppendFormatted(Temporal, (byte)'S', t, "c"); + AppendNumber(ValueKind.TimeSpan, t.Ticks); break; case DateOnly d: - AppendFormatted(Temporal, (byte)'D', d, "O"); + AppendNumber(ValueKind.DateOnly, d.DayNumber); break; case TimeOnly t: - AppendFormatted(Temporal, (byte)'Y', t, "O"); + AppendNumber(ValueKind.TimeOnly, t.Ticks); break; case Guid g: - AppendFormatted(GuidTag, (byte)'G', g, "D"); + Append(GuidTag); + AppendRaw(g); break; default: @@ -532,24 +500,20 @@ private void AppendComplexValue(object value, int depth) if (value is System.Collections.IEnumerable) { - // Other sequences may be lazy and can hit a database or throw when enumerated. + // Sequences other than arrays and List may be lazy and are never enumerated; + // they contribute their type identity only. AppendTypeFallback(type); return; } if (type.IsValueType) { - if (value is IFormattable formattable) - { - AppendOtherFormattableValue(formattable, type); - } - else - { - AppendStructValue(value, type, depth); - } + AppendStructValue(value, type, depth); return; } + // Reference types are never evaluated beyond their type identity so that hashing + // cannot execute user code. AppendTypeFallback(type); } @@ -559,64 +523,34 @@ private void AppendStringValue(string value) Append(value.Length); Append(':'); - // The length prefix keeps string payloads from aliasing element boundaries. Hashing - // the UTF-16 payload directly is lossless for any content, including unpaired - // surrogates, and cannot fail. - var chars = value.Length <= MaxCapturedStringChars - ? value.AsSpan() - : value.AsSpan(0, MaxCapturedStringChars); - - Append(MemoryMarshal.AsBytes(chars)); - AddCapturedBytes(chars.Length * 2); - - if (value.Length > MaxCapturedStringChars) - { - Append(Truncated); - } + // The length prefix keeps string payloads from aliasing element boundaries. The + // UTF-16 payload is hashed directly, which is lossless for any content, including + // unpaired surrogates. + Append(MemoryMarshal.AsBytes(value.AsSpan())); } private void AppendEnumValue(Enum value) { - var enumType = value.GetType(); Append(EnumTag); - AppendType(enumType); + AppendType(value.GetType()); Append('|'); - - try - { - var formatted = value.ToString("D"); - Append(formatted); - AddCapturedBytes(formatted.Length); - } - catch - { - AppendError(enumType); - } + Append(value.ToString("D")); } - private void AppendFormatted(ReadOnlySpan tag, byte discriminator, T value, ReadOnlySpan format) - where T : IUtf8SpanFormattable + private void AppendNumber(ValueKind kind, T value) + where T : unmanaged { - Append(tag); - Append(discriminator); - Append(':'); - - try - { - Span utf8 = stackalloc byte[128]; - if (value.TryFormat(utf8, out var written, format, CultureInfo.InvariantCulture)) - { - Append(utf8[..written]); - AddCapturedBytes(written); - return; - } - } - catch - { - // Fall through to the error marker. - } + Append(Num); + Append((byte)kind); + AppendRaw(value); + } - AppendError(typeof(T)); + private void AppendRaw(T value) + where T : unmanaged + { + Span bytes = stackalloc byte[16]; + MemoryMarshal.Write(bytes, in value); + Append(bytes[..Unsafe.SizeOf()]); } private void AppendCollection(System.Collections.IList items, Type type, int count, int depth) @@ -627,28 +561,9 @@ private void AppendCollection(System.Collections.IList items, Type type, int cou Append(count); Append(':'); - var position = _start; - try + for (var i = 0; i < count; i++) { - for (var i = 0; i < count; i++) - { - if (_capturedNodeCount > MaxCapturedNodes || _capturedByteCount > MaxCapturedBytes) - { - // A single marker with the remaining count keeps the work bounded. - Append(Truncated); - Append(count - i); - Append('|'); - break; - } - - AppendCapturedValue(items[i], depth + 1); - } - } - catch - { - _start = position; - AppendError(type); - return; + AppendCapturedValue(items[i], depth + 1); } Append(CollectionEnd); @@ -660,82 +575,16 @@ private void AppendStructValue(object value, Type type, int depth) AppendType(type); Append('|'); - var position = _start; - // Struct field reads run no user code, so captured value types like record structs, // tuples, and composite keys can be folded safely. - try - { - var fields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - Array.Sort(fields, static (a, b) => string.CompareOrdinal(a.Name, b.Name)); - - foreach (var field in fields) - { - if (_capturedNodeCount > MaxCapturedNodes || _capturedByteCount > MaxCapturedBytes) - { - Append(Truncated); - break; - } - - Append(field.Name); - Append('='); - AppendCapturedValue(field.GetValue(value), depth + 1); - } - } - catch - { - _start = position; - AppendError(type); - return; - } - - Append('|'); - } - - private void AppendOtherFormattableValue(IFormattable value, Type type) - { - Append(Num); - Append(OtherFormattableDiscriminator); - Append(':'); - AppendType(type); - Append('|'); - - try - { - var formatted = value.ToString(null, CultureInfo.InvariantCulture); - if (formatted is null) - { - Append(Null); - return; - } - - // Length-prefixed like any string so user-defined text cannot alias element boundaries. - AppendStringValue(formatted); - } - catch - { - AppendError(type); - } - } + var fields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + Array.Sort(fields, static (a, b) => string.CompareOrdinal(a.Name, b.Name)); - private void AppendTruncated(object? value) - { - Append(Truncated); - AddCapturedBytes(32); - - if (value is null) - { - Append(Null); - return; - } - - var type = value.GetType(); - AppendType(type); - - if (TryGetMaterializedCollectionCount(value, type, out var count)) + foreach (var field in fields) { - Append('|'); - Append(count); + Append(field.Name); + Append('='); + AppendCapturedValue(field.GetValue(value), depth + 1); } Append('|'); @@ -772,40 +621,28 @@ private void AppendTypeFallback(Type type) Append('|'); } - private void AppendError(Type type) - { - Append(ErrorTag); - AppendType(type); - Append('|'); - } - private static bool IsList(Type type) => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>); - private static bool TryGetMaterializedCollectionCount(object value, Type type, out int count) - { - if (value is Array { Rank: 1 } array && array.GetLowerBound(0) == 0) - { - count = array.Length; - return true; - } - - if (IsList(type) && value is System.Collections.ICollection collection) - { - count = collection.Count; - return true; - } - - count = 0; - return false; - } - - private void AddCapturedBytes(int count) - { - if (count > 0) - { - _capturedByteCount += count; - } + private enum ValueKind : byte + { + SByte = 1, + Byte, + Int16, + UInt16, + Int32, + UInt32, + Int64, + UInt64, + Single, + Double, + Decimal, + Char, + DateTime, + DateTimeOffset, + TimeSpan, + DateOnly, + TimeOnly } private void Append(int i) diff --git a/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs b/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs index ece71a99694..6e6a2337d01 100644 --- a/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs +++ b/src/GreenDonut/test/GreenDonut.Data.Tests/Internal/ExpressionHasherTests.cs @@ -417,24 +417,20 @@ public static void Queryable_Should_Not_Be_Enumerated() } [Fact] - public static void Self_Referential_Collection_Should_Terminate_And_Be_Deterministic() + public static void Self_Referential_Collection_Should_Throw() { // arrange + // a cyclic captured value has no hashable identity and must fail loudly var self = new object[2]; self[0] = self; self[1] = "x"; var expression = Expression.Constant(self); // act - string? hash1 = null; - string? hash2 = null; - var exception1 = Record.Exception(() => hash1 = new ExpressionHasher().Add(expression).Compute()); - var exception2 = Record.Exception(() => hash2 = new ExpressionHasher().Add(expression).Compute()); + var exception = Record.Exception(() => new ExpressionHasher().Add(expression).Compute()); // assert - Assert.Null(exception1); - Assert.Null(exception2); - Assert.Equal(hash1, hash2); + Assert.IsType(exception); } [Fact] @@ -567,14 +563,12 @@ static Expression> BuildTuplePredicate((string A, string B) } [Fact] - public static void Formattable_Struct_List_Boundaries_Should_Produce_Different_Hash() + public static void Struct_List_Element_Boundaries_Should_Produce_Different_Hash() { // arrange - // A payload that reproduces the per-element header could previously shift element - // boundaries so that two different lists encoded to identical bytes. - var header = "N" + (char)103 + ":" + typeof(AmbiguousFormattable).FullName + "|"; - var value1 = new[] { new AmbiguousFormattable("1" + header + "2"), new AmbiguousFormattable("3") }; - var value2 = new[] { new AmbiguousFormattable("1"), new AmbiguousFormattable("2" + header + "3") }; + // element content must not shift across element boundaries + var value1 = new[] { new AmbiguousFormattable("1"), new AmbiguousFormattable("23") }; + var value2 = new[] { new AmbiguousFormattable("12"), new AmbiguousFormattable("3") }; // act var hash1 = HashConstant(value1); @@ -615,21 +609,21 @@ public static void Generic_Method_Argument_Should_Produce_Different_Hash() } [Fact] - public static void Large_Captured_List_Should_Be_Bounded_And_Deterministic() + public static void Large_Captured_List_Should_Be_Deterministic_And_Value_Sensitive() { // arrange - var value = new int[100_000]; - var hasher = new ExpressionHasher(); + var value1 = new int[100_000]; + var value2 = new int[100_000]; + value2[^1] = 1; // act - hasher.Add(Expression.Constant(value)); - var bufferSize = hasher.BufferSize; - var hash1 = hasher.Compute(); - var hash2 = HashConstant(value); + var hash1 = HashConstant(value1); + var hash2 = HashConstant(value1); + var hash3 = HashConstant(value2); // assert - Assert.True(bufferSize < 1_048_576, $"buffer grew to {bufferSize} bytes"); Assert.Equal(hash1, hash2); + Assert.NotEqual(hash1, hash3); } private static string HashConstant(object value)