Fixed automatic persisted query requests being answered with the schema

RELEASE_NOTES.md

@@ -310,6 +310,8 @@
 * Added `GraphQLTransportWS.SubProtocol`, the `graphql-transport-ws` sub-protocol name
 * Fixed `graphql-transport-ws` delivery of `@defer` and `@stream` results, which are now sent as soon as they are produced instead of after a fixed 5 second delay, followed by a final payload with `hasNext: false`
 * Changed `graphql-transport-ws` incremental delivery of `@defer` and `@stream` results to the `pending`/`incremental`/`completed`/`hasNext` wire format used by graphql-js 17 and Apollo Client's `GraphQL17Alpha9Handler`, superseding the previous `data`/`path`/`hasNext` shape. Every deferred or streamed field is announced once, in a `pending` entry, and identified afterwards by a short id instead of its path. A deferred field is announced in the same payload as its own value, while a streamed field is announced as soon as the payload exposing its containing data is sent. A `@stream` field's items are always delivered to the client in list order, buffering an item that arrives out of turn until the item before it fills the gap, and a batch of items (grouped by `preferredBatchSize` or `StreamBatching`) is delivered as the `items` of a single `incremental` entry addressed by that id, rather than one payload per item
+* Fixed `GraphQLRequestHandler` answering an Apollo automatic persisted query request that carries only the hash in `extensions.persistedQuery` with the introspection result (`GET`) or a 400 "Invalid JSON body" problem (`POST`): such a request, in a JSON body, a multipart `operations` field or the `extensions` query string parameter of a `GET` request, is now answered as Apollo Server answers it with persisted queries disabled, with HTTP 200, `Cache-Control: private, no-cache, must-revalidate` and a body holding only a single `PersistedQueryNotSupported` error whose `extensions.code` is `PERSISTED_QUERY_NOT_SUPPORTED`, so that Apollo Client's persisted query link falls back to sending the full query; Apollo Client 4 would not fall back from a body with any other top-level member, such as `documentId`. `graphql-transport-ws` `subscribe` payloads, which must carry the query, are not checked
+* **Breaking Change** Changed `GraphQLRequestHandler` to reject a request that carries both a `query` and an `extensions.persistedQuery` with the same `PersistedQueryNotSupported` error, as Apollo Server does with persisted queries disabled, instead of executing the query and ignoring the extension
 * Added a completion signal to the engine's deferred/streamed event stream (`DeferredCompleted`), fired once after a `@defer` field's own payload and once after all of a `@stream` field's items, whether they succeeded or the source failed; used to build the `completed` entries of the new wire format
 * Changed `graphql-transport-ws` delivery of a deferred field to the addressing the specification and graphql-js/Apollo clients expect: the `pending` entry names the object containing the field and the `incremental` entry carries an object map of that one field; a field whose announcement the client never received, because the payload that should have exposed it resolved to `null` there, is no longer completed
 * Changed the `graphql-transport-ws` middleware to single-owner loops communicating through `System.Threading.Channels`: one reader of the socket, one control loop owning the subscription registry, one sender owning every write and the close of the socket, and one worker per subscription. No lock, no thread blocked on a send, a graceful close on application shutdown; a slow client's messages now queue in memory for the life of the connection instead of blocking the subscription's source

src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLRequestHandler.fs

@@ -1,6 +1,8 @@
 namespace FSharp.Data.GraphQL.Server.AspNetCore
 
 open System
+open System.Collections.Generic
+open System.Collections.Immutable
 open System.IO
 open System.Text.Json
 open System.Text.Json.Serialization
@@ -9,6 +11,7 @@ open Microsoft.AspNetCore.Http
 open Microsoft.Extensions.DependencyInjection
 open Microsoft.Extensions.Logging
 open Microsoft.Extensions.Options
+open Microsoft.Extensions.Primitives
 
 open FSharp.Data.GraphQL
 open FsToolkit.ErrorHandling
@@ -22,6 +25,154 @@ module private DeferredEventLogging =
     /// The path of a deferred event as one string for the log, its segments joined as GraphQL error paths print them
     let formatPath (path : obj list) = String.Join<obj> ("/", path)
 
+/// <summary>
+/// Recognizes requests for Apollo automatic persisted queries, which this server does not support.
+/// </summary>
+/// <remarks>
+/// <para>
+/// Apollo Client's persisted query link first sends only a hash of the query in <c>extensions.persistedQuery</c>, and sends
+/// the full query instead once the server answers with the <c>PersistedQueryNotSupported</c> error.
+/// </para>
+/// <para>
+/// The answer follows Apollo Server 5.5.1 with persisted queries disabled: <c>processGraphQLRequest</c> in
+/// <c>packages/server/src/requestPipeline.ts</c> rejects every request whose <c>extensions.persistedQuery</c> is truthy,
+/// whether it carries the query too or not, before looking at anything else, with the <c>PersistedQueryNotSupportedError</c>
+/// of <c>packages/server/src/internalErrorClasses.ts</c>: HTTP 200 and <c>cache-control: private, no-cache, must-revalidate</c>.
+/// </para>
+/// <para>
+/// The <c>subscribe</c> message of <c>graphql-transport-ws</c> is deliberately not checked: the protocol requires its payload
+/// to carry the query, a payload without one is already rejected, and its query is always executed, never answered with the
+/// introspection result. Apollo Client's <c>GraphQLWsLink</c> sends the full query together with the operation's extensions,
+/// a persisted query hash included, and the <c>graphql-ws</c> reference server Apollo Server's subscription setup relies on
+/// does not process persisted queries either, so rejecting such a payload would only break subscriptions that work.
+/// </para>
+/// </remarks>
+module internal PersistedQueries =
+
+    [<Literal>]
+    let NotSupportedMessage = "PersistedQueryNotSupported"
+
+    [<Literal>]
+    let NotSupportedCode = "PERSISTED_QUERY_NOT_SUPPORTED"
+
+    /// The member of the request extensions carrying the persisted query hash
+    [<Literal>]
+    let ExtensionName = "persistedQuery"
+
+    /// The query string parameter of a GET request carrying the request extensions as JSON
+    [<Literal>]
+    let ExtensionsParameterName = "extensions"
+
+    /// Forbids caching the answer, so that the full query the client sends next is not answered from a cache
+    let notSupportedCacheControl = StringValues "private, no-cache, must-revalidate"
+
+    /// The error Apollo Server answers with. Shared by every request, which is safe because both the record and its
+    /// immutable extensions dictionary cannot be changed.
+    let notSupportedError =
+        let extensions = ImmutableDictionary.CreateRange (StringComparer.Ordinal, [ kvpObj "code" NotSupportedCode ])
+        GQLProblemDetails.Create (NotSupportedMessage, extensions)
+
+/// <summary>
+/// Whether the <c>extensions</c> of a request ask for an Apollo automatic persisted query: whether their
+/// <c>persistedQuery</c> member is truthy in JavaScript, which is how Apollo Server tests it.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <see cref="PersistedQueryRequestConverter"/> reads it straight from the JSON of the request and skips every other value,
+/// so that extensions of any size are never copied: a client could otherwise make every request allocate a copy of a
+/// body as large as the server accepts.
+/// </para>
+/// <para>
+/// It is a struct rather than a record: the converters of the serializer options take precedence over the converter of a
+/// type, and the F# converter among them would read a record as one.
+/// </para>
+/// </remarks>
+[<Struct; JsonConverter(typeof<PersistedQueryRequestConverter>)>]
+type internal PersistedQueryRequest
+    /// <param name="isRequested">Whether the extensions ask for a persisted query.</param>
+    (isRequested : bool) =
+
+    /// Whether the extensions ask for a persisted query
+    member _.IsRequested = isRequested
+
+    /// <summary>
+    /// Whether the <c>extensions</c> query string parameter of a GET request asks for a persisted query. A value that is not
+    /// valid JSON is ignored, as the GET request handling ignores every other parameter.
+    /// </summary>
+    static member IsRequestedByQueryString (extensions : StringValues) =
+        extensions
+        |> Seq.exists (fun value ->
+            match value with
+            | null -> false
+            | value ->
+                try
+                    (JsonSerializer.Deserialize<PersistedQueryRequest> value).IsRequested
+                with :? JsonException ->
+                    false)
+
+/// <summary>Reads a <see cref="PersistedQueryRequest"/> from the <c>extensions</c> of a request.</summary>
+and [<Sealed>] internal PersistedQueryRequestConverter () =
+    inherit JsonConverter<PersistedQueryRequest> ()
+
+    /// <summary>
+    /// Whether the JSON value the reader is at is truthy in JavaScript, leaving the reader at the end of the value
+    /// </summary>
+    static member private IsTruthy (reader : byref<Utf8JsonReader>) =
+        match reader.TokenType with
+        | JsonTokenType.StartObject
+        | JsonTokenType.StartArray ->
+            reader.Skip ()
+            true
+        | JsonTokenType.True -> true
+        // The raw text of a string is empty only when the string is
+        | JsonTokenType.String ->
+            if reader.HasValueSequence then
+                reader.ValueSequence.Length > 0L
+            else
+                reader.ValueSpan.Length > 0
+        | JsonTokenType.Number ->
+            match reader.TryGetDouble () with
+            | true, number -> number <> 0.0
+            // Out of the range of a double, so not zero
+            | false, _ -> true
+        | _ -> false
+
+    override _.Read (reader, _, _) =
+        let mutable isRequested = false
+        if reader.TokenType = JsonTokenType.StartObject then
+            // Each property name is followed by its value; with duplicate members the last one counts, as in JSON.parse
+            while reader.Read () && reader.TokenType <> JsonTokenType.EndObject do
+                let isPersistedQuery = reader.ValueTextEquals PersistedQueries.ExtensionName
+                reader.Read () |> ignore
+                if isPersistedQuery then
+                    isRequested <- PersistedQueryRequestConverter.IsTruthy (&reader)
+                else
+                    reader.Skip ()
+        else
+            // Extensions that are not an object ask for nothing, as they hold no persistedQuery member
+            reader.Skip ()
+        PersistedQueryRequest isRequested
+
+    override _.Write (_, _, _) = raise (NotSupportedException "Persisted query requests are only read.")
+
+/// <summary>
+/// The members of a GraphQL over HTTP request body that <see cref="GQLRequestContent"/> does not carry.
+/// </summary>
+/// <remarks>
+/// Only the request handler reads it, alongside <see cref="GQLRequestContent"/>, so the public request type stays unchanged.
+/// </remarks>
+[<Struct>]
+type internal GQLRequestEnvelope = {
+    /// <summary>
+    /// Whether the <c>extensions</c> member of the request asks for a persisted query, the only extension the handler reads.
+    /// </summary>
+    /// <remarks>
+    /// <see cref="Skippable{T}"/>, as the optional members of <see cref="GQLRequestContent"/>, because the configured
+    /// FSharp.SystemTextJson converter rejects a body that leaves out a value option member.
+    /// </remarks>
+    Extensions : PersistedQueryRequest Skippable
+}
+
 /// Handles GraphQL requests using a provided root schema.
 type DefaultGraphQLRequestHandler<'Root>
     /// <summary>
@@ -155,6 +306,21 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
             return bytesRead > 0
     }
 
+    /// Rejects a request for an Apollo automatic persisted query with the answer Apollo Server gives when they are disabled,
+    /// so that Apollo Client's persisted query link sends the full query instead (see the PersistedQueries module)
+    let ensureNotPersistedQuery (isPersistedQuery : bool) : Result<unit, IResult> =
+        if not isPersistedQuery then
+            Ok ()
+        else
+            logger.LogDebug "Request asks for an automatic persisted query, which is not supported"
+            ctx.Response.Headers.CacheControl <- PersistedQueries.notSupportedCacheControl
+            // Only the errors, as Apollo Server answers: Apollo Client 4 takes a result with any other top-level member than
+            // data, errors and extensions for no GraphQL result at all, and then never falls back to the full query
+            let response = {| errors = [ PersistedQueries.notSupportedError ] |}
+            // HTTP 200 as Apollo Server answers, so that the client's handling of other statuses cannot mask the error;
+            // the error result stops the request from going any further
+            Error (TypedResults.Ok response :> IResult)
+
     /// Execute default or custom introspection query
     abstract ExecuteIntrospectionQuery : ast : Ast.Document voption -> Task<IResult>
 
@@ -175,14 +341,38 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
     /// and lastly by parsing document AST for introspection operation definition.
     /// </summary>
     /// <remarks>
+    /// <para>
     /// This consumes the request body: after JSON binding, the body stream position stays at its end,
     /// so calling this more than once (e.g. once from a derived handler and again from
     /// <see cref="HandleAsync"/>) will fail to bind the request on the second call.
+    /// </para>
+    /// <para>
+    /// Apollo automatic persisted queries are not supported. A request whose <c>extensions.persistedQuery</c> is set to anything
+    /// but <c>null</c>, <c>false</c>, <c>0</c> or <c>""</c> (the values JavaScript treats as false, as Apollo Server tests it),
+    /// in the JSON body or in the <c>extensions</c> query string parameter of a <c>GET</c> request, whether it carries the query
+    /// too or not, is answered as Apollo Server answers it with persisted queries disabled: the error result is an HTTP 200
+    /// response with a single <c>PersistedQueryNotSupported</c> error whose <c>extensions.code</c> is
+    /// <c>PERSISTED_QUERY_NOT_SUPPORTED</c>, and the <c>Cache-Control</c> header of the response is set to
+    /// <c>private, no-cache, must-revalidate</c>.
+    /// </para>
     /// </remarks>
     /// <returns>Result of check of <see cref="OperationType"/></returns>
     member _.CheckOperationType () = taskResult {
 
         let checkAnonymousFieldsOnly (ctx : HttpContext) = taskResult {
+            // Read ahead of the request itself, which cannot be bound without the query a persisted query request leaves out
+            let! envelope = ctx.TryBindJsonAsync<GQLRequestEnvelope>(GQLRequestContent.expectedJSON)
+            do!
+                envelope.Extensions
+                |> Skippable.toValueOption
+                |> ValueOption.exists _.IsRequested
+                |> ensureNotPersistedQuery
+
+            // Binding the envelope read a JSON body to its end, while a form's operations field is read again from the parsed form
+            let body = ctx.Request.Body
+            if body.CanSeek then
+                body.Seek (0L, SeekOrigin.Begin) |> ignore
+
             let! gqlRequest = ctx.TryBindJsonAsync<GQLRequestContent>(GQLRequestContent.expectedJSON)
             let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query
             let operationName = gqlRequest.OperationName |> Skippable.toValueOption
@@ -222,6 +412,10 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
         let request = ctx.Request
 
         if HttpMethods.Get = request.Method then
+            do!
+                request.Query[PersistedQueries.ExtensionsParameterName]
+                |> PersistedQueryRequest.IsRequestedByQueryString
+                |> ensureNotPersistedQuery
             logger.LogTrace ("Request is GET. Must be an introspection query")
             return IntrospectionQuery <| ValueNone
         else