From 979ac397f12e7d7e877ce4d0ed09c0e60e7fdaab Mon Sep 17 00:00:00 2001
From: Vamshi Surabhi <vamshi@hasura.io>
Date: Tue, 23 Apr 2019 18:11:27 +0530
Subject: [PATCH] fix validation of null values, closes #1981

---
 .../Hasura/GraphQL/Validate/InputValue.hs     |  6 +++++-
 .../graphql_validation/null_value_err.yaml    | 19 +++++++++++++++++
 .../null_variable_value_err.yaml              | 21 +++++++++++++++++++
 .../queries/graphql_validation/setup.yaml     | 15 +++++++++++++
 .../queries/graphql_validation/teardown.yaml  |  6 ++++++
 server/tests-py/super_classes.py              |  8 +++++--
 server/tests-py/test_validation.py            | 20 ++++++++++++++++++
 7 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 server/tests-py/queries/graphql_validation/null_value_err.yaml
 create mode 100644 server/tests-py/queries/graphql_validation/null_variable_value_err.yaml
 create mode 100644 server/tests-py/queries/graphql_validation/setup.yaml
 create mode 100644 server/tests-py/queries/graphql_validation/teardown.yaml
 create mode 100644 server/tests-py/test_validation.py

diff --git a/server/src-lib/Hasura/GraphQL/Validate/InputValue.hs b/server/src-lib/Hasura/GraphQL/Validate/InputValue.hs
index 02599b691e98f..a08858c28c04e 100644
--- a/server/src-lib/Hasura/GraphQL/Validate/InputValue.hs
+++ b/server/src-lib/Hasura/GraphQL/Validate/InputValue.hs
@@ -303,7 +303,11 @@ withParsed
 withParsed expectedTy valParser val fn = do
   parsedVal <- valParser val
   case unP parsedVal of
-    Nothing              -> AnnInpVal expectedTy Nothing <$> fn Nothing
+    Nothing              ->
+      if G.isNullable expectedTy
+      then AnnInpVal expectedTy Nothing <$> fn Nothing
+      else throwVE $ "null value found for non-nullable type: "
+           <> G.showGT expectedTy
     Just (Right v)       -> AnnInpVal expectedTy Nothing <$> fn (Just v)
     Just (Left (var, v)) -> do
       let varTxt = G.unName $ G.unVariable var
diff --git a/server/tests-py/queries/graphql_validation/null_value_err.yaml b/server/tests-py/queries/graphql_validation/null_value_err.yaml
new file mode 100644
index 0000000000000..b5bf0b604496a
--- /dev/null
+++ b/server/tests-py/queries/graphql_validation/null_value_err.yaml
@@ -0,0 +1,19 @@
+description: Passing null value for non nullable type
+url: /v1alpha1/graphql
+status: 400
+response:
+  errors:
+  - extensions:
+      path: "$.selectionSet.update_author.args.where"
+      code: "validation-failed"
+    message: "null value found for non-nullable type: author_bool_exp!"
+query:
+  query: |
+    mutation update_author {
+      update_author(where: null _set: {name: ""}) {
+        returning {
+          id
+          name
+        }
+      }
+    }
diff --git a/server/tests-py/queries/graphql_validation/null_variable_value_err.yaml b/server/tests-py/queries/graphql_validation/null_variable_value_err.yaml
new file mode 100644
index 0000000000000..6274df2c5a582
--- /dev/null
+++ b/server/tests-py/queries/graphql_validation/null_variable_value_err.yaml
@@ -0,0 +1,21 @@
+description: Passing null value for non nullable type
+url: /v1alpha1/graphql
+status: 400
+response:
+  errors:
+  - extensions:
+      path: "$.variableValues.author_id"
+      code: "validation-failed"
+    message: "null value found for non-nullable type: Int!"
+query:
+  variables:
+    author_id: null
+  query: |
+    mutation update_author($author_id: Int!) {
+      update_author(where: {id: {_eq: $author_id}}, _set: {name: ""}) {
+        returning {
+          id
+          name
+        }
+      }
+    }
diff --git a/server/tests-py/queries/graphql_validation/setup.yaml b/server/tests-py/queries/graphql_validation/setup.yaml
new file mode 100644
index 0000000000000..dd084a96828c4
--- /dev/null
+++ b/server/tests-py/queries/graphql_validation/setup.yaml
@@ -0,0 +1,15 @@
+type: bulk
+args:
+
+#Author table
+- type: run_sql
+  args:
+    sql: |
+      create table author(
+          id serial primary key,
+          name text unique
+      );
+- type: track_table
+  args:
+    schema: public
+    name: author
diff --git a/server/tests-py/queries/graphql_validation/teardown.yaml b/server/tests-py/queries/graphql_validation/teardown.yaml
new file mode 100644
index 0000000000000..b112569f5e2c1
--- /dev/null
+++ b/server/tests-py/queries/graphql_validation/teardown.yaml
@@ -0,0 +1,6 @@
+type: bulk
+args:
+- type: run_sql
+  args:
+    sql: |
+      drop table author
diff --git a/server/tests-py/super_classes.py b/server/tests-py/super_classes.py
index c68291221a76c..d2e7927e2bd28 100644
--- a/server/tests-py/super_classes.py
+++ b/server/tests-py/super_classes.py
@@ -51,8 +51,9 @@ def dir(self):
         pass
 
 
-
-class DefaultTestSelectQueries(ABC):
+# Any test which has a setup and a teardown
+# Ideally, DefaultTestSelectQueries should just be this
+class GraphQLEngineTest(ABC):
 
     @pytest.fixture(scope='class')
     def transact(self, request, hge_ctx):
@@ -69,3 +70,6 @@ def ensure_transact(self, transact):
     @abstractmethod
     def dir(self):
         pass
+
+class DefaultTestSelectQueries(GraphQLEngineTest):
+    pass
diff --git a/server/tests-py/test_validation.py b/server/tests-py/test_validation.py
new file mode 100644
index 0000000000000..ed32bdf6e6d05
--- /dev/null
+++ b/server/tests-py/test_validation.py
@@ -0,0 +1,20 @@
+import pytest
+import yaml
+from validate import check_query_f
+from super_classes import GraphQLEngineTest
+
+# @pytest.mark.parametrize("transport", ['http','websocket'])
+# graphql parser can't seem to parse {where: null}, disabling
+# websocket till then
+@pytest.mark.parametrize("transport", ['http'])
+class TestGraphQLValidation(GraphQLEngineTest):
+
+    def test_null_value(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + "/null_value_err.yaml", transport)
+
+    def test_null_variable_value(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + "/null_variable_value_err.yaml", transport)
+
+    @classmethod
+    def dir(cls):
+        return "queries/graphql_validation"