From f3ae52697f5190f9dbedf8d916469dd19104b15b Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 3 Oct 2026 02:30:44 +0200 Subject: [PATCH 1/2] Fixed parser denial of service through deep nesting and nested list types - `Parser.parse` and `Parser.tryParse` scan the document first and reject braces, brackets and parentheses nested deeper than `DocumentLimitsDefaults.MaxNestingDepth` (128) outside of strings and comments, instead of overflowing the stack - List types are parsed once before looking for `!`, instead of backtracking from a non-null attempt, which took exponential time in the nesting depth - An integer out of the 64-bit range is a syntax error instead of an `OverflowException` escaping `tryParse` - Moved the test helper running code on a 1 MiB stack into `Helpers.fs` Co-Authored-By: Claude Opus 5.5 --- RELEASE_NOTES.md | 3 + .../DocumentLimits.fs | 10 +- src/FSharp.Data.GraphQL.Shared/Parser.fs | 135 +++++++++-- .../FSharp.Data.GraphQL.Tests.fsproj | 1 + tests/FSharp.Data.GraphQL.Tests/Helpers.fs | 20 ++ .../ParserLimitsTests.fs | 224 ++++++++++++++++++ .../ValidationDoSTests.fs | 19 +- 7 files changed, 377 insertions(+), 35 deletions(-) create mode 100644 tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 194962139..c0a6fbde4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -334,3 +334,6 @@ * 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 +* **Breaking Change** `Parser.parse` and `Parser.tryParse` now reject a document whose braces, brackets and parentheses, outside of strings and comments, are nested deeper than `DocumentLimitsDefaults.MaxNestingDepth` (128), with a syntax error giving the line and column of the first one too deep. Deeper documents used to overflow the stack, which terminated the process +* Fixed parsing time growing exponentially with the nesting of list types in variable definitions: a 100-deep `[[…Int…]]` never finished parsing +* Fixed `Parser.tryParse` throwing `OverflowException`, and so an HTTP server answering with an unhandled exception, for an integer out of the range of 64-bit integers; it is now a syntax error diff --git a/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs index 27aa4f359..939d543bd 100644 --- a/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs +++ b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs @@ -1,13 +1,17 @@ namespace FSharp.Data.GraphQL -/// Default limits on the work that an untrusted document can cause while it is validated. +/// Default limits on the work that an untrusted document can cause while it is parsed and validated. [] module DocumentLimitsDefaults = /// - /// The maximum nesting depth of a document once its fragment spreads are inlined. + /// The maximum nesting depth of a document. /// - /// Every selection set of a field, every inline fragment and every fragment spread adds one level. + /// The parser counts nested braces, brackets and parentheses outside of strings and comments, before it parses the document. + /// + /// + /// Validation counts nesting once fragment spreads are inlined: every selection set of a field, every inline fragment and + /// every fragment spread adds one level. /// /// [] diff --git a/src/FSharp.Data.GraphQL.Shared/Parser.fs b/src/FSharp.Data.GraphQL.Shared/Parser.fs index d049a4192..f2f1e8f72 100644 --- a/src/FSharp.Data.GraphQL.Shared/Parser.fs +++ b/src/FSharp.Data.GraphQL.Shared/Parser.fs @@ -5,6 +5,7 @@ module FSharp.Data.GraphQL.Parser open System +open System.Globalization open FParsec open FSharp.Data.GraphQL.Ast open FsToolkit.ErrorHandling @@ -132,7 +133,13 @@ module internal Internal = // 2.9.1 IntValue // IntegerPart - let integerValue = integerPart |>> int64 + let integerValue = + // A conversion that throws would escape tryParse as an unhandled exception + integerPart + >>= fun text -> + match Int64.TryParse (text, NumberStyles.AllowLeadingSign, CultureInfo.InvariantCulture) with + | true, value -> preturn value + | false, _ -> fail $"The integer %s{text} is out of the range of 64-bit integers." // 2.9.2 FloatValue @@ -231,10 +238,13 @@ module internal Internal = let inputType, inputTypeRef = createParserForwardedToRef () let namedType = name |>> NamedType "NamedType" let listType = betweenChars '[' ']' inputType |>> ListType "ListType" - let nonNullType = - (listType <|> namedType) .>> pchar '!' |>> NonNullType - "NonNullType" - inputTypeRef.Value <- choice [ attempt nonNullType; namedType; listType ] + // Parses the type once and then looks for '!': trying a non-null type first and backtracking to a nullable one + // parsed every nested list type twice, which took exponential time in the nesting depth + inputTypeRef.Value <- + pipe2 (listType <|> namedType) (opt (pchar '!')) (fun inputType nonNull -> + match nonNull with + | Some _ -> NonNullType inputType + | None -> inputType) // 2.4 Selection Sets @@ -369,14 +379,111 @@ module internal Internal = |>> (fun definitions -> { Document.Definitions = definitions }) -/// Parses a GraphQL Document. Throws exception on invalid formats. +/// +/// The line and column of the first brace, bracket or parenthesis nested deeper than , +/// skipping comments, strings and block strings. +/// +/// +/// The parser recurses once per nesting level, so a deeply nested document overflows the stack, which terminates the process. +/// This linear scan rejects such a document before it is parsed. Lines and columns are 1-based, columns count UTF-16 code units, +/// and \r\n, \r and \n each end a line. +/// +let internal tryFindNestingViolation (maxDepth : int) (query : string) : struct (int * int) voption = + let length = query.Length + let isBlockStringQuote index = + index + 2 < length + && query[index] = '"' + && query[index + 1] = '"' + && query[index + 2] = '"' + let mutable depth = 0 + let mutable line = 1 + let mutable lineStart = 0 + let mutable index = 0 + let mutable violation = ValueNone + while violation.IsNone && index < length do + match query[index] with + | '#' -> + // A comment runs to the end of the line, which the next iteration counts + while index < length && query[index] <> '\n' && query[index] <> '\r' do + index <- index + 1 + | '"' when isBlockStringQuote index -> + index <- index + 3 + let mutable closed = false + while not closed && index < length do + match query[index] with + | '\\' when isBlockStringQuote (index + 1) -> index <- index + 4 + | '"' when isBlockStringQuote index -> + index <- index + 3 + closed <- true + | '\r' -> + if index + 1 < length && query[index + 1] = '\n' then + index <- index + 1 + index <- index + 1 + line <- line + 1 + lineStart <- index + | '\n' -> + index <- index + 1 + line <- line + 1 + lineStart <- index + | _ -> index <- index + 1 + | '"' -> + index <- index + 1 + let mutable closed = false + while not closed && index < length do + match query[index] with + | '\\' -> index <- index + 2 + | '"' -> + index <- index + 1 + closed <- true + // A string cannot span lines: the next iteration counts the line terminator + | '\n' + | '\r' -> closed <- true + | _ -> index <- index + 1 + | '{' + | '[' + | '(' -> + depth <- depth + 1 + if depth > maxDepth then + violation <- ValueSome (struct (line, index - lineStart + 1)) + index <- index + 1 + | '}' + | ']' + | ')' -> + if depth > 0 then + depth <- depth - 1 + index <- index + 1 + | '\r' -> + if index + 1 < length && query[index + 1] = '\n' then + index <- index + 1 + index <- index + 1 + line <- line + 1 + lineStart <- index + | '\n' -> + index <- index + 1 + line <- line + 1 + lineStart <- index + | _ -> index <- index + 1 + violation + +/// Formats the nesting violation the way FParsec formats a syntax error +let private nestingErrorMessage (struct (line : int, column : int)) = + $"Error in Ln: %i{line} Col: %i{column}\nThe document is nested deeper than %i{DocumentLimitsDefaults.MaxNestingDepth} braces, brackets and parentheses." + +/// Parses a GraphQL Document. Throws exception on invalid formats. +/// The document is not valid GraphQL or is nested too deeply. let parse query = - match run documents query with - | Success (result, _, _) -> result - | Failure (errorMsg, _, _) -> raise (System.FormatException (errorMsg)) - -/// Parses a GraphQL Document. Throws exception on invalid formats. + match tryFindNestingViolation DocumentLimitsDefaults.MaxNestingDepth query with + | ValueSome position -> raise (FormatException (nestingErrorMessage position)) + | ValueNone -> + match run documents query with + | Success (result, _, _) -> result + | Failure (errorMsg, _, _) -> raise (FormatException (errorMsg)) + +/// Parses a GraphQL Document, returning the error message when the document is not valid GraphQL or is nested too deeply. let tryParse query = - match run documents query with - | Success (result, _, _) -> Result.Ok result - | Failure (errorMsg, _, _) -> Result.Error errorMsg + match tryFindNestingViolation DocumentLimitsDefaults.MaxNestingDepth query with + | ValueSome position -> Result.Error (nestingErrorMessage position) + | ValueNone -> + match run documents query with + | Success (result, _, _) -> Result.Ok result + | Failure (errorMsg, _, _) -> Result.Error errorMsg 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 37e32914c..2a4821315 100644 --- a/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj +++ b/tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj @@ -105,6 +105,7 @@ + diff --git a/tests/FSharp.Data.GraphQL.Tests/Helpers.fs b/tests/FSharp.Data.GraphQL.Tests/Helpers.fs index 1812a397a..4b101a620 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Helpers.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Helpers.fs @@ -296,6 +296,26 @@ let waitForTask (timeout : TimeSpan) (message : string) (awaited : Task) : Task fail message } +/// +/// Runs the function on a thread with a 1 MiB stack, the smallest default thread stack among the supported platforms, +/// and fails the test when the function does not complete within the timeout. +/// +/// +/// A stack overflow still terminates the test process, which fails the test run. +/// +let runOnSmallStack (timeout : TimeSpan) (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 call did not complete within %O{timeout}." + completion.Task.Result + open FSharp.Control /// Returns the value after the scaled delay diff --git a/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs b/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs new file mode 100644 index 000000000..4c782d4d5 --- /dev/null +++ b/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs @@ -0,0 +1,224 @@ +module FSharp.Data.GraphQL.Tests.ParserLimitsTests + +open System +open Xunit + +open FSharp.Data.GraphQL +open FSharp.Data.GraphQL.Ast + +/// Generous enough for slow CI machines, yet far below the time that exponential parsing of the documents below takes +let private timeout = TimeSpan.FromSeconds 10.0 + +let private parseIsolated (query : string) = runOnSmallStack timeout (fun () -> Parser.tryParse query) + +let private wantDocument (result : Result) = + match result with + | Ok document -> document + | Error message -> + fail $"Expected the document to parse, but got: %s{message}" + Unchecked.defaultof<_> + +let private wantError (result : Result) = + match result with + | Ok _ -> + fail "Expected a syntax error, but the document parsed." + "" + | Error message -> message + +let private assertNestedTooDeeply (message : string) = + Assert.Contains ( + $"The document is nested deeper than %i{DocumentLimitsDefaults.MaxNestingDepth} braces, brackets and parentheses.", + message, + StringComparison.Ordinal + ) + +let private variableType (document : Document) = + match document.Definitions with + | [ OperationDefinition operation ] -> operation.VariableDefinitions.Head.Type + | definitions -> + fail $"Expected a single operation, but got %A{definitions}" + Unchecked.defaultof<_> + +let rec private listTypeDepth = + function + | ListType inner -> 1 + listTypeDepth inner + | NonNullType inner -> listTypeDepth inner + | NamedType _ -> 0 + +/// Selection sets nested to the given depth: "{ a { a … } }" +let private nestedSelections (depth : int) = String.replicate depth "{ a " + String.replicate depth "}" + +[] +let ``List type nested 100 deep parses in linear time`` () = + let inputType = String.replicate 100 "[" + "Int" + String.replicate 100 "]" + let document = + parseIsolated $"query ($v: %s{inputType}) {{ a }}" + |> wantDocument + document |> variableType |> listTypeDepth |> equals 100 + +[] +let ``Non-null list type nested 100 deep parses in linear time`` () = + let inputType = + String.replicate 100 "[" + + "Int!" + + String.replicate 100 "]!" + let document = + parseIsolated $"query ($v: %s{inputType}) {{ a }}" + |> wantDocument + let actual = variableType document + actual |> listTypeDepth |> equals 100 + match actual with + | NonNullType (ListType _) -> () + | other -> fail $"Expected a non-null list type, but got %A{other}" + +[] +let ``Non-null and list types parse as before`` () = + let document = Parser.parse "query ($a: Int!, $b: [Int!]!, $c: [Int]) { a }" + let actual = + match document.Definitions with + | [ OperationDefinition operation ] -> operation.VariableDefinitions |> List.map _.Type + | _ -> [] + actual + |> equals [ + NonNullType (NamedType "Int") + NonNullType (ListType (NonNullType (NamedType "Int"))) + ListType (NamedType "Int") + ] + +[] +let ``Selection sets nested to the limit parse`` () = + parseIsolated (nestedSelections DocumentLimitsDefaults.MaxNestingDepth) + |> wantDocument + |> ignore + +[] +let ``Values, types and inline fragments nested to the limit parse`` () = + // The braces and parentheses around them count as two levels + let depth = DocumentLimitsDefaults.MaxNestingDepth - 2 + let listValue = + String.replicate depth "[" + + "1" + + String.replicate depth "]" + let objectValue = + String.replicate depth "{ b: " + + "1" + + String.replicate depth " }" + let inputType = + String.replicate (depth + 1) "[" + + "Int!" + + String.replicate (depth + 1) "]!" + // "{ ... on Query { ... on Query { a } } }": each inline fragment opens one more selection set + let fragmentDepth = DocumentLimitsDefaults.MaxNestingDepth - 1 + let inlineFragments = + String.replicate fragmentDepth "{ ... on Query " + + "{ a }" + + String.replicate fragmentDepth " }" + for document in + [ + $"{{ f(a: %s{listValue}) }}" + $"{{ f(a: %s{objectValue}) }}" + $"query ($v: %s{inputType}) {{ a }}" + inlineFragments + ] do + parseIsolated document |> wantDocument |> ignore + +[] +[] +[] +[] +[] +let ``Selection sets nested past the limit are rejected`` (depth : int) = + parseIsolated (nestedSelections depth) + |> wantError + |> assertNestedTooDeeply + +[] +let ``List value nested 1000 deep is rejected`` () = + let value = String.replicate 1000 "[" + "1" + String.replicate 1000 "]" + parseIsolated $"{{ f(a: %s{value}) }}" + |> wantError + |> assertNestedTooDeeply + +[] +let ``Object value nested 1000 deep is rejected`` () = + let value = + String.replicate 1000 "{ b: " + + "1" + + String.replicate 1000 " }" + parseIsolated $"{{ f(a: %s{value}) }}" + |> wantError + |> assertNestedTooDeeply + +[] +let ``Twenty thousand unclosed selection sets are rejected`` () = + parseIsolated (String.replicate 20000 "{") + |> wantError + |> assertNestedTooDeeply + +[] +let ``Nesting beyond the limit throws a FormatException from parse`` () = + throws(fun () -> Parser.parse (nestedSelections 129) |> ignore) + |> _.Message + |> assertNestedTooDeeply + +[] +let ``Brackets in strings do not count as nesting`` () = + let brackets = String.replicate 200 "[" + parseIsolated $"{{ f(a: \"%s{brackets}\") }}" + |> wantDocument + |> ignore + parseIsolated $"{{ f(a: \"\\\"%s{brackets}\") }}" + |> wantDocument + |> ignore + +[] +let ``Brackets in comments do not count as nesting`` () = + let braces = String.replicate 200 "{" + parseIsolated $"# %s{braces}\n{{ a }}" + |> wantDocument + |> ignore + +[] +let ``Brackets in block strings do not count as nesting`` () = + let braces = String.replicate 200 "{" + let brackets = String.replicate 200 "[" + Parser.tryFindNestingViolation 128 $"\"\"\"%s{braces}\\\"\"\"%s{brackets}\"\"\" {{ a }}" + |> equals ValueNone + +[] +let ``A quote in a comment does not hide brackets`` () = + let braces = String.replicate 200 "{" + parseIsolated $"# \"\n%s{braces}" + |> wantError + |> assertNestedTooDeeply + +[] +let ``Nesting violation reports its line and column`` () = + Parser.tryFindNestingViolation 2 "{ a { b { c } } }" + |> equals (ValueSome (struct (1, 9))) + Parser.tryFindNestingViolation 2 "{\r\n {\r\n {" + |> equals (ValueSome (struct (3, 3))) + Parser.tryFindNestingViolation 2 "{\n{\r{" + |> equals (ValueSome (struct (3, 1))) + Parser.tryFindNestingViolation 2 "{ a }\n{ b }\n{ c }" + |> equals ValueNone + +[] +let ``Integers out of the 64-bit range are syntax errors`` () = + let message = Parser.tryParse "{ f(a: 9223372036854775808) }" |> wantError + Assert.Contains ("The integer 9223372036854775808 is out of the range of 64-bit integers.", message, StringComparison.Ordinal) + throws(fun () -> Parser.parse "{ f(a: -9223372036854775809) }" |> ignore) + |> ignore + +[] +let ``Integers at the ends of the 64-bit range parse`` () = + let document = Parser.parse "{ f(a: 9223372036854775807, b: -9223372036854775808) }" + let actual = + match document.Definitions with + | [ OperationDefinition operation ] -> + match operation.SelectionSet with + | [ Field field ] -> field.Arguments |> List.map _.Value + | _ -> [] + | _ -> [] + actual + |> equals [ IntValue Int64.MaxValue; IntValue Int64.MinValue ] diff --git a/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs b/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs index 0ca661cfe..3204d3767 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ValidationDoSTests.fs @@ -5,8 +5,6 @@ module FSharp.Data.GraphQL.Tests.ValidationDoSTests open System open System.Text -open System.Threading -open System.Threading.Tasks open Xunit open FSharp.Data.GraphQL @@ -60,22 +58,7 @@ 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 runIsolated (f : unit -> 'T) : 'T = runOnSmallStack timeout f let private errorMessages (result : ValidationResult) = match result with From f9077f98c03d14f47fc2ab89ba3f601cbc3ed93a Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 3 Oct 2026 02:56:31 +0200 Subject: [PATCH 2/2] Aligned the nesting scan with how the grammar splits comments and strings The scan ended comments only at `\n` and `\r` and skipped `"""` block strings, while the grammar also ends comments and strings at U+2028 and U+2029 and has no block strings, reading `""""` as two empty strings. Brackets after such a separator or a run of quotes were parsed but not counted, so deep nesting still overflowed the stack. - The scan and the grammar share one set of line terminators - The scan reads strings exactly as the grammar does, without block strings - Regression tests go through `Parser.tryParse` Co-Authored-By: Claude Opus 5.5 --- .../DocumentLimits.fs | 2 +- src/FSharp.Data.GraphQL.Shared/Parser.fs | 65 ++++++++----------- .../ParserLimitsTests.fs | 21 ++++-- 3 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs index 939d543bd..418405eb2 100644 --- a/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs +++ b/src/FSharp.Data.GraphQL.Shared/DocumentLimits.fs @@ -24,7 +24,7 @@ module DocumentLimitsDefaults = /// 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 + /// The work of validating and planning a document grows with this number, so a higher limit lets a small document /// keep a server busy for longer. /// /// diff --git a/src/FSharp.Data.GraphQL.Shared/Parser.fs b/src/FSharp.Data.GraphQL.Shared/Parser.fs index f2f1e8f72..698b336d4 100644 --- a/src/FSharp.Data.GraphQL.Shared/Parser.fs +++ b/src/FSharp.Data.GraphQL.Shared/Parser.fs @@ -13,16 +13,20 @@ open FsToolkit.ErrorHandling [] module internal Internal = + // 2.1.3 LineTerminator + // New Line (U+000A) + // Carriage Return (U+000D)New Line (U+000A) | (U+000D)New Line (U+000A) + // This grammar also ends comments and strings at the Unicode line (U+2028) and paragraph (U+2029) separators. + // tryFindNestingViolation must split comments and strings exactly as the grammar does, so both use this set. + let lineTerminatorChars = [| '\u000A'; '\u000D'; '\u2028'; '\u2029' |] + // 2.1.7 Ignored tokens let ignored = // 2.1.2 WhiteSpace // Horizontal Tab (U+0009) | Space (U+0020) let whiteSpace = skipAnyOf [| '\u0009'; '\u000B'; '\u000C'; '\u0020'; '\u00A0' |] - // 2.1.3 LineTerminator - // New Line (U+000A) - // Carriage Return (U+000D)New Line (U+000A) | (U+000D)New Line (U+000A) - let lineTerminators = skipAnyOf [| '\u000A'; '\u000D'; '\u2028'; '\u2029' |] + let lineTerminators = skipAnyOf lineTerminatorChars // 2.1.4 CommentChar // SourceCharacter but not LineTerminator @@ -104,7 +108,7 @@ module internal Internal = |> char) pchar '\\' >>. (escaped <|> unicode) - let normalCharacter = noneOf [| '\u000A'; '\u000D'; '\u2028'; '\u2029'; '"' |] + let normalCharacter = noneOf [| yield! lineTerminatorChars; '"' |] let quote = pchar '"' between quote quote (manyChars (escapedCharacter <|> normalCharacter)) @@ -381,20 +385,26 @@ module internal Internal = /// /// The line and column of the first brace, bracket or parenthesis nested deeper than , -/// skipping comments, strings and block strings. +/// skipping comments and strings. /// /// +/// /// The parser recurses once per nesting level, so a deeply nested document overflows the stack, which terminates the process. -/// This linear scan rejects such a document before it is parsed. Lines and columns are 1-based, columns count UTF-16 code units, -/// and \r\n, \r and \n each end a line. +/// This linear scan rejects such a document before it is parsed. +/// +/// +/// It must split comments and strings exactly as the grammar does: a bracket the scan takes for comment or string text, +/// but the grammar parses, escapes the limit. Comments and strings therefore end at the same line terminators as in the +/// grammar, and there are no block strings, which the grammar does not support either. +/// +/// +/// Lines and columns are 1-based and columns count UTF-16 code units. As in FParsec error positions, +/// \r\n, \r and \n each start a new line. +/// /// let internal tryFindNestingViolation (maxDepth : int) (query : string) : struct (int * int) voption = let length = query.Length - let isBlockStringQuote index = - index + 2 < length - && query[index] = '"' - && query[index + 1] = '"' - && query[index + 2] = '"' + let isLineTerminator (c : char) = Array.contains c lineTerminatorChars let mutable depth = 0 let mutable line = 1 let mutable lineStart = 0 @@ -403,29 +413,9 @@ let internal tryFindNestingViolation (maxDepth : int) (query : string) : struct while violation.IsNone && index < length do match query[index] with | '#' -> - // A comment runs to the end of the line, which the next iteration counts - while index < length && query[index] <> '\n' && query[index] <> '\r' do + // A comment runs to the line terminator, which the next iteration handles + while index < length && not (isLineTerminator query[index]) do index <- index + 1 - | '"' when isBlockStringQuote index -> - index <- index + 3 - let mutable closed = false - while not closed && index < length do - match query[index] with - | '\\' when isBlockStringQuote (index + 1) -> index <- index + 4 - | '"' when isBlockStringQuote index -> - index <- index + 3 - closed <- true - | '\r' -> - if index + 1 < length && query[index + 1] = '\n' then - index <- index + 1 - index <- index + 1 - line <- line + 1 - lineStart <- index - | '\n' -> - index <- index + 1 - line <- line + 1 - lineStart <- index - | _ -> index <- index + 1 | '"' -> index <- index + 1 let mutable closed = false @@ -435,9 +425,8 @@ let internal tryFindNestingViolation (maxDepth : int) (query : string) : struct | '"' -> index <- index + 1 closed <- true - // A string cannot span lines: the next iteration counts the line terminator - | '\n' - | '\r' -> closed <- true + // A string cannot contain a line terminator, which the next iteration handles + | c when isLineTerminator c -> closed <- true | _ -> index <- index + 1 | '{' | '[' diff --git a/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs b/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs index 4c782d4d5..14423ffe4 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ParserLimitsTests.fs @@ -179,11 +179,22 @@ let ``Brackets in comments do not count as nesting`` () = |> ignore [] -let ``Brackets in block strings do not count as nesting`` () = - let braces = String.replicate 200 "{" - let brackets = String.replicate 200 "[" - Parser.tryFindNestingViolation 128 $"\"\"\"%s{braces}\\\"\"\"%s{brackets}\"\"\" {{ a }}" - |> equals ValueNone +let ``Runs of quotes do not hide nesting`` () = + // The grammar has no block strings: it reads """" as two empty strings, so the brackets after them are nested values + let value = String.replicate 140 "[" + "1" + String.replicate 140 "]" + parseIsolated $"{{ f(a: [\"\"\"\" %s{value} \"\"\"\"\n]) }}" + |> wantError + |> assertNestedTooDeeply + +[] +[] +[] +let ``Comments ended by a Unicode line or paragraph separator do not hide nesting`` (separator : int) = + // The grammar ends a comment at these separators, so the braces after them are nested selection sets. + // The separator is built from its code because Fantomas would write an escape of it out as the raw character. + parseIsolated ("# c" + string (char separator) + nestedSelections 200) + |> wantError + |> assertNestedTooDeeply [] let ``A quote in a comment does not hide brackets`` () =