diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution.Types/PlannerTopologyCache.cs b/src/HotChocolate/Fusion/src/Fusion.Execution.Types/PlannerTopologyCache.cs index 82074318f42..2c51a8ce96e 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution.Types/PlannerTopologyCache.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution.Types/PlannerTopologyCache.cs @@ -81,7 +81,7 @@ public bool TryGetTypeScatter( out TypeScatterInfo scatter) => _typeScatter.TryGetValue(typeName, out scatter); - private static HashSet CollectSchemaNames( + private static ImmutableArray CollectSchemaNames( IEnumerable complexTypes) { var schemaNames = new HashSet(StringComparer.Ordinal); @@ -94,7 +94,7 @@ private static HashSet CollectSchemaNames( } } - return schemaNames; + return [.. schemaNames.OrderBy(static t => t, StringComparer.Ordinal)]; } private static Dictionary BuildFieldResolutions( diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs index f0b86520753..4b7859ae1fe 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs @@ -412,7 +412,7 @@ private static void IndexDependencies( // Plan steps store which steps they feed into ("dependents"). // We invert that here so each step knows which steps it // depends on, which is what the executor needs for scheduling. - foreach (var dependent in operationPlanStep.Dependents) + foreach (var dependent in operationPlanStep.Dependents.Order()) { if (!ctx.DependenciesByStepId.TryGetValue(dependent, out var dependencies)) { @@ -446,7 +446,7 @@ private static void IndexDependencies( ctx.BranchesByNodeId.Add( nodePlanStep.Id, - nodePlanStep.Branches.ToDictionary(x => x.Key, x => x.Value.Id)); + nodePlanStep.Branches.ToDictionary(static t => t.Key, static t => t.Value.Id)); ctx.FallbackByNodeId.Add(nodePlanStep.Id, nodePlanStep.FallbackQuery.Id); } } @@ -566,7 +566,7 @@ private static OperationExecutionNode CreateOperationExecutionNode( operationStep.Conditions, requiresFileUpload); - foreach (var parentDependency in operationStep.ParentDependencies) + foreach (var parentDependency in operationStep.ParentDependencies.OrderBy(static t => t.StepId)) { node.AddParentDependency(parentDependency.StepId); } @@ -615,9 +615,9 @@ private static void MergeAndBatchOperations( // step below will rewrite the dependency lookup as it merges nodes. var originalDependencies = new Dictionary(ctx.DependenciesByStepId.Count); - foreach (var (nodeId, dependencies) in ctx.DependenciesByStepId) + foreach (var (nodeId, dependencies) in ctx.DependenciesByStepId.OrderBy(static t => t.Key)) { - originalDependencies[nodeId] = dependencies.ToArray(); + originalDependencies[nodeId] = [.. dependencies.Order()]; } var perOperationDependencies = GroupBySchemaAndDepthIntoBatches( @@ -639,7 +639,7 @@ private static Dictionary MergeStructurallyIdenticalOperations { var candidates = new Dictionary>(StringComparer.Ordinal); - foreach (var node in ctx.ExecutionNodes.Values.OfType()) + foreach (var node in ctx.ExecutionNodes.Values.OfType().OrderBy(static t => t.Id)) { if (node.Operation.Type != OperationType.Query) { @@ -664,7 +664,7 @@ private static Dictionary MergeStructurallyIdenticalOperations var mergeResults = new Dictionary(); - foreach (var (_, equivalentNodes) in candidates) + foreach (var (_, equivalentNodes) in candidates.OrderBy(static t => t.Key, StringComparer.Ordinal)) { if (equivalentNodes.Count <= 1) { @@ -770,8 +770,8 @@ private static Dictionary> var queryNodes = ctx.ExecutionNodes.Values .OfType() - .Where(n => n.Operation.Type == OperationType.Query) - .Where(n => !IsNodeFieldBound(n.Id, ctx, nodeFieldBoundCache)) + .Where(n => n.Operation.Type == OperationType.Query && !IsNodeFieldBound(n.Id, ctx, nodeFieldBoundCache)) + .OrderBy(static t => t.Id) .ToList(); var depthLookup = new Dictionary(); @@ -801,7 +801,9 @@ private static Dictionary> // Process from shallowest to deepest so that deeper groups // reference the already-redirected identifiers from earlier merges. - foreach (var (_, groupMembers) in batchGroups.OrderBy(t => t.Key.depth)) + foreach (var (_, groupMembers) in batchGroups + .OrderBy(static t => t.Key.depth) + .ThenBy(static t => t.Key.schema, StringComparer.Ordinal)) { if (groupMembers.Count <= 1) { @@ -864,7 +866,7 @@ private static void WrapRemainingMergedOperations( Dictionary> perOperationDependencies, Dictionary originalDependencies) { - foreach (var (primaryId, merge) in remainingMerges) + foreach (var (primaryId, merge) in remainingMerges.OrderBy(static t => t.Key)) { var operationDefinition = CreateBatchOperationDefinition(merge); var standaloneBatchNode = new OperationBatchExecutionNode(primaryId, [operationDefinition]); @@ -896,7 +898,7 @@ private static void WirePerOperationDependencies( var planNodeById = new Dictionary(); - foreach (var node in ctx.ExecutionNodes.Values) + foreach (var node in ctx.ExecutionNodes.Values.OrderBy(static t => t.Id)) { planNodeById[node.Id] = node; @@ -909,9 +911,9 @@ private static void WirePerOperationDependencies( } } - foreach (var (_, memberDependencies) in perOperationDependencies) + foreach (var (_, memberDependencies) in perOperationDependencies.OrderBy(static t => t.Key.Id)) { - foreach (var (operationId, dependencyIds) in memberDependencies) + foreach (var (operationId, dependencyIds) in memberDependencies.OrderBy(static t => t.Key)) { if (planNodeById.TryGetValue(operationId, out var operationNode) && operationNode is OperationDefinition operationDefinition) @@ -1110,7 +1112,7 @@ private static void WireOperationDependencies(ExecutionPlanBuildContext ctx) // inner operation identifier also maps back to the parent batch node. var executionNodeById = new Dictionary(); - foreach (var node in ctx.ExecutionNodes.Values) + foreach (var node in ctx.ExecutionNodes.Values.OrderBy(static t => t.Id)) { executionNodeById[node.Id] = node; @@ -1123,7 +1125,7 @@ private static void WireOperationDependencies(ExecutionPlanBuildContext ctx) } } - foreach (var (nodeId, stepDependencies) in ctx.DependenciesByStepId) + foreach (var (nodeId, stepDependencies) in ctx.DependenciesByStepId.OrderBy(static t => t.Key)) { if (!ctx.ExecutionNodes.TryGetValue(nodeId, out var entry) || entry is not (OperationExecutionNode or OperationBatchExecutionNode)) @@ -1138,7 +1140,7 @@ private static void WireOperationDependencies(ExecutionPlanBuildContext ctx) } // For a standalone operation node, attach dependencies directly. - foreach (var dependencyId in stepDependencies) + foreach (var dependencyId in stepDependencies.Order()) { if (!ctx.ExecutionNodes.TryGetValue(dependencyId, out var childEntry) || childEntry is not ( @@ -1163,7 +1165,7 @@ private static void WireBatchNodeDependencies( { var seenExecutionDependencies = new HashSet(); - foreach (var dependencyId in stepDependencies) + foreach (var dependencyId in stepDependencies.Order()) { if (dependencyId == batchEntry.Id) { @@ -1199,7 +1201,7 @@ private static void WireBatchNodeDependencies( private static void WireNodeFieldBranchesAndFallbacks(ExecutionPlanBuildContext ctx) { - foreach (var (nodeId, branches) in ctx.BranchesByNodeId) + foreach (var (nodeId, branches) in ctx.BranchesByNodeId.OrderBy(static t => t.Key)) { if (!ctx.ExecutionNodes.TryGetValue(nodeId, out var entry) || entry is not NodeFieldExecutionNode node) { @@ -1215,7 +1217,7 @@ private static void WireNodeFieldBranchesAndFallbacks(ExecutionPlanBuildContext } } - foreach (var (nodeId, fallbackNodeId) in ctx.FallbackByNodeId) + foreach (var (nodeId, fallbackNodeId) in ctx.FallbackByNodeId.OrderBy(static t => t.Key)) { if (!ctx.ExecutionNodes.TryGetValue(nodeId, out var entry) || entry is not NodeFieldExecutionNode node) { @@ -1267,7 +1269,7 @@ internal static Dictionary CreateBatchingGroupLookup( var dependencyDepthLookup = new Dictionary(); var recursionStack = new HashSet(); - foreach (var serviceSteps in queryStepsByService.Values) + foreach (var (_, serviceSteps) in queryStepsByService.OrderBy(static t => t.Key, StringComparer.Ordinal)) { foreach (var step in serviceSteps) { @@ -1345,7 +1347,7 @@ private static int GetDependencyDepth( var maxDepth = 0; - foreach (var dependency in directDependencies.OrderBy(t => t)) + foreach (var dependency in directDependencies) { var dependencyDepth = GetDependencyDepth( dependency, diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs index cdf9d30ff75..434445998f6 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs @@ -202,7 +202,7 @@ private ImmutableList ApplyDeferRequirementsToParent( // Collect steps that depend directly on the initial requirement step. var downstreamByStepId = new Dictionary(); - foreach (var dependentStepId in selfFetch.Dependents) + foreach (var dependentStepId in selfFetch.Dependents.Order()) { if (incrementalPlanSteps.ById(dependentStepId) is OperationPlanStep dependentStep) { diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/PlanQueue.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/PlanQueue.cs index fcea944fe73..8eeec18f4c5 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/PlanQueue.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/PlanQueue.cs @@ -144,11 +144,9 @@ private void EnqueueLookupPlanNodes( continue; } - if (schema.TryGetBestDirectLookup( - type, - allCandidateSchemas.Remove(toSchema), - toSchema, - out var bestLookup)) + var fromSchemas = GetTransitionSourceSchemas(allCandidateSchemas, toSchema, workItem); + + if (schema.TryGetBestDirectLookup(type, fromSchemas, toSchema, out var bestLookup)) { var lookupWorkItem = workItem with { Lookup = bestLookup }; var branchBacklog = backlog.Push(lookupWorkItem); @@ -244,7 +242,7 @@ private bool TryEnqueueConcreteTypeLookupPlanNodes( } var branchBacklog = backlog; - var fromSchemas = allCandidateSchemas.Remove(toSchema); + var fromSchemas = GetTransitionSourceSchemas(allCandidateSchemas, toSchema, workItem); var allFound = true; // for each concrete type that implements the abstract type, @@ -332,11 +330,10 @@ private bool TryEnqueueConcreteTypeLookupPlanNodes( continue; } - if (schema.TryGetBestDirectLookup( - possibleType, - allCandidateSchemas.Remove(candidateSchema), - candidateSchema, - out var directLookup)) + var fromSchemas = + GetTransitionSourceSchemas(allCandidateSchemas, candidateSchema, workItem); + + if (schema.TryGetBestDirectLookup(possibleType, fromSchemas, candidateSchema, out var directLookup)) { concreteLookup = directLookup; lookupSchema = candidateSchema; @@ -401,6 +398,21 @@ private static double EstimateRemainingCost(PlanNode planNodeTemplate, Backlog b planNodeTemplate.OpsPerLevel, branchBacklog.Cost); + private static ImmutableHashSet GetTransitionSourceSchemas( + ImmutableHashSet candidateSchemas, + string toSchema, + OperationWorkItem workItem) + { + var fromSchemas = candidateSchemas.Remove(toSchema); + + if (!workItem.Dependents.IsEmpty && !string.IsNullOrEmpty(workItem.FromSchema)) + { + fromSchemas = fromSchemas.Remove(workItem.FromSchema); + } + + return fromSchemas; + } + private double GetResolutionCost(SelectionSet selectionSet, string schemaName) { foreach (var (candidateSchema, candidateCost) in schema.GetPossibleSchemas(selectionSet)) diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Steps/NodeFieldPlanStep.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Steps/NodeFieldPlanStep.cs index 14e58fd08b7..28eb888bc70 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Steps/NodeFieldPlanStep.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Steps/NodeFieldPlanStep.cs @@ -1,4 +1,4 @@ -using System.Collections.Immutable; +using HotChocolate.Collections.Immutable; using HotChocolate.Fusion.Execution.Nodes; using HotChocolate.Language; @@ -14,10 +14,10 @@ public record NodeFieldPlanStep : PlanStep public required OperationPlanStep FallbackQuery { get; init; } - public ImmutableDictionary Branches { get; set; } + public ImmutableOrderedDictionary Branches { get; set; } #if NET10_0_OR_GREATER = []; #else - = ImmutableDictionary.Empty; + = ImmutableOrderedDictionary.Empty; #endif } diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionMergeReproTests.cs b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionMergeReproTests.cs new file mode 100644 index 00000000000..54de4b55f04 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionMergeReproTests.cs @@ -0,0 +1,310 @@ +using System.Text.Json; +using HotChocolate.Language; +using HotChocolate.Transport; +using HotChocolate.Transport.Http; + +namespace HotChocolate.Fusion; + +// Repro for customer report: a Relay connection query (`organizations(first: 100, after: $cursor)`) +// against a Fusion v16 gateway intermittently produces a corrupted subgraph request, surfacing as +// HC0011 "Invalid number, expected digit but got: `c`" (the bad byte is borrowed from elsewhere in +// the document, e.g. "cursor"/"endCursor"/"__typename"). It is the signature of an offset/memory +// corruption in the serialized subgraph operation and was reported to be layout/field-order +// sensitive and non-deterministic. +// +// Unlike OrganizationsConnectionReproTests (single passthrough subgraph, exactly one operation), +// this test SPLITS the Organization entity across two subgraphs so that requesting +// `node { id Number displayName }` forces a batched entity lookup to subgraph B for every edge. +// Those structurally-identical lookups exercise the operation MERGING/BATCHING path +// (MergeStructurallyIdenticalOperations) that the single-subgraph repro never touches. +// +// Because the suspected corruption was iteration-order dependent, we build a fresh gateway/plan +// every iteration, run a loop, pass both `cursor: null` and `cursor: "abc"`, and re-parse EVERY +// captured subgraph request body with Utf8GraphQLParser. Any unparseable subgraph request (or any +// gateway error) fails the test. +public class OrganizationsConnectionMergeReproTests : FusionTestBase +{ + private const int Iterations = 25; + + private const string SubgraphA = + """ + interface Node { + id: ID! + } + + type Query { + node(id: ID!): Node @lookup @shareable + organizations(first: Int, after: String): OrganizationConnection + } + + type OrganizationConnection { + pageInfo: PageInfo! + edges: [OrganizationEdge!] + } + + type OrganizationEdge { + cursor: String! + node: Organization! + } + + type PageInfo @shareable { + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String + } + + type Organization implements Node { + id: ID! + displayName: String! + } + """; + + private const string SubgraphB = + """ + interface Node { + id: ID! + } + + type Query { + node(id: ID!): Node @lookup @shareable + organizationByIdFromB(id: ID!): Organization @lookup + } + + type Organization implements Node { + id: ID! + Number: Int! + } + """; + + // Exact customer query. + private const string CustomerQuery = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + id + Number + displayName + __typename + } + """; + + // Customer-reported layout sensitivity: reordering `id` and `Number` "makes it work". + // Exercise the reversed order too, in case the corruption only manifested for one layout. + private const string CustomerQueryReorderedFragment = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + Number + id + displayName + __typename + } + """; + + // Customer note: removing __typename from pageInfo flips the bad char to '_'. + private const string CustomerQueryNoPageInfoTypename = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + id + Number + displayName + __typename + } + """; + + [Fact] + public async Task Organizations_Connection_Split_Entity_Cursor_Null() + => await RunReproLoopAsync(CustomerQuery, cursor: null); + + [Fact] + public async Task Organizations_Connection_Split_Entity_Cursor_NonNull() + => await RunReproLoopAsync(CustomerQuery, cursor: "abc"); + + [Fact] + public async Task Organizations_Connection_Split_Entity_Reordered_Fragment() + => await RunReproLoopAsync(CustomerQueryReorderedFragment, cursor: "abc"); + + [Fact] + public async Task Organizations_Connection_Split_Entity_No_PageInfo_Typename() + => await RunReproLoopAsync(CustomerQueryNoPageInfoTypename, cursor: null); + + private async Task RunReproLoopAsync(string operationDocument, string? cursor) + { + var failures = new List(); + var sawMergedLookups = false; + + for (var iteration = 0; iteration < Iterations; iteration++) + { + // arrange + // fresh subgraphs + gateway every iteration so the planner re-plans + // (the suspected corruption was iteration-order dependent). + using var serverA = CreateSourceSchema("A", SubgraphA); + using var serverB = CreateSourceSchema("B", SubgraphB); + + using var gateway = await CreateCompositeSchemaAsync( + [ + ("A", serverA), + ("B", serverB) + ]); + + using var client = GraphQLHttpClient.Create(gateway.CreateClient()); + + var request = new OperationRequest( + operationDocument, + variables: new Dictionary + { + ["cursor"] = cursor + }); + + // act + using var response = await client.PostAsync( + request, + new Uri("http://localhost:5000/graphql")); + + using var result = await response.ReadAsResultAsync(); + + // assert + // 1. the gateway result must not contain errors (HC0011 would show up here). + if (result.Errors.ValueKind is JsonValueKind.Array + && result.Errors.GetArrayLength() > 0) + { + failures.Add( + $"iteration {iteration} (cursor: {Describe(cursor)}): gateway returned errors: " + + result.Errors.GetRawText()); + } + + // 2. every captured subgraph request body must re-parse cleanly. + // a corrupted `first: 100` (or any mangled literal) surfaces as a SyntaxException here. + var lookupRequestCount = 0; + + foreach (var (schemaName, schemaInteractions) in gateway.Interactions) + { + foreach (var interaction in schemaInteractions.Values) + { + if (interaction.Request is not { } rawRequest) + { + continue; + } + + rawRequest.Body.Position = 0; + using var json = JsonDocument.Parse(rawRequest.Body); + + foreach (var query in EnumerateQueries(json.RootElement)) + { + if (schemaName == "B" && query.Contains("Number")) + { + lookupRequestCount++; + } + + try + { + _ = Utf8GraphQLParser.Parse(query); + } + catch (Exception ex) + { + failures.Add( + $"iteration {iteration} (cursor: {Describe(cursor)}): subgraph '{schemaName}' " + + $"request did not re-parse: {ex.Message}\n----\n{query}\n----"); + } + } + } + } + + // a single batched lookup operation to B that resolves several entities (one merged + // operation, multiple variable sets) still counts as exercising the merge/batch path. + if (lookupRequestCount > 0) + { + sawMergedLookups = true; + } + } + + // The merge/batch path must actually have been exercised, otherwise a clean run proves nothing. + Assert.True( + sawMergedLookups, + "Expected at least one entity lookup to subgraph 'B' (proving the split-entity merge/batch " + + "path was exercised), but none were captured. The composition or query is not forcing lookups."); + + if (failures.Count > 0) + { + Assert.Fail( + $"Reproduced corrupted/failing subgraph requests in {failures.Count} of {Iterations} iterations:\n\n" + + string.Join("\n\n", failures)); + } + } + + private static IEnumerable EnumerateQueries(JsonElement root) + { + if (root.ValueKind is JsonValueKind.Array) + { + foreach (var item in root.EnumerateArray()) + { + if (item.TryGetProperty("query", out var batchQuery) + && batchQuery.ValueKind is JsonValueKind.String) + { + yield return batchQuery.GetString()!; + } + } + + yield break; + } + + if (root.ValueKind is JsonValueKind.Object + && root.TryGetProperty("query", out var query) + && query.ValueKind is JsonValueKind.String) + { + yield return query.GetString()!; + } + } + + private static string Describe(string? cursor) + => cursor is null ? "null" : $"\"{cursor}\""; +} diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionReproTests.cs b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionReproTests.cs new file mode 100644 index 00000000000..872ff9ccda8 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsConnectionReproTests.cs @@ -0,0 +1,89 @@ +using HotChocolate.Transport; +using HotChocolate.Transport.Http; + +namespace HotChocolate.Fusion; + +public class OrganizationsConnectionReproTests : FusionTestBase +{ + // Repro for customer report: a Relay connection query with `first: 100` against a + // Fusion gateway produces a corrupted subgraph request, causing the subgraph to fail + // parsing with "Invalid number, expected digit but got: `c`" (HC0011). + // MatchSnapshotAsync re-parses every captured subgraph request body, so a corrupted + // `first: 100` literal surfaces as a SyntaxException there. + [Fact] + public async Task Organizations_Connection_With_Fragment_And_Typename() + { + // arrange + using var server1 = CreateSourceSchema( + "A", + """ + type Query { + organizations(first: Int, after: String): OrganizationConnection + } + + type OrganizationConnection { + pageInfo: PageInfo! + edges: [OrganizationEdge!] + } + + type OrganizationEdge { + cursor: String! + node: Organization! + } + + type PageInfo { + hasNextPage: Boolean! + endCursor: String + } + + type Organization { + id: ID! + Number: Int! + displayName: String! + } + """); + + using var gateway = await CreateCompositeSchemaAsync( + [ + ("A", server1) + ]); + + // act + using var client = GraphQLHttpClient.Create(gateway.CreateClient()); + + var request = new OperationRequest( + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + id + Number + displayName + __typename + } + """); + + using var result = await client.PostAsync( + request, + new Uri("http://localhost:5000/graphql"), + TestContext.Current.CancellationToken); + + // assert + await MatchSnapshotAsync(gateway, request, result); + } +} diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsNodeFanoutReproTests.cs b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsNodeFanoutReproTests.cs new file mode 100644 index 00000000000..99671c6edee --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/OrganizationsNodeFanoutReproTests.cs @@ -0,0 +1,286 @@ +using System.Text.Json; +using HotChocolate.Language; +using HotChocolate.Transport; +using HotChocolate.Transport.Http; + +namespace HotChocolate.Fusion; + +// Repro for customer report: HC0011 "Invalid number, expected digit but got: `c`" for a Relay +// connection query, the signature of an offset/memory corruption in the serialized subgraph +// operation, layout/field-order sensitive and non-deterministic. +// +// This variant targets the Relay `node(id:): Node @lookup @shareable` fan-out path +// (OperationPlanner.PlanNode -> NodeFieldPlanStep), whose `Branches` collection was changed from +// ImmutableDictionary to ImmutableOrderedDictionary in the "Planner Stabilizations" commit +// (non-deterministic -> deterministic iteration). +// +// The Organization entity is split across two subgraphs and has NO dedicated by-id lookup, so the +// only way to resolve a cross-subgraph field (`Number` from B) for an Organization is the shared +// `node`/`nodes` fan-out. Both a direct `node(id:)` query (which definitively creates a +// NodeFieldPlanStep with multiple branches) and the customer's connection query are exercised. +// +// Each iteration freshly plans/executes, passes `$cursor` (null and a value), and re-parses every +// captured subgraph request body with Utf8GraphQLParser. Any unparseable subgraph request, any +// corrupted `first: 100`, or any gateway error fails the test. +public class OrganizationsNodeFanoutReproTests : FusionTestBase +{ + private const int Iterations = 30; + + // Subgraph A owns the connection and Organization.displayName. It exposes ONLY the shared node + // lookup (no organizationById), so cross-schema resolution must go through node/nodes. + private const string SubgraphA = + """ + interface Node { + id: ID! + } + + type Query { + node(id: ID!): Node @lookup @shareable + nodes(ids: [ID!]!): [Node]! @shareable + organizations(first: Int, after: String): OrganizationConnection + } + + type OrganizationConnection { + pageInfo: PageInfo! + edges: [OrganizationEdge!] + } + + type OrganizationEdge { + cursor: String! + node: Organization! + } + + type PageInfo @shareable { + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String + } + + type Organization implements Node { + id: ID! + displayName: String! + } + """; + + // Subgraph B owns Organization.Number. It exposes ONLY the shared node lookup (no + // organizationById), so resolving Number for an Organization from A must fan out through node. + private const string SubgraphB = + """ + interface Node { + id: ID! + } + + type Query { + node(id: ID!): Node @lookup @shareable + nodes(ids: [ID!]!): [Node]! @shareable + } + + type Organization implements Node { + id: ID! + Number: Int! + } + """; + + // Exact customer connection query. + private const string ConnectionQuery = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + id + Number + displayName + __typename + } + """; + + // Direct node(id:) query selecting the split Organization. This is the canonical trigger for + // OperationPlanner.PlanNode -> NodeFieldPlanStep with multiple branches. The id is an inline + // literal (a String variable is not assignable to the ID! node argument). + private const string NodeQuery = + """ + query OrganizationNode { + node(id: "T3JnYW5pemF0aW9uOjE=") { + ... on Organization { + id + Number + displayName + __typename + } + __typename + } + } + """; + + [Fact] + public async Task Connection_NodeFanout_Cursor_Null() + => await RunReproLoopAsync(ConnectionQuery, cursor: null, expectNodeFanout: true); + + [Fact] + public async Task Connection_NodeFanout_Cursor_NonNull() + => await RunReproLoopAsync(ConnectionQuery, cursor: "abc", expectNodeFanout: true); + + [Fact] + public async Task Direct_Node_Fanout() + => await RunReproLoopAsync(NodeQuery, cursor: null, expectNodeFanout: true); + + private async Task RunReproLoopAsync(string operationDocument, string? cursor, bool expectNodeFanout) + { + var failures = new List(); + var sawNodeFanout = false; + var sawNumberFromB = false; + + for (var iteration = 0; iteration < Iterations; iteration++) + { + // arrange + // fresh subgraphs + gateway every iteration so the planner re-plans. + using var serverA = CreateSourceSchema("A", SubgraphA); + using var serverB = CreateSourceSchema("B", SubgraphB); + + using var gateway = await CreateCompositeSchemaAsync( + [ + ("A", serverA), + ("B", serverB) + ]); + + using var client = GraphQLHttpClient.Create(gateway.CreateClient()); + + var request = new OperationRequest( + operationDocument, + variables: new Dictionary + { + ["cursor"] = cursor + }); + + // act + using var response = await client.PostAsync( + request, + new Uri("http://localhost:5000/graphql")); + + using var result = await response.ReadAsResultAsync(); + + // assert + // 1. the gateway result must not contain errors (HC0011 would show up here). + if (result.Errors.ValueKind is JsonValueKind.Array + && result.Errors.GetArrayLength() > 0) + { + failures.Add( + $"iteration {iteration} (cursor: {Describe(cursor)}): gateway returned errors: " + + result.Errors.GetRawText()); + } + + // 2. every captured subgraph request body must re-parse cleanly. + // a corrupted `first: 100` (or any mangled literal) surfaces as a SyntaxException here. + foreach (var (schemaName, schemaInteractions) in gateway.Interactions) + { + foreach (var interaction in schemaInteractions.Values) + { + if (interaction.Request is not { } rawRequest) + { + continue; + } + + rawRequest.Body.Position = 0; + using var json = JsonDocument.Parse(rawRequest.Body); + + foreach (var query in EnumerateQueries(json.RootElement)) + { + // node/nodes appearing in a subgraph request proves the node fan-out path. + if (ContainsNodeField(query)) + { + sawNodeFanout = true; + } + + if (schemaName == "B" && query.Contains("Number")) + { + sawNumberFromB = true; + } + + try + { + _ = Utf8GraphQLParser.Parse(query); + } + catch (Exception ex) + { + failures.Add( + $"iteration {iteration} (cursor: {Describe(cursor)}): subgraph '{schemaName}' " + + $"request did not re-parse: {ex.Message}\n----\n{query}\n----"); + } + } + } + } + } + + if (expectNodeFanout) + { + // Prove the node(id:)/nodes fan-out was actually exercised, otherwise a clean run proves nothing. + Assert.True( + sawNodeFanout, + "Expected at least one subgraph request to use the Relay `node`/`nodes` field (proving the " + + "NodeFieldPlanStep fan-out path was exercised), but none were captured."); + + Assert.True( + sawNumberFromB, + "Expected at least one fan-out request to subgraph 'B' resolving `Number` (proving the split " + + "entity was resolved cross-schema via node), but none were captured."); + } + + if (failures.Count > 0) + { + Assert.Fail( + $"Reproduced corrupted/failing subgraph requests in {failures.Count} of {Iterations} iterations:\n\n" + + string.Join("\n\n", failures)); + } + } + + private static bool ContainsNodeField(string query) + { + // crude but sufficient: the fan-out emits `node(id:` or `nodes(ids:` at the root. + return query.Contains("node(id:") + || query.Contains("nodes(ids:") + || query.Contains("node(") && query.Contains("on Organization"); + } + + private static IEnumerable EnumerateQueries(JsonElement root) + { + if (root.ValueKind is JsonValueKind.Array) + { + foreach (var item in root.EnumerateArray()) + { + if (item.TryGetProperty("query", out var batchQuery) + && batchQuery.ValueKind is JsonValueKind.String) + { + yield return batchQuery.GetString()!; + } + } + + yield break; + } + + if (root.ValueKind is JsonValueKind.Object + && root.TryGetProperty("query", out var query) + && query.ValueKind is JsonValueKind.String) + { + yield return query.GetString()!; + } + } + + private static string Describe(string? cursor) + => cursor is null ? "null" : $"\"{cursor}\""; +} diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/OrganizationsConnectionReproTests.Organizations_Connection_With_Fragment_And_Typename.yaml b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/OrganizationsConnectionReproTests.Organizations_Connection_With_Fragment_And_Typename.yaml new file mode 100644 index 00000000000..db789ceaba4 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/OrganizationsConnectionReproTests.Organizations_Connection_With_Fragment_And_Typename.yaml @@ -0,0 +1,222 @@ +title: Organizations_Connection_With_Fragment_And_Typename +request: + document: | + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + + fragment Organization on Organization { + id + Number + displayName + __typename + } +response: + body: | + { + "data": { + "organizations": { + "pageInfo": { + "hasNextPage": true, + "endCursor": "PageInfo: UGFnZUluZm86Mg==", + "__typename": "PageInfo" + }, + "edges": [ + { + "node": { + "id": "T3JnYW5pemF0aW9uOjY=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjY=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + }, + { + "node": { + "id": "T3JnYW5pemF0aW9uOjc=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjc=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + }, + { + "node": { + "id": "T3JnYW5pemF0aW9uOjg=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjg=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + } + ], + "__typename": "OrganizationConnection" + } + } + } +sourceSchemas: + - name: A + schema: | + schema { + query: Query + } + + type Query { + organizations(first: Int, after: String): OrganizationConnection + } + + type Organization { + id: ID! + Number: Int! + displayName: String! + } + + type OrganizationConnection { + pageInfo: PageInfo! + edges: [OrganizationEdge!] + } + + type OrganizationEdge { + cursor: String! + node: Organization! + } + + type PageInfo { + hasNextPage: Boolean! + endCursor: String + } + interactions: + - request: + accept: application/graphql-response+json; charset=utf-8, application/json; charset=utf-8, application/jsonl; charset=utf-8, text/event-stream; charset=utf-8 + document: | + query Organizations_680a78e9_1($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + id + Number + displayName + __typename + } + __typename + } + __typename + } + } + variables: | + {} + response: + results: + - | + { + "data": { + "organizations": { + "pageInfo": { + "hasNextPage": true, + "endCursor": "PageInfo: UGFnZUluZm86Mg==", + "__typename": "PageInfo" + }, + "edges": [ + { + "node": { + "id": "T3JnYW5pemF0aW9uOjY=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjY=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + }, + { + "node": { + "id": "T3JnYW5pemF0aW9uOjc=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjc=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + }, + { + "node": { + "id": "T3JnYW5pemF0aW9uOjg=", + "Number": 123, + "displayName": "Organization: T3JnYW5pemF0aW9uOjg=", + "__typename": "Organization" + }, + "__typename": "OrganizationEdge" + } + ], + "__typename": "OrganizationConnection" + } + } + } +operationPlan: + operation: + - document: | + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + id + Number + displayName + __typename + } + __typename + } + __typename + } + } + name: Organizations + hash: 680a78e9d72d8e2a033d38e6d6df4025 + searchSpace: 1 + expandedNodes: 1 + nodes: + - id: 1 + type: Operation + schema: A + operation: | + query Organizations_680a78e9_1($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + id + Number + displayName + __typename + } + __typename + } + __typename + } + } + forwardedVariables: + - cursor diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/OperationPlannerNumberLiteralReproTests.cs b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/OperationPlannerNumberLiteralReproTests.cs new file mode 100644 index 00000000000..a7337c05e9b --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/OperationPlannerNumberLiteralReproTests.cs @@ -0,0 +1,63 @@ +using HotChocolate.Fusion.Execution.Nodes; +using HotChocolate.Language; + +namespace HotChocolate.Fusion.Planning; + +public class OperationPlannerNumberLiteralReproTests : FusionTestBase +{ + // Repro for customer report: a Relay connection query with `first: 100` produces a + // subgraph operation whose serialized source text is corrupted, e.g. the `100` literal + // becomes a non-digit character, causing the subgraph parse to fail with + // "Invalid number, expected digit but got: `c`" (HC0011). + [Fact] + public void Plan_Users_Connection_SubgraphOperations_AreValidGraphQL() + { + // arrange + var schema = ComposeShoppingSchema(); + + // act + var plan = PlanOperation( + schema, + """ + query Users($cursor: String) { + users(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...UserFields + __typename + } + __typename + } + __typename + } + } + fragment UserFields on User { + id + name + username + __typename + } + """); + + // assert + var executionNodes = plan.AllNodes.OfType().ToList(); + Assert.NotEmpty(executionNodes); + + foreach (var node in executionNodes) + { + var sourceText = node.Operation.SourceText; + + // Re-parse each subgraph operation. The bug corrupts the `first: 100` literal, + // making this throw SyntaxException ("Invalid number, expected digit but got ..."). + var ex = Record.Exception(() => Utf8GraphQLParser.Parse(sourceText)); + Assert.True( + ex is null, + $"Subgraph operation did not parse:\n{sourceText}\n\nError: {ex?.Message}"); + } + } +} diff --git a/src/HotChocolate/Language/test/Language.Tests/Parser/MultiSegmentNumberReproTests.cs b/src/HotChocolate/Language/test/Language.Tests/Parser/MultiSegmentNumberReproTests.cs new file mode 100644 index 00000000000..e935e6ba4aa --- /dev/null +++ b/src/HotChocolate/Language/test/Language.Tests/Parser/MultiSegmentNumberReproTests.cs @@ -0,0 +1,74 @@ +using System.Buffers; +using System.Text; + +namespace HotChocolate.Language; + +public class MultiSegmentNumberReproTests +{ + private const string CustomerQuery = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + + fragment Organization on Organization { + id + Number + displayName + __typename + } + """; + + [Fact] + public void Parse_CustomerQuery_AllChunkSizes_DoNotCorruptNumber() + { + // arrange + var data = Encoding.UTF8.GetBytes(CustomerQuery); + var failures = new List(); + + // act + for (var chunkSize = 1; chunkSize <= data.Length; chunkSize++) + { + var sequence = TestSequenceSegment.CreateMultiSegment(data, chunkSize); + + try + { + var reader = new Utf8GraphQLReader(sequence); + while (reader.Read()) + { + if (reader.Kind == TokenKind.Integer) + { + var value = Encoding.UTF8.GetString(reader.Value); + if (value != "100") + { + failures.Add($"chunkSize={chunkSize}: integer token = '{value}' (expected '100')"); + } + } + } + } + catch (SyntaxException ex) + { + failures.Add($"chunkSize={chunkSize}: {ex.Message}"); + } + } + + // assert + Assert.True( + failures.Count == 0, + "Multi-segment reader corrupted the number token:\n" + string.Join("\n", failures)); + } +} diff --git a/src/HotChocolate/Language/test/Language.Tests/Parser/UnescapeNumberReproTests.cs b/src/HotChocolate/Language/test/Language.Tests/Parser/UnescapeNumberReproTests.cs new file mode 100644 index 00000000000..9ac47eacda8 --- /dev/null +++ b/src/HotChocolate/Language/test/Language.Tests/Parser/UnescapeNumberReproTests.cs @@ -0,0 +1,171 @@ +using System.Text; + +namespace HotChocolate.Language; + +public class UnescapeNumberReproTests +{ + // JSON-escapes a byte sequence the way a GraphQL-over-HTTP client encodes the + // "query" string (newline -> \n, quote -> \", backslash -> \\). + private static byte[] JsonEscape(byte[] target) + { + var sb = new List(target.Length * 2); + foreach (var b in target) + { + switch (b) + { + case (byte)'\n': + sb.Add((byte)'\\'); + sb.Add((byte)'n'); + break; + case (byte)'\r': + sb.Add((byte)'\\'); + sb.Add((byte)'r'); + break; + case (byte)'\t': + sb.Add((byte)'\\'); + sb.Add((byte)'t'); + break; + case (byte)'"': + sb.Add((byte)'\\'); + sb.Add((byte)'"'); + break; + case (byte)'\\': + sb.Add((byte)'\\'); + sb.Add((byte)'\\'); + break; + default: + sb.Add(b); + break; + } + } + + return sb.ToArray(); + } + + private static byte[] Unescape(byte[] escaped) + { + var buffer = new byte[escaped.Length]; + var input = new ReadOnlySpan(escaped); + var output = new Span(buffer); + Utf8Helper.Unescape(in input, ref output, isBlockString: false); + return output.ToArray(); + } + + [Fact] + public void Unescape_SingleNewline_AtEveryPosition_AndLength_RoundTrips() + { + // arrange / act / assert + var failures = new List(); + + // cross the SIMD 16/32-byte boundaries + for (var length = 1; length <= 140; length++) + { + for (var newlinePos = 0; newlinePos < length; newlinePos++) + { + var target = new byte[length]; + for (var i = 0; i < length; i++) + { + // printable filler; newline at the chosen position + target[i] = i == newlinePos ? (byte)'\n' : (byte)('a' + (i % 26)); + } + + var escaped = JsonEscape(target); + var actual = Unescape(escaped); + + if (!actual.AsSpan().SequenceEqual(target)) + { + failures.Add( + $"length={length} newlinePos={newlinePos}: " + + $"expected '{Encoding.UTF8.GetString(target)}' " + + $"got '{Encoding.UTF8.GetString(actual)}'"); + } + } + } + + Assert.True(failures.Count == 0, string.Join("\n", failures.Take(10))); + } + + [Fact] + public void Unescape_ManyEscapes_RandomLayouts_RoundTrip() + { + // arrange + // deterministic LCG so failures are reproducible + ulong state = 0x1234_5678_9abc_def0; + int Next(int maxExclusive) + { + state = state * 6364136223846793005UL + 1442695040888963407UL; + return (int)((state >> 33) % (ulong)maxExclusive); + } + + var escapeTargets = new byte[] { (byte)'\n', (byte)'\r', (byte)'\t', (byte)'"', (byte)'\\' }; + var failures = new List(); + + // act / assert + for (var iteration = 0; iteration < 20000 && failures.Count == 0; iteration++) + { + var length = 1 + Next(160); + var target = new byte[length]; + for (var i = 0; i < length; i++) + { + // ~35% chance of an escapable byte, clustering escapes + target[i] = Next(100) < 35 + ? escapeTargets[Next(escapeTargets.Length)] + : (byte)('a' + Next(26)); + } + + var escaped = JsonEscape(target); + var actual = Unescape(escaped); + + if (!actual.AsSpan().SequenceEqual(target)) + { + failures.Add( + $"iteration={iteration} length={length}\n" + + $" target =[{string.Join(",", target.Select(b => (int)b))}]\n" + + $" actual =[{string.Join(",", actual.Select(b => (int)b))}]"); + } + } + + Assert.True(failures.Count == 0, string.Join("\n", failures)); + } + + [Fact] + public void Unescape_CustomerQuery_RoundTrips() + { + // arrange + const string query = + """ + query Organizations($cursor: String) { + organizations(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + __typename + } + edges { + node { + ...Organization + __typename + } + __typename + } + __typename + } + } + fragment Organization on Organization { + id + Number + displayName + __typename + } + """; + + var target = Encoding.UTF8.GetBytes(query); + var escaped = JsonEscape(target); + + // act + var actual = Unescape(escaped); + + // assert + Assert.Equal(query, Encoding.UTF8.GetString(actual)); + } +}