fix: GraphQL over HTTP compliance (v3) (#1685)

* integrate graphql-http tests * fail on warn too * useAccept * leak or whatever * utf-8 * event-stream * adapt tests useAccept adjustments and friends * respond with status 500 on thrown error without status code * accepting application/json uses always 200 * improved useCheckGraphQLQueryParams * omit data entry on pre-operation errors * adjust integration tests * bump graphql-http * Allow result processor plugin to extend acceptable media types (#1693) Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
dotansimha · Sep 5, 2022 · fd11462 · fd11462
1 parent d8554b1
commit fd11462
Show file tree

Hide file tree

Showing 27 changed files with 426 additions and 225 deletions.
diff --git a/examples/cloudflare-modules/__integration-tests__/index.spec.ts b/examples/cloudflare-modules/__integration-tests__/index.spec.ts
@@ -32,7 +32,7 @@ test('should succeeds introspection query', async () => {
   const response = await worker.fetch(request)
 
   expect(response.status).toBe(200)
-  expect(response.headers.get('content-type')).toBe('application/json')
+  expect(response.headers.get('content-type')).toContain('application/json')
   expect(await response.json()).toMatchObject({
     data: {
       __schema: {

diff --git a/examples/express/__integration-tests__/express.spec.ts b/examples/express/__integration-tests__/express.spec.ts
@@ -15,7 +15,9 @@ function getTests(app: Express.Application) {
       .post('/graphql')
       .send({ query: '{ hello }' })
     expect(response.statusCode).toBe(200)
-    expect(response.headers['content-type']).toContain('application/json')
+    expect(response.headers['content-type']).toContain(
+      'application/graphql-response+json',
+    )
     expect(response.body).toStrictEqual({
       data: {
         hello: 'world',
@@ -38,7 +40,9 @@ function getTests(app: Express.Application) {
         contentType: 'plain/text',
       })
     expect(response.statusCode).toBe(200)
-    expect(response.headers['content-type']).toContain('application/json')
+    expect(response.headers['content-type']).toContain(
+      'application/graphql-response+json',
+    )
     expect(response.body).toStrictEqual({
       data: {
         getFileName: 'file.txt',

diff --git a/examples/fastify/__integration-tests__/fastify.spec.ts b/examples/fastify/__integration-tests__/fastify.spec.ts
@@ -222,7 +222,9 @@ describe('fastify example integration', () => {
         contentType: 'plain/text',
       })
     expect(response.statusCode).toBe(200)
-    expect(response.headers['content-type']).toContain('application/json')
+    expect(response.headers['content-type']).toContain(
+      'application/graphql-response+json',
+    )
     expect(response.body).toStrictEqual({
       data: {
         getFileName: 'file.txt',

diff --git a/examples/koa/__integration-tests__/koa.spec.ts b/examples/koa/__integration-tests__/koa.spec.ts
@@ -15,7 +15,9 @@ describe('koa', () => {
       .post('/graphql')
       .send({ query: '{ hello }' })
     expect(response.statusCode).toBe(200)
-    expect(response.headers['content-type']).toContain('application/json')
+    expect(response.headers['content-type']).toContain(
+      'application/graphql-response+json',
+    )
     expect(response.body).toStrictEqual({
       data: {
         hello: 'world',
@@ -27,7 +29,9 @@ describe('koa', () => {
       .post('/graphql')
       .send({ query: '{ isKoa }' })
     expect(response.statusCode).toBe(200)
-    expect(response.headers['content-type']).toContain('application/json')
+    expect(response.headers['content-type']).toContain(
+      'application/graphql-response+json',
+    )
     expect(response.body).toStrictEqual({
       data: {
         isKoa: true,

diff --git a/examples/service-worker/__integration-tests__/integration.spec.ts b/examples/service-worker/__integration-tests__/integration.spec.ts
@@ -111,7 +111,7 @@ describe('Service worker', () => {
     })
 
     expect(response.status).toBe(200)
-    expect(response.headers.get('content-type')).toBe('application/json')
+    expect(response.headers.get('content-type')).toContain('application/json')
     expect(await response.json()).toMatchObject({
       data: {
         __schema: {

diff --git a/packages/graphql-yoga/__tests__/accept-header.spec.ts b/packages/graphql-yoga/__tests__/accept-header.spec.ts
@@ -18,6 +18,7 @@ describe('accept header', () => {
         accept: 'text/event-stream',
       },
     })
+
     expect(response.headers.get('content-type')).toEqual('text/event-stream')
     const valueStr = await response.text()
     expect(valueStr).toContain(
@@ -225,7 +226,7 @@ describe('accept header', () => {
       },
     })
     expect(response.headers.get('content-type')).toEqual(
-      'application/graphql-response+json',
+      'application/graphql-response+json; charset=utf-8',
     )
     const result = await response.json()
     expect(result).toEqual({ data: { ping: 'pong' } })
@@ -251,8 +252,9 @@ describe('accept header', () => {
           'application/graphql-response+json; charset=utf-8, application/json; charset=utf-8',
       },
     })
+
     expect(response.headers.get('content-type')).toEqual(
-      'application/graphql-response+json',
+      'application/graphql-response+json; charset=utf-8',
     )
     const result = await response.json()
     expect(result).toEqual({ data: { ping: 'pong' } })

diff --git a/packages/graphql-yoga/__tests__/error-masking.spec.ts b/packages/graphql-yoga/__tests__/error-masking.spec.ts
@@ -186,15 +186,14 @@ describe('error masking', () => {
     })
     const body = JSON.parse(await response.text())
     expect(body).toMatchInlineSnapshot(`
-            Object {
-              "data": null,
-              "errors": Array [
-                Object {
-                  "message": "I like turtles",
-                },
-              ],
-            }
-          `)
+      Object {
+        "errors": Array [
+          Object {
+            "message": "I like turtles",
+          },
+        ],
+      }
+    `)
   })
 
   it('error thrown within context factory is masked', async () => {
@@ -213,15 +212,14 @@ describe('error masking', () => {
     })
     const body = JSON.parse(await response.text())
     expect(body).toMatchInlineSnapshot(`
-            Object {
-              "data": null,
-              "errors": Array [
-                Object {
-                  "message": "Unexpected error.",
-                },
-              ],
-            }
-          `)
+      Object {
+        "errors": Array [
+          Object {
+            "message": "Unexpected error.",
+          },
+        ],
+      }
+    `)
   })
 
   it('GraphQLError thrown within context factory with error masking is not masked', async () => {
@@ -240,15 +238,14 @@ describe('error masking', () => {
     })
     const body = JSON.parse(await response.text())
     expect(body).toMatchInlineSnapshot(`
-            Object {
-              "data": null,
-              "errors": Array [
-                Object {
-                  "message": "I like turtles",
-                },
-              ],
-            }
-          `)
+      Object {
+        "errors": Array [
+          Object {
+            "message": "I like turtles",
+          },
+        ],
+      }
+    `)
   })
 
   it('GraphQLError thrown within context factory has error extensions exposed on the response', async () => {
@@ -276,18 +273,19 @@ describe('error masking', () => {
       body: JSON.stringify({ query: '{ greetings }' }),
     })
     const body = JSON.parse(await response.text())
-    expect(body).toStrictEqual({
-      data: null,
-      errors: [
-        {
-          message: 'I like turtles',
-          extensions: {
-            foo: 1,
+    expect(body).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          Object {
+            "extensions": Object {
+              "foo": 1,
+            },
+            "message": "I like turtles",
           },
-        },
-      ],
-    })
-    expect(response.status).toEqual(200)
+        ],
+      }
+    `)
+    expect(response.status).toEqual(500)
   })
 
   it('parse error is not masked', async () => {
@@ -313,20 +311,21 @@ describe('error masking', () => {
     expect(response.status).toEqual(400)
     const body = JSON.parse(await response.text())
 
-    expect(body).toEqual({
-      data: null,
-      errors: [
-        {
-          locations: [
-            {
-              column: 10,
-              line: 1,
-            },
-          ],
-          message: 'Syntax Error: Expected Name, found <EOF>.',
-        },
-      ],
-    })
+    expect(body).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          Object {
+            "locations": Array [
+              Object {
+                "column": 10,
+                "line": 1,
+              },
+            ],
+            "message": "Syntax Error: Expected Name, found <EOF>.",
+          },
+        ],
+      }
+    `)
   })
 
   it('validation error is not masked', async () => {
@@ -352,20 +351,20 @@ describe('error masking', () => {
     expect(response.status).toEqual(400)
     const body = JSON.parse(await response.text())
 
-    expect(body).toEqual({
-      data: null,
-      errors: [
-        {
-          locations: [
-            {
-              column: 2,
-              line: 1,
-            },
-          ],
-
-          message: 'Cannot query field "libl_pls" on type "Query".',
-        },
-      ],
-    })
+    expect(body).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          Object {
+            "locations": Array [
+              Object {
+                "column": 2,
+                "line": 1,
+              },
+            ],
+            "message": "Cannot query field \\"libl_pls\\" on type \\"Query\\".",
+          },
+        ],
+      }
+    `)
   })
 })
diff --git a/packages/graphql-yoga/__tests__/graphql-http.spec.ts b/packages/graphql-yoga/__tests__/graphql-http.spec.ts
@@ -0,0 +1,24 @@
+import { createSchema, createYoga } from 'graphql-yoga'
+import { serverAudits } from 'graphql-http'
+
+const yoga = createYoga({
+  schema: createSchema({
+    typeDefs: /* GraphQL */ `
+      type Query {
+        _: String
+      }
+    `,
+  }),
+})
+
+for (const audit of serverAudits({
+  url: 'http://yoga/graphql',
+  fetchFn: yoga.fetch,
+})) {
+  test(audit.name, async () => {
+    const result = await audit.fn()
+    if (result.status !== 'ok') {
+      throw result.reason
+    }
+  })
+}
diff --git a/packages/graphql-yoga/__tests__/http-extensions.spec.ts b/packages/graphql-yoga/__tests__/http-extensions.spec.ts
@@ -246,4 +246,38 @@ describe('GraphQLError.extensions.http', () => {
     })
     expect(response.status).toBe(400)
   })
+
+  it('should respond with status 500 when error without http extension is thrown', async () => {
+    const yoga = createYoga({
+      schema: {
+        typeDefs: /* GraphQL */ `
+          type Query {
+            _: String
+          }
+        `,
+      },
+      context: () => {
+        throw new GraphQLError('No http status extension', {
+          extensions: { http: { headers: { 'x-foo': 'bar' } } },
+        })
+      },
+    })
+
+    const response = await yoga.fetch('http://yoga/graphql', {
+      method: 'POST',
+      headers: { 'content-type': 'application/json' },
+      body: JSON.stringify({ query: '{ __typename }' }),
+    })
+    expect(response.status).toBe(500)
+    expect(response.headers.get('x-foo')).toBe('bar')
+    expect(await response.json()).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          Object {
+            "message": "No http status extension",
+          },
+        ],
+      }
+    `)
+  })
 })
diff --git a/packages/graphql-yoga/__tests__/introspection.spec.ts b/packages/graphql-yoga/__tests__/introspection.spec.ts
@@ -51,9 +51,11 @@ describe('introspection', () => {
     })
 
     expect(response.status).toBe(400)
-    expect(response.headers.get('content-type')).toBe('application/json')
+    expect(response.headers.get('content-type')).toBe(
+      'application/graphql-response+json; charset=utf-8',
+    )
     const body = await response.json()
-    expect(body.data).toBeNull()
+    expect(body.data).toBeUndefined()
     expect(body.errors![0]).toMatchInlineSnapshot(`
             Object {
               "locations": Array [