fix(gate): explicit null on query arg (#453)

Solves MET-268 + fixes an edgecase for "weak validation"
metatypedev · Oct 19, 2023 · 2ca1a92 · 2ca1a92
1 parent 3c5b117
commit 2ca1a92
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 10 deletions.
diff --git a/typegate/src/engine/planner/args.ts b/typegate/src/engine/planner/args.ts
@@ -167,7 +167,8 @@ interface Dependencies {
  *      array_arg: [  # o -> collectArg() -> collectArrayArg()
  *          'hello',  # -> collectArg()
  *          'world',  # -> collectArg()
- *      ]
+ *      ],
+ *      explicit_null_arg: null,  # o -> collectArg() => ((_) => null)
  *  ) {
  *      selection1
  *     selection2
@@ -242,7 +243,7 @@ class ArgumentCollector {
       // fallthrough: the user provided value
     }
 
-    // in case the argument node of the query is null,
+    // in case the argument node of the query is not defined
     // try to get a default value for it, else throw an error
     if (astNode == null) {
       if (typ.type === Type.OPTIONAL) {
@@ -274,6 +275,13 @@ class ArgumentCollector {
       return ({ variables: vars }) => vars[varName];
     }
 
+    // Note: this occurs when the graphql query arg has an *explicit* null value
+    // func( .., node: null, ..) { .. }
+    // https://spec.graphql.org/June2018/#sec-Null-Value
+    if (valueNode.kind === Kind.NULL) {
+      return (_args) => null;
+    }
+
     switch (typ.type) {
       case Type.OBJECT: {
         return this.collectObjectArg(valueNode, typ);

diff --git a/typegate/src/engine/typecheck/input.ts b/typegate/src/engine/typecheck/input.ts
@@ -59,7 +59,7 @@ export function generateWeakValidator(
   switch (node.type) {
     case "object":
       return (value: unknown) => {
-        const filtered = filterDeclaredFields(tg, value, node, {});
+        const filtered = filterDeclaredFields(tg, value, node);
         validator(filtered);
       };
     case "optional":
@@ -74,28 +74,29 @@ function filterDeclaredFields(
   tg: TypeGraph,
   value: any,
   node: TypeNode,
-  result: Record<string, unknown>,
 ): unknown {
   switch (node.type) {
     case "object": {
       const explicitlyDeclared = Object.entries(node.properties);
+      const result = {} as Record<string, unknown>;
       for (const [field, idx] of explicitlyDeclared) {
         const nextNode = tg.type(idx);
         result[field] = filterDeclaredFields(
           tg,
-          value[field],
+          value?.[field],
           nextNode,
-          {},
         );
       }
       return result;
     }
     case "optional":
+      if (value === undefined || value === null) {
+        return null;
+      }
       return filterDeclaredFields(
         tg,
         value,
         tg.type(node.item),
-        {},
       );
     default:
       return value;

diff --git a/typegate/tests/typecheck/__snapshots__/typecheck_test.ts.snap b/typegate/tests/typecheck/__snapshots__/typecheck_test.ts.snap
@@ -1,7 +1,7 @@
 export const snapshot = {};
 
 snapshot[`typecheck 1`] = `
-"function validate_53_1(value, path, errors, context) {
+"function validate_62_1(value, path, errors, context) {
   if (typeof value !== \\"object\\") {
     errors.push([path, \`expected an object, got \${typeof value}\`]);
   } else if (value == null) {
@@ -112,7 +112,7 @@ function validate_4(value, path, errors, context) {
     }
   }
 }
-return validate_53_1;
+return validate_62_1;
 "
 `;
 

diff --git a/typegate/tests/typecheck/typecheck.py b/typegate/tests/typecheck/typecheck.py
@@ -92,11 +92,23 @@ def typecheck(g: Graph):
         }
     )
 
+    product = t.struct(
+        {
+            "name": t.string(),
+            "equivalent": t.array(t.ref("Product")).optional(),
+            "score": t.either(
+                [t.string(enum=["bad", "decent", "good"]), t.integer()]
+            ).optional(),
+        },
+        name="Product",
+    )
+
     g.expose(
         my_policy,
         createUser=create_user,
         posts=posts,
         findPost=find_post,
         createPost=create_post,
         enums=deno.identity(enums),
+        findProduct=deno.identity(product),
     )
diff --git a/typegate/tests/typecheck/typecheck_test.ts b/typegate/tests/typecheck/typecheck_test.ts
@@ -1,7 +1,7 @@
 // Copyright Metatype OÜ, licensed under the Elastic License 2.0.
 // SPDX-License-Identifier: Elastic-2.0
 
-import { Meta } from "../utils/mod.ts";
+import { gql, Meta } from "../utils/mod.ts";
 import { assertThrows } from "std/assert/mod.ts";
 import { findOperation } from "../../src/transports/graphql/graphql.ts";
 import { parse } from "graphql";
@@ -200,4 +200,40 @@ Meta.test("typecheck", async (t) => {
       }],
     });
   });
+
+  await t.should("accept explicit null value", async () => {
+    await gql`
+      query {
+        findProduct(
+            name: "A"
+            equivalent: [
+              { name: "B", equivalent: null },
+              { name: "C", score: null },
+              { name: "D", score: 10 },
+            ],
+            score: null
+        ) {
+          name
+          equivalent {
+            name
+            equivalent { name }
+            score
+          }
+          score
+        }
+      }
+    `
+      .expectData({
+        findProduct: {
+          name: "A",
+          equivalent: [
+            { name: "B" },
+            { name: "C" },
+            { name: "D", score: 10 },
+          ],
+          score: null,
+        },
+      })
+      .on(e);
+  });
 });