proper header handling

.changeset/khaki-hairs-protect.md

@@ -0,0 +1,11 @@
+---
+"graphql-helix": minor
+---
+
+Adjust the handling of the `accept` header.
+
+- Clients accepting the `application/graphql+json` header (via the `accept` header) will receive responses that use `Content-Type: application/graphql+json` over `application/json` ([which is only for supporting legacy clients](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#content-types)).
+  Clients that do not specify any `accept` headers will still receive `Content-Type: application/graphql`. This is not considered a breaking change as clients accepting the `application/graphql+json` header should also be able to process it.
+  Note: When using the `application/graphql+json` content-type header you need to configure your HTTP server/framework to parse this content-type as JSON.
+- GET `text/event-stream` requests will now ALWAYS return a `PushResponse` instead of a `MultipartResponse`. Previously helix would send a `MultipartResponse` if the accept header was not a strict equal to `accept: text/event-stream`.
+- POST requests that try to execute Subscription operations will now receive an error and 405 status code. This is not considered a breaking change as SSE is not doable over POST by the specification and was never officially supported.

packages/core/lib/process-request.ts

@@ -13,7 +13,15 @@ import {
 } from "graphql";
 import { stopAsyncIteration, isAsyncIterable, isHttpMethod } from "./util/index";
 import { HttpError } from "./errors";
-import { ExecutionContext, ExecutionPatchResult, MultipartResponse, ProcessRequestOptions, ProcessRequestResult } from "./types";
+import {
+  ExecutionContext,
+  ExecutionPatchResult,
+  MultipartResponse,
+  ProcessRequestOptions,
+  ProcessRequestResult,
+  Request,
+} from "./types";
+import { getRankedResponseProtocols, RankedResponseProtocols } from "./util/get-ranked-response-protocols";
 
 const parseQuery = (query: string | DocumentNode, parse: typeof defaultParse): DocumentNode | Promise<DocumentNode> => {
   if (typeof query !== "string" && query.kind === "Document") {
@@ -61,6 +69,17 @@ const getExecutableOperation = (document: DocumentNode, operationName?: string):
   return operation;
 };
 
+// If clients do not accept application/graphql+json use application/json - otherwise respect the order in the accept header
+const getSingleResponseContentType = (protocols: RankedResponseProtocols): "application/graphql+json" | "application/json" => {
+  if (protocols["application/graphql+json"] === -1) {
+    return "application/json";
+  }
+  return protocols["application/graphql+json"] > protocols["application/json"] ? "application/graphql+json" : "application/json";
+};
+
+const getHeader = (request: Request, headerName: string): unknown =>
+  typeof request.headers.get === "function" ? request.headers.get(headerName) : (request.headers as any)[headerName];
+
 export const processRequest = async <TContext = {}, TRootValue = {}>(
   options: ProcessRequestOptions<TContext, TRootValue>
 ): Promise<ProcessRequestResult<TContext, TRootValue>> => {
@@ -86,18 +105,29 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
   let operation: OperationDefinitionNode | undefined;
 
   const result = await (async (): Promise<ProcessRequestResult<TContext, TRootValue>> => {
-    const accept = typeof request.headers.get === "function" ? request.headers.get("accept") : (request.headers as any).accept;
-    const isEventStream = accept === "text/event-stream";
+    const accept: unknown = getHeader(request, "accept");
+    const contentType: unknown = getHeader(request, "contentType");
+
+    const rankedProtocols = getRankedResponseProtocols(accept, contentType);
+
+    const isEventStreamAccepted = rankedProtocols["text/event-stream"] !== -1;
+
+    const defaultSingleResponseHeaders = [
+      {
+        name: "Content-Type",
+        value: getSingleResponseContentType(rankedProtocols),
+      },
+    ];
 
     try {
       if (!isHttpMethod("GET", request.method) && !isHttpMethod("POST", request.method)) {
         throw new HttpError(405, "GraphQL only supports GET and POST requests.", {
-          headers: [{ name: "Allow", value: "GET, POST" }],
+          headers: [...defaultSingleResponseHeaders, { name: "Allow", value: "GET, POST" }],
         });
       }
 
       if (query == null) {
-        throw new HttpError(400, "Must provide query string.");
+        throw new HttpError(400, "Must provide query string.", { headers: defaultSingleResponseHeaders });
       }
 
       document = await parseQuery(query, parse);
@@ -108,7 +138,7 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
 
       if (operation.operation === "mutation" && isHttpMethod("GET", request.method)) {
         throw new HttpError(405, "Can only perform a mutation operation from a POST request.", {
-          headers: [{ name: "Allow", value: "POST" }],
+          headers: [...defaultSingleResponseHeaders, { name: "Allow", value: "POST" }],
         });
       }
 
@@ -119,7 +149,9 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
           variableValues = typeof variables === "string" ? JSON.parse(variables) : variables;
         }
       } catch (_error) {
-        throw new HttpError(400, "Variables are invalid JSON.");
+        throw new HttpError(400, "Variables are invalid JSON.", {
+          headers: defaultSingleResponseHeaders,
+        });
       }
 
       try {
@@ -133,6 +165,11 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
         rootValue = rootValueFactory ? await rootValueFactory(executionContext) : ({} as TRootValue);
 
         if (operation.operation === "subscription") {
+          if (!isHttpMethod("GET", request.method)) {
+            throw new HttpError(405, "Can only perform subscription operation from a GET request.", {
+              headers: [...defaultSingleResponseHeaders, { name: "Allow", value: "GET" }],
+            });
+          }
           const result = await subscribe({
             schema,
             document,
@@ -183,38 +220,22 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
                 stopAsyncIteration(result);
               },
             };
-          } else {
-            if (isEventStream) {
-              return {
-                type: "PUSH",
-                subscribe: async (onResult) => {
-                  onResult(
-                    formatPayload({
-                      payload: result,
-                      context,
-                      rootValue,
-                      document,
-                      operation,
-                    })
-                  );
-                },
-                unsubscribe: () => undefined,
-              };
-            } else {
-              return {
-                type: "RESPONSE",
-                payload: formatPayload({
+          }
+          return {
+            type: "PUSH",
+            subscribe: async (onResult) => {
+              onResult(
+                formatPayload({
                   payload: result,
                   context,
                   rootValue,
                   document,
                   operation,
-                }),
-                status: 200,
-                headers: [],
-              };
-            }
-          }
+                })
+              );
+            },
+            unsubscribe: () => undefined,
+          };
         } else {
           const result = await execute({
             schema,
@@ -229,7 +250,7 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
           // execution result.
           if (isAsyncIterable<ExecutionPatchResult>(result)) {
             return {
-              type: isEventStream ? "PUSH" : "MULTIPART_RESPONSE",
+              type: isHttpMethod("GET", request.method) ? "PUSH" : "MULTIPART_RESPONSE",
               subscribe: async (onResult) => {
                 for await (const payload of result) {
                   onResult(
@@ -251,7 +272,7 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
             return {
               type: "RESPONSE",
               status: 200,
-              headers: [],
+              headers: defaultSingleResponseHeaders,
               payload: formatPayload({
                 payload: result,
                 context,
@@ -266,12 +287,14 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
         if (executionError instanceof GraphQLError) {
           throw new HttpError(200, "GraphQLError encountered white executed GraphQL request.", {
             graphqlErrors: [executionError],
+            headers: defaultSingleResponseHeaders,
           });
         } else if (executionError instanceof HttpError) {
           throw executionError;
         } else {
           throw new HttpError(500, "Unexpected error encountered while executing GraphQL request.", {
             graphqlErrors: [new GraphQLError((executionError as Error).message)],
+            headers: defaultSingleResponseHeaders,
           });
         }
       }
@@ -280,7 +303,7 @@ export const processRequest = async <TContext = {}, TRootValue = {}>(
         errors: ((error as HttpError).graphqlErrors as GraphQLError[]) || [new GraphQLError((error as Error).message)],
       };
 
-      if (isEventStream) {
+      if (isEventStreamAccepted) {
         return {
           type: "PUSH",
           subscribe: async (onResult) => {

packages/core/lib/send-result/node-http.ts

@@ -16,10 +16,10 @@ export async function sendResponseResult(
   for (const { name, value } of responseResult.headers) {
     rawResponse.setHeader(name, value);
   }
-  const data = JSON.stringify(transformResult(responseResult.payload))
+  const data = JSON.stringify(transformResult(responseResult.payload));
   rawResponse.writeHead(responseResult.status, {
     "content-type": "application/json",
-    "content-length": calculateByteLength(data)
+    "content-length": calculateByteLength(data),
   });
   rawResponse.end(data);
 }
@@ -29,56 +29,68 @@ export async function sendMultipartResponseResult(
   rawResponse: RawResponse,
   transformResult: TransformResultFn = DEFAULT_TRANSFORM_RESULT_FN
 ): Promise<void> {
-  rawResponse.writeHead(200, {
+  /**
+   * TypeScript complains because of function call signature mismatches.
+   * The signatures, however, are identical, thus we cas this here to suppress warnings,
+   */
+  const response: ServerResponse = rawResponse as ServerResponse;
+  response.writeHead(200, {
     // prettier-ignore
     "Connection": "keep-alive",
     "Content-Type": 'multipart/mixed; boundary="-"',
     "Transfer-Encoding": "chunked",
   });
 
-  rawResponse.on("close", () => {
+  response.on("close", () => {
     multipartResult.unsubscribe();
   });
-  // @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
-  rawResponse.write("---");
+  response.write("---");
 
-  await multipartResult.subscribe(result => {
+  await multipartResult.subscribe((result) => {
     const chunk = JSON.stringify(transformResult(result));
-    const data = ["", "Content-Type: application/json; charset=utf-8", "Content-Length: " + calculateByteLength(chunk), "", chunk];
+    const data = [
+      "",
+      "Content-Type: application/json; charset=utf-8",
+      "Content-Length: " + calculateByteLength(chunk),
+      "",
+      chunk,
+    ];
 
     if (result.hasNext) {
       data.push("---");
     }
-    // @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
-    rawResponse.write(data.join("\r\n"));
+    response.write(data.join("\r\n"));
   });
 
-  // @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
-  rawResponse.write("\r\n-----\r\n");
-  rawResponse.end();
+  response.write("\r\n-----\r\n");
+  response.end();
 }
 
 export async function sendPushResult(
   pushResult: Push<any, any>,
   rawResponse: RawResponse,
   transformResult: TransformResultFn = DEFAULT_TRANSFORM_RESULT_FN
 ): Promise<void> {
-  rawResponse.writeHead(200, {
+  /**
+   * TypeScript complains because of function call signature mismatches.
+   * The signatures, however, are identical, thus we cas this here to suppress warnings,
+   */
+  const response: ServerResponse = rawResponse as ServerResponse;
+  response.writeHead(200, {
     "Content-Type": "text/event-stream",
     // prettier-ignore
     "Connection": "keep-alive",
     "Cache-Control": "no-cache",
   });
 
-  rawResponse.on("close", () => {
+  response.on("close", () => {
     pushResult.unsubscribe();
   });
 
   await pushResult.subscribe((result) => {
-    // @ts-expect-error - Different Signature between ServerResponse and Http2ServerResponse but still compatible.
- rawResponse.write(`data: ${JSON.stringify(transformResult(result))}\n\n`);   
+    response.write(`data: ${JSON.stringify(transformResult(result))}\n\n`);
   });
-  rawResponse.end();
+  response.end();
 }
 
 export async function sendResult(

packages/core/lib/util/get-ranked-response-protocols.spec.ts

@@ -0,0 +1,86 @@
+import { getRankedResponseProtocols } from "./get-ranked-response-protocols";
+
+it.each([
+  [
+    "application/json",
+    undefined,
+    {
+      "application/graphql+json": -1,
+      "application/json": 0,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    "application/graphql+json",
+    undefined,
+    {
+      "application/graphql+json": 0,
+      "application/json": -1,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    "application/json",
+    undefined,
+    {
+      "application/graphql+json": -1,
+      "application/json": 0,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    undefined,
+    "application/json",
+    {
+      "application/graphql+json": -1,
+      "application/json": 0,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    undefined,
+    "   application/json   ",
+    {
+      "application/graphql+json": -1,
+      "application/json": 0,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    "application/graphql+json, application/json",
+    "application/json",
+    {
+      "application/graphql+json": 0,
+      "application/json": 1,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    undefined,
+    "application/graphql+json",
+    {
+      "application/graphql+json": 0,
+      "application/json": -1,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+  [
+    "   application/graphql+json; encoding=foo  ,  application/json; encoding= what    ",
+    "application/graphql+json",
+    {
+      "application/graphql+json": 0,
+      "application/json": 1,
+      "text/event-stream": -1,
+      "multipart/mixed": -1,
+    },
+  ],
+])("getRankedResponseProtocols(%s, %s)", (acceptHeader, contentTypeHeader, rankingResult) => {
+  expect(getRankedResponseProtocols(acceptHeader, contentTypeHeader)).toEqual(rankingResult);
+});