Merge pull request #18 from graphql/safe-get

Disable GET for non-query operations

Author: leebyron

README.md

@@ -67,7 +67,7 @@ If not found in the query-string, it will look in the POST request body.
 If a previous middleware has already parsed the POST body, the `request.body`
 value will be used. Use [`multer`][] or a similar middleware to add support
 for `multipart/form-data` content, which may be useful for GraphQL mutations
-involving uploading files. See an [example using multer](https://github.com/graphql/express-graphql/blob/master/src/__tests__/http-test.js#L462).
+involving uploading files. See an [example using multer](https://github.com/graphql/express-graphql/blob/master/src/__tests__/http-test.js#L603).
 
 If the POST body has not yet been parsed, graphql-express will interpret it
 depending on the provided *Content-Type* header.

package.json

@@ -51,7 +51,7 @@
     "raw-body": "~2.1.2"
   },
   "peerDependencies": {
-    "graphql": "~0.4.2"
+    "graphql": "~0.4.5"
   },
   "devDependencies": {
     "babel": "5.8.21",
@@ -66,7 +66,7 @@
     "express": "4.13.3",
     "express3": "*",
     "flow-bin": "0.14.0",
-    "graphql": "0.4.2",
+    "graphql": "0.4.5",
     "isparta": "3.0.3",
     "mocha": "2.2.5",
     "multer": "1.0.3",

src/__tests__/http-test.js

@@ -28,23 +28,33 @@ import {
 } from 'graphql';
 import graphqlHTTP from '../';
 
+var QueryRootType = new GraphQLObjectType({
+  name: 'QueryRoot',
+  fields: {
+    test: {
+      type: GraphQLString,
+      args: {
+        who: {
+          type: GraphQLString
+        }
+      },
+      resolve: (root, { who }) => 'Hello ' + (who || 'World')
+    },
+    thrower: {
+      type: new GraphQLNonNull(GraphQLString),
+      resolve: () => { throw new Error('Throws!'); }
+    }
+  }
+});
 
 var TestSchema = new GraphQLSchema({
-  query: new GraphQLObjectType({
-    name: 'Root',
+  query: QueryRootType,
+  mutation: new GraphQLObjectType({
+    name: 'MutationRoot',
     fields: {
-      test: {
-        type: GraphQLString,
-        args: {
-          who: {
-            type: GraphQLString
-          }
-        },
-        resolve: (root, { who }) => 'Hello ' + (who || 'World')
-      },
-      thrower: {
-        type: new GraphQLNonNull(GraphQLString),
-        resolve: () => { throw new Error('Throws!'); }
+      writeTest: {
+        type: QueryRootType,
+        resolve: () => ({})
       }
     }
   })
@@ -168,7 +178,7 @@ describe('test harness', () => {
               query helloYou { test(who: "You"), ...shared }
               query helloWorld { test(who: "World"), ...shared }
               query helloDolly { test(who: "Dolly"), ...shared }
-              fragment shared on Root {
+              fragment shared on QueryRoot {
                 shared: test(who: "Everyone")
               }
             `,
@@ -183,6 +193,122 @@ describe('test harness', () => {
         });
       });
 
+      it('Reports validation errors', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var error = await catchError(
+          request(app)
+            .get(urlString({
+              query: `{ test, unknownOne, unknownTwo }`
+            }))
+        );
+
+        expect(error.response.status).to.equal(400);
+        expect(JSON.parse(error.response.text)).to.deep.equal({
+          errors: [
+            {
+              message: 'Cannot query field "unknownOne" on "QueryRoot".',
+              locations: [ { line: 1, column: 9 } ]
+            },
+            {
+              message: 'Cannot query field "unknownTwo" on "QueryRoot".',
+              locations: [ { line: 1, column: 21 } ]
+            }
+          ]
+        });
+      });
+
+      it('Errors when missing operation name', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var error = await catchError(
+          request(app)
+            .get(urlString({
+              query: `
+                query TestQuery { test }
+                mutation TestMutation { writeTest { test } }
+              `
+            }))
+        );
+
+        expect(error.response.status).to.equal(400);
+        expect(JSON.parse(error.response.text)).to.deep.equal({
+          errors: [
+            { message: 'Must provide operation name if query contains multiple operations.' }
+          ]
+        });
+      });
+
+      it('Errors when sending a mutation via GET', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var error = await catchError(
+          request(app)
+            .get(urlString({
+              query: 'mutation TestMutation { writeTest { test } }'
+            }))
+        );
+
+        expect(error.response.status).to.equal(405);
+        expect(JSON.parse(error.response.text)).to.deep.equal({
+          errors: [
+            { message: 'Can only perform a mutation operation from a POST request.' }
+          ]
+        });
+      });
+
+      it('Errors when selecting a mutation within a GET', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var error = await catchError(
+          request(app)
+            .get(urlString({
+              operationName: 'TestMutation',
+              query: `
+                query TestQuery { test }
+                mutation TestMutation { writeTest { test } }
+              `
+            }))
+        );
+
+        expect(error.response.status).to.equal(405);
+        expect(JSON.parse(error.response.text)).to.deep.equal({
+          errors: [
+            { message: 'Can only perform a mutation operation from a POST request.' }
+          ]
+        });
+      });
+
+      it('Allows a mutation to exist within a GET', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var response = await request(app)
+          .get(urlString({
+            operationName: 'TestQuery',
+            query: `
+              mutation TestMutation { writeTest { test } }
+              query TestQuery { test }
+            `
+          }));
+
+        expect(response.status).to.equal(200);
+        expect(JSON.parse(response.text)).to.deep.equal({
+          data: {
+            test: 'Hello World'
+          }
+        });
+      });
+
     });
 
     describe('POST functionality', () => {
@@ -201,6 +327,21 @@ describe('test harness', () => {
         );
       });
 
+      it('Allows sending a mutation via POST', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({ schema: TestSchema }));
+
+        var response = await request(app)
+          .post(urlString())
+          .send({ query: 'mutation TestMutation { writeTest { test } }' });
+
+        expect(response.status).to.equal(200);
+        expect(response.text).to.equal(
+          '{"data":{"writeTest":{"test":"Hello World"}}}'
+        );
+      });
+
       it('allows POST with url encoding', async () => {
         var app = express();
 
@@ -345,7 +486,7 @@ describe('test harness', () => {
               query helloYou { test(who: "You"), ...shared }
               query helloWorld { test(who: "World"), ...shared }
               query helloDolly { test(who: "Dolly"), ...shared }
-              fragment shared on Root {
+              fragment shared on QueryRoot {
                 shared: test(who: "Everyone")
               }
             `,
@@ -376,7 +517,7 @@ describe('test harness', () => {
             query helloYou { test(who: "You"), ...shared }
             query helloWorld { test(who: "World"), ...shared }
             query helloDolly { test(who: "Dolly"), ...shared }
-            fragment shared on Root {
+            fragment shared on QueryRoot {
               shared: test(who: "Everyone")
             }
           `);
@@ -978,6 +1119,28 @@ describe('test harness', () => {
         expect(response.text).to.include('response: null');
       });
 
+      it('GraphiQL accepts a mutation query - does not execute it', async () => {
+        var app = express();
+
+        app.use(urlString(), graphqlHTTP({
+          schema: TestSchema,
+          graphiql: true
+        }));
+
+        var response = await request(app)
+          .get(urlString({
+            query: 'mutation TestMutation { writeTest { test } }'
+          }))
+          .set('Accept', 'text/html');
+
+        expect(response.status).to.equal(200);
+        expect(response.type).to.equal('text/html');
+        expect(response.text).to.include(
+          'query: "mutation TestMutation { writeTest { test } }"'
+        );
+        expect(response.text).to.include('response: null');
+      });
+
       it('returns HTML if preferred', async () => {
         var app = express();
 

src/index.js

@@ -9,8 +9,11 @@
  */
 
 import httpError from 'http-errors';
-import { graphql } from 'graphql';
 import { formatError } from 'graphql/error';
+import { execute } from 'graphql/execution';
+import { parse, Source } from 'graphql/language';
+import { validate } from 'graphql/validation';
+import { getOperationAST } from 'graphql/utilities/getOperationAST';
 import { parseBody } from './parseBody';
 import { renderGraphiQL } from './renderGraphiQL';
 import type { Request, Response } from 'express';
@@ -68,11 +71,11 @@ export default function graphqlHTTP(options: Options): Middleware {
     }
 
     // Parse the Request body.
-    parseBody(request, (error, data = {}) => {
+    parseBody(request, (parseError, data = {}) => {
 
       // Format any request errors the same as GraphQL errors.
-      if (error) {
-        return sendError(response, error, pretty);
+      if (parseError) {
+        return sendError(response, parseError, pretty);
       }
 
       // Get GraphQL params from the request and POST body data.
@@ -90,13 +93,56 @@ export default function graphqlHTTP(options: Options): Middleware {
       }
 
       // Run GraphQL query.
-      graphql(
-        schema,
-        query,
-        rootValue,
-        variables,
-        operationName
-      ).then(result => {
+      new Promise(resolve => {
+        var source = new Source(query, 'GraphQL request');
+        var documentAST = parse(source);
+        var validationErrors = validate(schema, documentAST);
+        if (validationErrors.length > 0) {
+          resolve({ errors: validationErrors });
+        } else {
+
+          // Only query operations are allowed on GET requests.
+          if (request.method === 'GET') {
+            // Determine if this GET request will perform a non-query.
+            var operationAST = getOperationAST(documentAST, operationName);
+            if (operationAST && operationAST.operation !== 'query') {
+              // If GraphiQL can be shown, do not perform this query, but
+              // provide it to GraphiQL so that the requester may perform it
+              // themselves if desired.
+              if (graphiql && canDisplayGraphiQL(request, data)) {
+                return response
+                  .set('Content-Type', 'text/html')
+                  .send(renderGraphiQL({ query, variables }));
+              }
+
+              // Otherwise, report a 405 Method Not Allowed error.
+              response.set('Allow', 'POST');
+              return sendError(
+                response,
+                httpError(
+                  405,
+                  `Can only perform a ${operationAST.operation} operation ` +
+                  `from a POST request.`
+                ),
+                pretty
+              );
+            }
+          }
+
+          // Perform the execution.
+          resolve(
+            execute(
+              schema,
+              documentAST,
+              rootValue,
+              variables,
+              operationName
+            )
+          );
+        }
+      }).catch(error => {
+        return { errors: [ error ] };
+      }).then(result => {
 
         // Format any encountered errors.
         if (result.errors) {

src/renderGraphiQL.js

@@ -8,7 +8,7 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
-type GraphiQLData = { query: ?string, variables: ?Object, result: Object };
+type GraphiQLData = { query: ?string, variables: ?Object, result?: Object };
 
 // Current latest version of GraphiQL.
 var GRAPHIQL_VERSION = '0.2.4';