diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index ec961106..19496213 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -327,3 +327,10 @@ * Fixed validation of an inline fragment without a type condition (`... { … }`), which used to fail with an exception instead of applying to the parent type * Fixed the GraphQL client provider building `Operation<...>` again for every file that uses the same operation, which added another operation type of the same name to `Operations` each time * Removed the internal `Observable.withCompletionMarker` +* **Breaking Change** Validation now rejects a document, before building anything else for it, when validating it would inline more than `DocumentLimitsDefaults.MaxRecursiveSelections` (25 000) selections or nest deeper than `DocumentLimitsDefaults.MaxNestingDepth` (128) levels. Selections are fields, inline fragments and fragment spreads, counted over all the operations and fragment definitions of the document with their fragment spreads inlined, and each inline fragment and fragment spread adds a nesting level. Validation also stops after `DocumentLimitsDefaults.MaxValidationErrors` (100) errors and then adds a final "Too many validation errors, error limit reached. Validation aborted." error, as graphql-js does +* **Breaking Change** Validation no longer inlines the spreads of fragments that form a cycle, so it reports the cycle but not the other errors inside those fragments' spreads. The fragment cycle rule now reports each fragment of a cycle once, including fragments whose type condition is not in the schema, and no longer reports fragments that only spread a cycle +* Fixed a stack overflow that crashed the whole server when a document contained a subscription operation spreading a fragment that spreads itself, even when that operation was not the one executed and the schema had no subscription type +* Fixed validation time growing exponentially with fragments that spread other fragments several times, and factorially with fragments that spread each other in a cycle: a document of 20 such fragments and 821 characters took 31 seconds to validate +* Fixed validation error accumulation, which was quadratic in the number of errors +* Fixed the subscription single root field rule ignoring the fields selected before a fragment spread and counting a fragment spread twice when it is spread twice +* Added `AstError.Create`, which creates a validation error diff --git a/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs new file mode 100644 index 00000000..27aa4f35 --- /dev/null +++ b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs @@ -0,0 +1,32 @@ +namespace FSharp.Data.GraphQL + +/// Default limits on the work that an untrusted document can cause while it is validated. +[] +module DocumentLimitsDefaults = + + /// + /// The maximum nesting depth of a document once its fragment spreads are inlined. + /// + /// Every selection set of a field, every inline fragment and every fragment spread adds one level. + /// + /// + [] + let MaxNestingDepth = 128 + + /// + /// The maximum number of selections that the validation of a document may inline. + /// + /// Fields, inline fragments and fragment spreads are counted after fragment spreads are inlined, over all the + /// operations and fragment definitions of the document. + /// + /// + /// Validation and planning take time in proportion to this number, so a higher limit lets a small document + /// keep a server busy for longer. + /// + /// + [] + let MaxRecursiveSelections = 25_000 + + /// The maximum number of errors that the validation of a document reports before it stops. + [] + let MaxValidationErrors = 100 diff --git a/src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj b/src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj index 3dc6ee7b..d253d6f0 100644 --- a/src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj +++ b/src/FSharp.Data.GraphQL.Shared/FSharp.Data.GraphQL.Shared.fsproj @@ -44,6 +44,7 @@ + diff --git a/src/FSharp.Data.GraphQL.Shared/Validation.fs b/src/FSharp.Data.GraphQL.Shared/Validation.fs index dc1a41d0..e52fb758 100644 --- a/src/FSharp.Data.GraphQL.Shared/Validation.fs +++ b/src/FSharp.Data.GraphQL.Shared/Validation.fs @@ -3,6 +3,7 @@ namespace FSharp.Data.GraphQL.Validation +open System open System.Collections.Generic open FSharp.Data.GraphQL open FSharp.Data.GraphQL.Ast @@ -444,9 +445,214 @@ module Ast = | FragmentDefinition x when x.Name.IsSome -> ValueSome x | _ -> ValueNone) + /// The named fragments of the document by name. As in fragment spread resolution, the first definition of a name wins. + let private getFragmentsByName (fragmentDefinitions : FragmentDefinition list) = + let fragments = Dictionary (StringComparer.Ordinal) + for fragment in fragmentDefinitions do + if not (fragments.ContainsKey fragment.Name.Value) then + fragments.Add (fragment.Name.Value, fragment) + fragments + + /// The size of a selection set before its fragment spreads are inlined. + [] + type private SelectionSetShape = { + /// The number of fields, inline fragments and fragment spreads, nested ones included. + Selections : int64 + /// The deepest nesting level; the selection set itself is level 1, and each field selection set, inline fragment and fragment spread adds one. + Depth : int + /// The fragment spreads with the level of the selection set that contains each of them. + Spreads : struct (string * int)[] + } + + // Iterative, so a deeply nested document cannot overflow the stack here + let private getSelectionSetShape (root : Selection list) = + let mutable selections = 0L + let mutable depth = 0 + let spreads = ResizeArray () + let pending = Stack () + pending.Push (struct (root, 1)) + while pending.Count > 0 do + let struct (selectionSet, level) = pending.Pop () + if not selectionSet.IsEmpty && level > depth then + depth <- level + for selection in selectionSet do + selections <- selections + 1L + match selection with + | Field field when not field.SelectionSet.IsEmpty -> pending.Push (struct (field.SelectionSet, level + 1)) + | Field _ -> () + | InlineFragment fragment -> pending.Push (struct (fragment.SelectionSet, level + 1)) + | FragmentSpread spread -> spreads.Add (struct (spread.Name, level)) + { Selections = selections; Depth = depth; Spreads = spreads.ToArray () } + + let private getFragmentShapes (fragments : Dictionary) = + let shapes = Dictionary (fragments.Count, StringComparer.Ordinal) + for KeyValue (name, fragment) in fragments do + shapes.Add (name, getSelectionSetShape fragment.SelectionSet) + shapes + + /// + /// The names of the fragments that are part of a fragment spread cycle, including fragments that spread themselves. + /// + /// + /// Iterative Tarjan's strongly connected components algorithm, linear in the number of fragment spreads. + /// + let private findCyclicFragments (shapes : Dictionary) = + let targets = Dictionary (shapes.Count, StringComparer.Ordinal) + for KeyValue (name, shape) in shapes do + targets.Add ( + name, + shape.Spreads + |> Seq.map (fun struct (target, _) -> target) + |> Seq.filter shapes.ContainsKey + |> Seq.distinct + |> Seq.toArray + ) + let indexes = Dictionary (shapes.Count, StringComparer.Ordinal) + let lowLinks = Dictionary (shapes.Count, StringComparer.Ordinal) + let onStack = HashSet (StringComparer.Ordinal) + let componentStack = Stack () + let cyclic = HashSet (StringComparer.Ordinal) + let mutable nextIndex = 0 + let visit (name : string) = + indexes.Add (name, nextIndex) + lowLinks.Add (name, nextIndex) + nextIndex <- nextIndex + 1 + componentStack.Push name + onStack.Add name |> ignore + for root in shapes.Keys do + if not (indexes.ContainsKey root) then + // Each entry is a fragment and the position of the next spread target to explore + let work = Stack () + visit root + work.Push (struct (root, 0)) + while work.Count > 0 do + let struct (name, position) = work.Pop () + let nameTargets = targets[name] + if position < nameTargets.Length then + work.Push (struct (name, position + 1)) + let target = nameTargets[position] + if not (indexes.ContainsKey target) then + visit target + work.Push (struct (target, 0)) + elif onStack.Contains target then + lowLinks[name] <- min (lowLinks[name]) (indexes[target]) + else + if work.Count > 0 then + let struct (parent, _) = work.Peek () + lowLinks[parent] <- min (lowLinks[parent]) (lowLinks[name]) + if lowLinks[name] = indexes[name] then + // The fragment is the root of a strongly connected component: pop the whole component + let componentNames = ResizeArray () + let mutable isRoot = false + while not isRoot do + let componentName = componentStack.Pop () + onStack.Remove componentName |> ignore + componentNames.Add componentName + isRoot <- String.Equals (componentName, name, StringComparison.Ordinal) + if componentNames.Count > 1 || Array.contains name nameTargets then + for componentName in componentNames do + cyclic.Add componentName |> ignore + cyclic + + /// + /// The named fragments whose spreads are inlined while the document is validated: all of them except the ones that form a cycle. + /// + /// + /// Following the spreads of a cycle inlines every simple path through it, which grows factorially with the size of the cycle. + /// The cycles themselves are reported by . + /// + let private getInlinableFragmentDefinitions (fragmentDefinitions : FragmentDefinition list) = + let cyclic = + fragmentDefinitions + |> getFragmentsByName + |> getFragmentShapes + |> findCyclicFragments + if cyclic.Count = 0 then + fragmentDefinitions + else + fragmentDefinitions + |> List.filter (fun fragment -> not (cyclic.Contains fragment.Name.Value)) + + /// + /// The number of selections and the nesting depth that validating the document inlines, saturated at . + /// + /// + /// + /// Every operation and fragment definition is counted with its fragment spreads inlined, as + /// builds them. Spreads of fragments that form a cycle are not inlined. + /// + /// + /// The count of each fragment is computed once (as in Apollo Server's RecursiveSelectionsLimit rule), + /// and the traversal is iterative, so neither an exponential fragment bomb nor a long chain of fragments can stall + /// or overflow it. + /// + /// + let internal measureDocument (selectionCap : int64) (ast : Document) : struct (int64 * int) = + let shapes = + getFragmentDefinitions ast + |> getFragmentsByName + |> getFragmentShapes + let cyclic = findCyclicFragments shapes + let isInlined name = shapes.ContainsKey name && not (cyclic.Contains name) + let inlined = Dictionary (shapes.Count, StringComparer.Ordinal) + // Requires the inlined size of every inlined spread target to be known already + let inlineSpreads (shape : SelectionSetShape) = + let mutable selections = shape.Selections + let mutable depth = shape.Depth + for spread in shape.Spreads do + let struct (target, level) = spread + if isInlined target then + let struct (targetSelections, targetDepth) = inlined[target] + selections <- min selectionCap (selections + targetSelections) + depth <- max depth (level + targetDepth) + struct (selections, depth) + let pushMissingTargets (pending : Stack) (shape : SelectionSetShape) = + let mutable pushed = false + for spread in shape.Spreads do + let struct (target, _) = spread + if isInlined target && not (inlined.ContainsKey target) then + pending.Push target + pushed <- true + pushed + let measure (shape : SelectionSetShape) = + // Post-order over the spread targets, which form no cycle, so this terminates + let pending = Stack () + pushMissingTargets pending shape |> ignore + while pending.Count > 0 do + let name = pending.Peek () + if inlined.ContainsKey name then + pending.Pop () |> ignore + else + let nameShape = shapes[name] + if not (pushMissingTargets pending nameShape) then + pending.Pop () |> ignore + inlined.Add (name, inlineSpreads nameShape) + inlineSpreads shape + let mutable selections = 0L + let mutable depth = 0 + for definition in ast.Definitions do + let struct (definitionSelections, definitionDepth) = measure (getSelectionSetShape definition.SelectionSet) + selections <- min selectionCap (selections + definitionSelections) + depth <- max depth definitionDepth + struct (selections, depth) + + /// + /// Rejects a document whose validation would inline more selections or nest deeper than allowed, + /// before any validation context is built for it. + /// + let internal checkDocumentLimits (maxRecursiveSelections : int) (maxNestingDepth : int) (ast : Document) = + let struct (selections, depth) = measureDocument (int64 maxRecursiveSelections + 1L) ast + if selections > int64 maxRecursiveSelections then + AstError.AsResult $"The document recursively requests too many selections (more than %i{maxRecursiveSelections})." + elif depth > maxNestingDepth then + AstError.AsResult $"The document is nested too deeply once fragment spreads are inlined (more than %i{maxNestingDepth} levels)." + else + Success + /// Prepare a ValidationContext for the given Document and SchemaInfo to make validation operations easier. let internal getValidationContext (schemaInfo : SchemaInfo) (ast : Document) = let fragmentDefinitions = getFragmentDefinitions ast + let inlinableFragmentDefinitions = getInlinableFragmentDefinitions fragmentDefinitions let fragmentInfos = fragmentDefinitions |> List.vchoose (fun def -> voption { @@ -454,7 +660,7 @@ module Ast = let! fragType = schemaInfo.TryGetTypeByName typeCondition let fragCtx = { Schema = schemaInfo - FragmentDefinitions = fragmentDefinitions + FragmentDefinitions = inlinableFragmentDefinitions ParentType = fragType FragmentType = ValueSome (Spread (def.Name.Value, def.Directives, fragType)) Path = [ def.Name.Value ] @@ -469,7 +675,7 @@ module Ast = let path = def.Name |> ValueOption.map box |> ValueOption.toList let opCtx = { Schema = schemaInfo - FragmentDefinitions = fragmentDefinitions + FragmentDefinitions = inlinableFragmentDefinitions ParentType = parentType FragmentType = ValueNone Path = path @@ -506,21 +712,27 @@ module Ast = "An anonymous operation must be the only operation in a document. This document has at least one anonymous operation and more than one operation." let internal validateSubscriptionSingleRootField (ctx : ValidationContext) = - let fragmentDefinitions = getFragmentDefinitions ctx.Document - let rec getFieldNames (selectionSet : Selection list) = - ([], selectionSet) - ||> List.fold (fun acc -> - function - | Field field -> field.AliasOrName :: acc - | InlineFragment frag -> List.append (getFieldNames frag.SelectionSet) acc - | FragmentSpread spread -> - fragmentDefinitions - |> List.vtryFind (fun x -> x.Name.IsSome && x.Name.Value = spread.Name) - |> ValueOption.unwrap acc (fun frag -> getFieldNames frag.SelectionSet)) + let fragments = + getFragmentDefinitions ctx.Document + |> getFragmentsByName ctx.Document.Definitions |> ValidationResult.collect (function | OperationDefinition def when def.OperationType = Subscription -> - let fieldNames = getFieldNames def.SelectionSet + // As in CollectFields, each fragment is collected once per operation, + // which also stops on fragment spread cycles + let visitedFragments = HashSet (StringComparer.Ordinal) + let rec getFieldNames (names : string list) (selectionSet : Selection list) = + (names, selectionSet) + ||> List.fold (fun acc -> + function + | Field field -> field.AliasOrName :: acc + | InlineFragment frag -> getFieldNames acc frag.SelectionSet + | FragmentSpread spread when visitedFragments.Add spread.Name -> + match fragments.TryGetValue spread.Name with + | true, frag -> getFieldNames acc frag.SelectionSet + | false, _ -> acc + | FragmentSpread _ -> acc) + let fieldNames = getFieldNames [] def.SelectionSet if fieldNames.Length <= 1 then Success else @@ -899,47 +1111,22 @@ module Ast = odef.SelectionSet |> ValidationResult.collect (fragmentSpreadTargetDefinedInSelection fragmentDefinitionNames path)) - let rec private checkFragmentMustNotHaveCycles - (fragmentDefinitions : FragmentDefinition list) - (visited : string list) - (fragName : string) - (fragSelectionSet : Selection list) - = - let visitCount = - visited - |> Seq.filter (fun x -> x = fragName) - |> Seq.length - if visitCount > 1 then - AstError.AsResult $"Fragment '%s{fragName}' is making a cyclic reference." - else - fragSelectionSet - |> ValidationResult.collect (checkFragmentsMustNotHaveCyclesInSelection fragmentDefinitions (fragName :: visited)) - - and private checkFragmentsMustNotHaveCyclesInSelection (fragmentDefinitions : FragmentDefinition list) (visited : string list) = - function - | Field field -> - field.SelectionSet - |> ValidationResult.collect (checkFragmentsMustNotHaveCyclesInSelection fragmentDefinitions visited) - | InlineFragment inlineFrag -> - inlineFrag.SelectionSet - |> ValidationResult.collect (checkFragmentsMustNotHaveCyclesInSelection fragmentDefinitions visited) - | FragmentSpread spread -> - match - fragmentDefinitions - |> List.vtryFind (fun f -> f.Name.IsSome && f.Name.Value = spread.Name) - with - | ValueSome frag -> checkFragmentMustNotHaveCycles fragmentDefinitions visited spread.Name frag.SelectionSet - | ValueNone -> Success - + /// Reports, in definition order, every fragment that is part of a fragment spread cycle, including fragments that spread themselves. let internal validateFragmentsMustNotFormCycles (ctx : ValidationContext) = - let fragmentDefinitions = - ctx.FragmentDefinitions - |> List.map (fun frag -> frag.Definition) - let fragNamesAndSelections = + let fragmentDefinitions = getFragmentDefinitions ctx.Document + let cyclic = fragmentDefinitions - |> List.vchoose (fun frag -> frag.Name |> ValueOption.map (fun n -> n, frag.SelectionSet)) - fragNamesAndSelections - |> ValidationResult.collect (fun (name, selectionSet) -> checkFragmentMustNotHaveCycles fragmentDefinitions [] name selectionSet) + |> getFragmentsByName + |> getFragmentShapes + |> findCyclicFragments + let reported = HashSet (StringComparer.Ordinal) + fragmentDefinitions + |> ValidationResult.collect (fun fragment -> + let name = fragment.Name.Value + if cyclic.Contains name && reported.Add name then + AstError.AsResult $"Fragment '%s{name}' is making a cyclic reference." + else + Success) let private checkFragmentSpreadIsPossibleInSelection (path : FieldPath, parentType : IntrospectionType, fragmentType : IntrospectionType) = if not (typesAreApplicable (parentType, fragmentType)) then @@ -1402,63 +1589,48 @@ module Ast = | _ -> false) go (args |> List.map _.Value) - let rec private variableIsUsedInFragmentSpread - (name : string) - (fragmentDefinitions : FragmentDefinition list) - (visitedFragments : string list) - (spread : FragmentSpread) - = - if List.contains spread.Name visitedFragments then - false - else - let usedInSpread = - match - fragmentDefinitions - |> List.vtryFind (fun x -> x.Name.IsSome && x.Name.Value = spread.Name) - with - | ValueSome frag -> - let usedInSelection = - frag.SelectionSet - |> List.exists (variableIsUsedInSelection name fragmentDefinitions (spread.Name :: visitedFragments)) - usedInSelection - || (frag.Directives - |> List.exists (fun directive -> argumentsContains name directive.Arguments)) - | ValueNone -> false - usedInSpread - || (spread.Directives - |> List.exists (fun directive -> argumentsContains name directive.Arguments)) - - and private variableIsUsedInSelection (name : string) (fragmentDefinitions : FragmentDefinition list) (visitedFragments : string list) = - function - | Field field -> - if argumentsContains name field.Arguments then - true - else - let usedInSelection = - field.SelectionSet - |> List.exists (variableIsUsedInSelection name fragmentDefinitions visitedFragments) - usedInSelection - || (field.Directives - |> List.exists (fun directive -> argumentsContains name directive.Arguments)) - | InlineFragment frag -> - let usedInSelection = - frag.SelectionSet - |> List.exists (variableIsUsedInSelection name fragmentDefinitions visitedFragments) - usedInSelection - || (frag.Directives - |> List.exists (fun directive -> argumentsContains name directive.Arguments)) - | FragmentSpread spread -> variableIsUsedInFragmentSpread name fragmentDefinitions visitedFragments spread + /// + /// Whether the variable is used in the selection set, following its fragment spreads. + /// + /// + /// Each fragment is searched once: a fragment that has already been searched without finding the variable cannot contain it. + /// Searching each fragment once per path instead grows exponentially with nested spreads and factorially with spread cycles. + /// + let private variableIsUsed (name : string) (fragments : Dictionary) (selectionSet : Selection list) = + let searchedFragments = HashSet (StringComparer.Ordinal) + let usedInDirectives (directives : Directive list) = + directives + |> List.exists (fun directive -> argumentsContains name directive.Arguments) + let rec usedInSelection = + function + | Field field -> + argumentsContains name field.Arguments + || List.exists usedInSelection field.SelectionSet + || usedInDirectives field.Directives + | InlineFragment frag -> + List.exists usedInSelection frag.SelectionSet + || usedInDirectives frag.Directives + | FragmentSpread spread -> + let usedInFragment () = + searchedFragments.Add spread.Name + && (match fragments.TryGetValue spread.Name with + | true, frag -> + List.exists usedInSelection frag.SelectionSet + || usedInDirectives frag.Directives + | false, _ -> false) + usedInFragment () || usedInDirectives spread.Directives + List.exists usedInSelection selectionSet let internal validateAllVariablesUsed (ctx : ValidationContext) = - let fragmentDefinitions = getFragmentDefinitions ctx.Document + let fragments = + getFragmentDefinitions ctx.Document + |> getFragmentsByName ctx.Document.Definitions |> ValidationResult.collect (function | OperationDefinition def -> def.VariableDefinitions |> ValidationResult.collect (fun varDef -> - let isUsed = - def.SelectionSet - |> List.exists (variableIsUsedInSelection varDef.VariableName fragmentDefinitions []) + let isUsed = variableIsUsed varDef.VariableName fragments def.SelectionSet match def.Name, isUsed with | _, true -> Success | ValueSome operationName, _ -> @@ -1681,7 +1853,9 @@ module Ast = /// (Defer And Stream Directives Are Used On Valid Operations). /// let internal validateDeferStreamDirectivesOnValidOperations (ctx : ValidationContext) = - let fragmentDefinitions = getFragmentDefinitions ctx.Document + let fragmentDefinitions = + getFragmentDefinitions ctx.Document + |> getInlinableFragmentDefinitions ctx.Document.Definitions |> ValidationResult.collect (function | OperationDefinition def when def.OperationType = Subscription -> @@ -1699,7 +1873,9 @@ module Ast = /// (Defer And Stream Directives Are Used On Valid Root Field). /// let internal validateDeferStreamDirectivesOnRootFields (ctx : ValidationContext) = - let fragmentDefinitions = getFragmentDefinitions ctx.Document + let fragmentDefinitions = + getFragmentDefinitions ctx.Document + |> getInlinableFragmentDefinitions let mutationTypeName = ctx.Schema.MutationType |> ValueOption.map _.Name @@ -1722,7 +1898,9 @@ module Ast = /// (Defer And Stream Directive Labels Are Unique). /// let internal validateDeferStreamDirectiveLabels (ctx : ValidationContext) = - let fragmentDefinitions = getFragmentDefinitions ctx.Document + let fragmentDefinitions = + getFragmentDefinitions ctx.Document + |> getInlinableFragmentDefinitions let labelOf (directive : Directive) = directive.Arguments |> List.vtryFind (fun argument -> argument.Name = "label") @@ -1786,9 +1964,51 @@ module Ast = validateVariableUsagesAllowed ] + /// + /// Runs all validations against the document, unless the document exceeds the size limits, + /// and stops once more than errors are found. + /// + /// + /// Like graphql-js, the result then holds the first errors followed by an error saying + /// that validation was aborted. + /// + let internal validateDocumentWithLimits + (maxRecursiveSelections : int) + (maxNestingDepth : int) + (maxErrors : int) + (schema : IntrospectionSchema) + (ast : Document) + = + match checkDocumentLimits maxRecursiveSelections maxNestingDepth ast with + | ValidationError _ as limitExceeded -> limitExceeded + | Success -> + let schemaInfo = SchemaInfo.FromIntrospectionSchema (schema) + let context = getValidationContext schemaInfo ast + let errors = ResizeArray () + let mutable failed = false + let mutable validations = allValidations + while not validations.IsEmpty && errors.Count <= maxErrors do + match validations.Head context with + | Success -> () + | ValidationError validationErrors -> + failed <- true + errors.AddRange validationErrors + validations <- validations.Tail + if not failed then + Success + elif errors.Count > maxErrors then + ValidationError [ + yield! Seq.take maxErrors errors + AstError.Create "Too many validation errors, error limit reached. Validation aborted." + ] + else + ValidationError (List.ofSeq errors) + /// Run all available Ast validations against the given Document and IntrospectionSchema let validateDocument (schema : IntrospectionSchema) (ast : Document) = - let schemaInfo = SchemaInfo.FromIntrospectionSchema (schema) - let context = getValidationContext schemaInfo ast - allValidations - |> ValidationResult.collect (fun validate -> validate context) + validateDocumentWithLimits + DocumentLimitsDefaults.MaxRecursiveSelections + DocumentLimitsDefaults.MaxNestingDepth + DocumentLimitsDefaults.MaxValidationErrors + schema + ast diff --git a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs index 1f86a40a..8ec95226 100644 --- a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs +++ b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs @@ -2,6 +2,7 @@ namespace FSharp.Data.GraphQL.Validation open System.Collections.Generic open System.Text.Json.Serialization +open Microsoft.FSharp.Core.CompilerServices open FsToolkit.ErrorHandling open FSharp.Data.GraphQL @@ -35,8 +36,17 @@ module ValidationResult = /// Call the given sequence of validations, accumulating any errors, and return one ValidationResult. let collect (f : 'T -> ValidationResult<'Err>) (xs : 'T seq) : ValidationResult<'Err> = - // TODO: Use PSeq - Seq.fold (fun acc t -> acc @@ (f t)) Success xs + // Appending to the accumulated list with @@ copies it on every step, + // which makes a validation reporting many errors quadratic + let mutable errors = ListCollector<'Err> () + let mutable failed = false + for x in xs do + match f x with + | Success -> () + | ValidationError e -> + failed <- true + errors.AddMany e + if failed then ValidationError (errors.Close ()) else Success let mapErrors (f : 'Err1 -> 'Err2) (res : ValidationResult<'Err1>) : ValidationResult<'Err2> = match res with @@ -52,18 +62,21 @@ module GQLValidator = [] type AstError = + /// Creates a validation error. + /// The message of the error. + /// The reversed path of the selection that the error is about. + static member Create (message : string, ?path : FieldPath) : GQLProblemDetails = { + Message = message + Exception = ValueNone + Path = path |> Skippable.ofOption |> Skippable.map List.rev + Locations = Skip + Extensions = + Include ( + Dictionary () + |> GQLProblemDetails.SetErrorKind ErrorKind.Validation + ) + } + static member AsResult (message : string, ?path : FieldPath) = - [ - { - Message = message - Exception = ValueNone - Path = path |> Skippable.ofOption |> Skippable.map List.rev - Locations = Skip - Extensions = - Include ( - Dictionary () - |> GQLProblemDetails.SetErrorKind ErrorKind.Validation - ) - } - ] + [ AstError.Create (message, ?path = path) ] |> ValidationResult.ValidationError diff --git a/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj b/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj index 7fa78649..37e32914 100644 --- a/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj +++ b/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj @@ -104,6 +104,7 @@ + diff --git a/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs b/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs new file mode 100644 index 00000000..0ca661cf --- /dev/null +++ b/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs @@ -0,0 +1,424 @@ +// Some of the documents below are ported from the tests of Hot Chocolate +// (MIT License, Copyright (c) 2018 - present ChilliCream Inc.) +// and of Apollo Server (MIT License, Copyright (c) 2016-2020 Apollo Graph, Inc.). +module FSharp.Data.GraphQL.Tests.ValidationDoSTests + +open System +open System.Text +open System.Threading +open System.Threading.Tasks +open Xunit + +open FSharp.Data.GraphQL +open FSharp.Data.GraphQL.Types +open FSharp.Data.GraphQL.Validation +open FSharp.Data.GraphQL.Validation.Ast + +#nowarn "40" + +let rec UserType : ObjectDef = + DefineRec.Object( + "User", + fieldsFn = + fun () -> [ + Define.Field ("id", StringType, (fun _ _ -> "")) + Define.Field ("name", StringType, (fun _ _ -> "")) + Define.Field ("email", StringType, (fun _ _ -> "")) + Define.Field ("age", IntType, (fun _ _ -> 0)) + Define.Field ("address", StringType, (fun _ _ -> "")) + Define.Field ("phone", StringType, (fun _ _ -> "")) + Define.Field ("friend", UserType, (fun _ user -> user)) + ] + ) + +and QueryType : ObjectDef = + DefineRec.Object( + "Query", + fieldsFn = + fun () -> [ + Define.Field ("hello", StringType, (fun _ _ -> "")) + Define.Field ("field", QueryType, (fun _ root -> root)) + Define.Field ("me", UserType, (fun _ root -> root)) + Define.Field ("user", UserType, (fun _ root -> root)) + for name in [ "a"; "b"; "c"; "d"; "e" ] do + Define.Field (name, StringType, (fun _ _ -> "")) + ] + ) + +#warnon "40" + +let private schema = Schema (QueryType) + +let private introspectionSchema = (schema :> ISchema).Introspected + +let private schemaInfo = SchemaInfo.FromIntrospectionSchema introspectionSchema + +let private getContext = Parser.parse >> getValidationContext schemaInfo + +let private validate = Parser.parse >> validateDocument introspectionSchema + +/// Generous enough for slow CI machines, yet far below the minutes or hours that an exponential validation takes +let private timeout = TimeSpan.FromSeconds 10.0 + +/// +/// Runs the function on a thread with a 1 MiB stack, the smallest default thread stack among the supported platforms, +/// and fails when the function does not complete within the timeout. +/// +let private runIsolated (f : unit -> 'T) : 'T = + let completion = TaskCompletionSource<'T>(TaskCreationOptions.RunContinuationsAsynchronously) + let run () = + try + completion.SetResult (f ()) + with ex -> + completion.SetException ex + let thread = Thread (ThreadStart run, 1024 * 1024, IsBackground = true) + thread.Start () + if not (completion.Task.Wait timeout) then + fail $"The validation did not complete within %O{timeout}." + completion.Task.Result + +let private errorMessages (result : ValidationResult) = + match result with + | ValidationError errors -> errors |> List.map _.Message + | Success -> + fail "Expected validation errors, but the document is valid." + [] + +let private tooManySelections limit = $"The document recursively requests too many selections (more than %i{limit})." + +let private nestedTooDeeply limit = + $"The document is nested too deeply once fragment spreads are inlined (more than %i{limit} levels)." + +let private cyclicReference name = $"Fragment '%s{name}' is making a cyclic reference." + +/// The operation, followed by the fragments F0 to F(levels - 1) that each spread the next fragment twice, and F(levels) +let private fragmentBomb (operation : string) (levels : int) = + let document = StringBuilder () + document.AppendLine operation |> ignore + for i in 0 .. levels - 1 do + document.AppendLine $"fragment F%i{i} on Query {{ ...F%i{i + 1} ...F%i{i + 1} }}" + |> ignore + document.AppendLine $"fragment F%i{levels} on Query {{ a }}" + |> ignore + document.ToString () + +/// The operation, followed by the fragments F0 to F(length - 1) that each spread the next fragment once, and F(length) +let private fragmentChain (operation : string) (length : int) = + let document = StringBuilder () + document.AppendLine operation |> ignore + for i in 0 .. length - 1 do + document.AppendLine $"fragment F%i{i} on Query {{ ...F%i{i + 1} }}" + |> ignore + document.AppendLine $"fragment F%i{length} on Query {{ a }}" + |> ignore + document.ToString () + +[] +let ``Subscription root field rule terminates on a fragment that spreads itself`` () = + let actual = + runIsolated (fun () -> + AstValidationTests.getContext "subscription S { ...F } fragment F on Subscription { ...F }" + |> validateSubscriptionSingleRootField) + actual |> equals Success + +[] +let ``Subscription root field rule terminates on fragments that spread each other through an inline fragment`` () = + let actual = + runIsolated (fun () -> + AstValidationTests.getContext + """subscription S { ... on Subscription { ...A } } +fragment A on Subscription { ping ...B } +fragment B on Subscription { ...A }""" + |> validateSubscriptionSingleRootField) + actual |> equals Success + +[] +let ``Subscription root field rule counts fields selected before a fragment spread`` () = + let actual = + AstValidationTests.getContext "subscription S { first: ping ...F } fragment F on Subscription { second: ping }" + |> validateSubscriptionSingleRootField + actual + |> errorMessages + |> equals [ + "Subscription operations should have only one root field. Operation 'S' has 2 fields (second, first)." + ] + +[] +let ``Subscription root field rule collects a fragment spread twice only once`` () = + let actual = + AstValidationTests.getContext "subscription S { ...F ...F } fragment F on Subscription { ping }" + |> validateSubscriptionSingleRootField + actual |> equals Success + +[] +let ``Executor rejects a cyclic fragment in an unselected subscription instead of crashing`` () = + let executor = Executor (schema) + let result = + runIsolated (fun () -> + executor.CreateExecutionPlan ("query Q { hello } subscription S { ...F } fragment F on Query { ...F }", operationName = "Q")) + match result with + | Ok _ -> fail "Expected the execution plan to be rejected." + | Error (struct (_, errors)) -> errors |> hasError (cyclicReference "F") + +[] +let ``Fragment cycle rule reports only the fragments of the cycle`` () = + let actual = + getContext "{ a } fragment X on Query { ...A } fragment A on Query { ...A }" + |> validateFragmentsMustNotFormCycles + actual |> errorMessages |> equals [ cyclicReference "A" ] + +[] +let ``Validation of a cyclic fragment bomb completes quickly`` () = + let document = StringBuilder () + document.AppendLine "{ ...F0 }" |> ignore + for i in 0..29 do + let next = (i + 1) % 30 + document.AppendLine $"fragment F%i{i} on Query {{ ...F%i{next} ...F%i{next} }}" + |> ignore + let errors = runIsolated (fun () -> validate (document.ToString ()) |> errorMessages) + errors + |> List.filter (fun message -> message.EndsWith ("is making a cyclic reference.", StringComparison.Ordinal)) + |> List.length + |> equals 30 + +[] +let ``Validation of fragments that all spread each other completes quickly`` () = + // Following the spreads per path visits every ordering of the fragments: 11! paths + let names = [ for i in 0..11 -> $"F%i{i}" ] + let document = StringBuilder () + document.AppendLine "query Q($unused: Int) { ...F0 }" + |> ignore + for name in names do + let spreads = + names + |> List.filter (fun other -> other <> name) + |> List.map (fun other -> $"...%s{other} @defer") + |> String.concat " " + document.AppendLine $"fragment %s{name} on Query {{ a %s{spreads} }}" + |> ignore + let errors = runIsolated (fun () -> validate (document.ToString ()) |> errorMessages) + errors |> contains (cyclicReference "F11") |> ignore + errors + |> contains "A variable '$unused' is not used in operation 'Q'. Every variable must be used." + |> ignore + +let private apolloQuery = + """query { + user { + id + name + ...UserDetails + } +} + +fragment UserDetails on User { + email + age + ...MoreDetails +} + +fragment MoreDetails on User { + address + phone +}""" + +let private apolloBiggerQuery = + """query { + user { + email + age + address + phone + ...UserDetails + } +} + +fragment UserDetails on User { + id + name + email + age + ...MoreDetails +} + +fragment MoreDetails on User { + id + name + address + phone +}""" + +[] +let ``Document size counts every definition with its fragment spreads inlined`` () = + // Apollo Server counts 9 and 15 selections for the operations alone; the fragment definitions add 5 + 2 and 9 + 4 + let struct (querySelections, _) = measureDocument Int64.MaxValue (Parser.parse apolloQuery) + let struct (biggerQuerySelections, _) = measureDocument Int64.MaxValue (Parser.parse apolloBiggerQuery) + querySelections |> equals 16L + biggerQuerySelections |> equals 28L + +[] +let ``Selection limit accepts a document at the limit and rejects a bigger one`` () = + let validateWithLimit = + Parser.parse + >> validateDocumentWithLimits 16 128 100 introspectionSchema + validateWithLimit apolloQuery |> equals Success + validateWithLimit apolloBiggerQuery + |> errorMessages + |> equals [ tooManySelections 16 ] + +[] +let ``Fragment bomb is rejected quickly`` () = + let errors = + runIsolated (fun () -> + validate (fragmentBomb "query Q { ...F0 }" 20) + |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Unused fragment bomb is rejected quickly`` () = + let errors = runIsolated (fun () -> validate (fragmentBomb "{ a }" 20) |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Many operations spreading a large fragment are rejected`` () = + let document = StringBuilder () + for i in 0..199 do + document.AppendLine $"query Q%i{i} {{ ...Big }}" |> ignore + document.Append "fragment Big on Query {" |> ignore + for i in 0..999 do + document.Append $" a%i{i}: a" |> ignore + document.AppendLine " }" |> ignore + let errors = runIsolated (fun () -> validate (document.ToString ()) |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Recursive fragments fail`` () = + // Hot Chocolate: Ensure_Recursive_Fragments_Fail + let errors = runIsolated (fun () -> validate "fragment f on Query{...f} {...f}" |> errorMessages) + errors |> contains (cyclicReference "f") |> ignore + +[] +let ``Recursive fragments nested in fields fail`` () = + // Hot Chocolate: Ensure_Recursive_Fragments_Fail_2 + let document = + """fragment f on Query { + ...f + field { + ...f + field { + ...f + } + } +} + +{...f}""" + let errors = runIsolated (fun () -> validate document |> errorMessages) + errors |> contains (cyclicReference "f") |> ignore + +[] +let ``Fragment traversal bomb is rejected quickly`` () = + // Hot Chocolate: Fragment_Traversal_Bomb_Should_Complete_Quickly (CVE-2025-32032). + // Hot Chocolate accepts this document; it inlines 50^9 selections here, so the selection limit rejects it. + let names = "ABCDEFGHIJ" + let document = StringBuilder () + document.AppendLine "{...A}" |> ignore + for i in 0 .. names.Length - 2 do + let spreads = String.replicate 50 $" ...%c{names[i + 1]}" + document.AppendLine $"fragment %c{names[i]} on Query {{%s{spreads} }}" + |> ignore + document.AppendLine $"fragment %c{names[names.Length - 1]} on Query {{ __typename }}" + |> ignore + let errors = runIsolated (fun () -> validate (document.ToString ()) |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Fragment expansion bomb is rejected quickly`` () = + // Hot Chocolate: Fragment_Expansion_Bomb_Should_Complete_Quickly (CVE-2025-32032). + // Hot Chocolate accepts this document; it inlines 2^20 field selections here, so the selection limit rejects it. + let depth = 20 + let document = StringBuilder () + document.AppendLine "{ field { ...F0 } }" |> ignore + for i in 0 .. depth - 1 do + document.AppendLine $"fragment F%i{i} on Query {{ fa: field {{ ...F%i{i + 1} }} fb: field {{ ...F%i{i + 1} }} }}" + |> ignore + document.AppendLine $"fragment F%i{depth} on Query {{ __typename }}" + |> ignore + let errors = runIsolated (fun () -> validate (document.ToString ()) |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Deep fragment expansion is valid`` () = + // Hot Chocolate: Deep_Fragment_Expansion + let document = StringBuilder () + document.AppendLine "query { me { ...F0 } }" |> ignore + for i in 0..9 do + document.AppendLine $"fragment F%i{i} on User {{ a: friend {{ ...F%i{i + 1} }} b: friend {{ ...F%i{i + 1} }} }}" + |> ignore + document.AppendLine "fragment F10 on User { a: id b: id }" + |> ignore + let actual = runIsolated (fun () -> validate (document.ToString ())) + actual |> equals Success + +[] +let ``Inline fragments with many aliased typename fields validate quickly`` () = + // Hot Chocolate: Inline_Fragment_TypeName_Amplification_Should_Be_Rejected_By_Parser (CVE-2023-26144) + let document = StringBuilder () + document.AppendLine "{" |> ignore + for _ in 0..9 do + document.Append " ... on Query {" |> ignore + for j in 0..499 do + document.Append $" f%i{j}: __typename" |> ignore + document.AppendLine " }" |> ignore + document.AppendLine "}" |> ignore + let actual = runIsolated (fun () -> validate (document.ToString ())) + actual |> equals Success + +[] +let ``Long fragment chain is rejected without overflowing the stack`` () = + let errors = runIsolated (fun () -> validate (fragmentChain "{ ...F0 }" 10_000) |> errorMessages) + errors + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ] + +[] +let ``Fragment chain deeper than the nesting limit is rejected`` () = + let errors = runIsolated (fun () -> validate (fragmentChain "{ ...F0 }" 200) |> errorMessages) + errors + |> equals [ nestedTooDeeply DocumentLimitsDefaults.MaxNestingDepth ] + +[] +let ``Validation errors are capped`` () = + let fields = [ for i in 0..499 -> $"zz%i{i}" ] |> String.concat " " + let errors = validate $"{{ %s{fields} }}" |> errorMessages + errors + |> List.length + |> equals (DocumentLimitsDefaults.MaxValidationErrors + 1) + errors + |> List.last + |> equals "Too many validation errors, error limit reached. Validation aborted." + +[] +let ``Validation reporting fifty thousand errors completes quickly`` () = + let fields = [ for i in 0..49_999 -> $"zz%i{i}" ] |> String.concat " " + let document = Parser.parse $"{{ %s{fields} }}" + let errors = + runIsolated (fun () -> + validateDocumentWithLimits Int32.MaxValue 128 Int32.MaxValue introspectionSchema document + |> errorMessages) + errors |> List.length |> equals 50_000 + +[] +let ``Executor rejects a document with two hundred thousand fields`` () = + let query = "{ " + String.Join (" ", Seq.replicate 200_000 "a") + " }" + let executor = Executor (schema) + let result = runIsolated (fun () -> executor.CreateExecutionPlan query) + match result with + | Ok _ -> fail "Expected the execution plan to be rejected." + | Error (struct (_, errors)) -> + errors + |> List.map _.Message + |> equals [ tooManySelections DocumentLimitsDefaults.MaxRecursiveSelections ]