SPEC/BUG: Ambiguity with null variable values and default values

This change corresponds to a spec proposal which solves an ambiguity in how variable values and default values behave with explicit null values. Otherwise, this ambiguity allows for null values to appear in non-null argument values, which may result in unforseen null-pointer-errors.

This appears in three distinct but related issues:

**VariablesInAllowedPosition validation rule**

The explicit value `null` may be used as a default value for a variable, however `VariablesInAllowedPositions` allowed a nullable type with a default value to flow into an argument expecting a non-null type. This validation rule must explicitly not allow `null` default values to flow in this manner.

**Coercing Variable Values**

coerceVariableValues allows the explicit `null` value to be used over a default value, which can result in flowing a null value to a non-null argument when paired with the validation rule mentioned above. Instead a default value must be used even when an explicit `null` value is provided.

**Coercing Argument Values**

coerceArgumentValues allows the explicit `null` default value to be used before checking for a non-null type. This could inadvertently allow a null value into a non-null typed argument.

Author: leebyron

src/execution/__tests__/nonnull-test.js

@@ -486,4 +486,192 @@ describe('Execute: handles non-nullable types', () => {
       ],
     },
   );
+
+  describe('Handles non-null argument', () => {
+    const schemaWithNonNullArg = new GraphQLSchema({
+      query: new GraphQLObjectType({
+        name: 'Query',
+        fields: {
+          withNonNullArg: {
+            type: GraphQLString,
+            args: {
+              cannotBeNull: {
+                type: GraphQLNonNull(GraphQLString),
+                defaultValue: null,
+              },
+            },
+            resolve: async (_, args) => {
+              if (typeof args.cannotBeNull === 'string') {
+                return 'Passed: ' + args.cannotBeNull;
+              }
+            },
+          },
+        },
+      }),
+    });
+
+    it('succeeds when passed non-null literal value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg (cannotBeNull: "literal value")
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: literal value',
+        },
+      });
+    });
+
+    it('succeeds when passed non-null variable value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String!) {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          testVar: 'variable value',
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: variable value',
+        },
+      });
+    });
+
+    it('succeeds when missing variable has default value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String = "default value") {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          // Intentionally missing variable
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: default value',
+        },
+      });
+    });
+
+    it('succeeds when null variable has default value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String = "default value") {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          testVar: null,
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: default value',
+        },
+      });
+    });
+
+    it('field error when missing non-null arg', async () => {
+      // Note: validation should identify this issue first (missing args rule)
+      // however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of required type "String!" was not provided.',
+            locations: [{ line: 3, column: 13 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+
+    it('field error when non-null arg provided null', async () => {
+      // Note: validation should identify this issue first (values of correct
+      // type rule) however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg(cannotBeNull: null)
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of non-null type "String!" must ' +
+              'not be null.',
+            locations: [{ line: 3, column: 42 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+
+    it('field error when non-null arg not provided variable value', async () => {
+      // Note: validation should identify this issue first (variables in allowed
+      // position rule) however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String) {
+            withNonNullArg(cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          // Intentionally missing variable
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of required type "String!" was ' +
+              'provided the variable "$testVar" which was not provided a ' +
+              'runtime value.',
+            locations: [{ line: 3, column: 42 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+  });
 });

src/execution/__tests__/variables-test.js

@@ -82,6 +82,16 @@ const TestType = new GraphQLObjectType({
       args: { input: { type: GraphQLString, defaultValue: 'Hello World' } },
       resolve: (_, { input }) => input && JSON.stringify(input),
     },
+    fieldWithNonNullableStringInputAndDefaultArgumentValue: {
+      type: GraphQLString,
+      args: {
+        input: {
+          type: GraphQLNonNull(GraphQLString),
+          defaultValue: 'Unreachable',
+        },
+      },
+      resolve: (_, { input }) => input && JSON.stringify(input),
+    },
     fieldWithNestedInputObject: {
       type: GraphQLString,
       args: {
@@ -264,6 +274,72 @@ describe('Execute: Handles inputs', () => {
         });
       });
 
+      it('does not use default value when provided', async () => {
+        const withDefaultsAST = parse(`
+          query q($input: String = "Default value") {
+            fieldWithNullableStringInput(input: $input)
+          }
+        `);
+
+        const result = await execute({
+          schema,
+          document: withDefaultsAST,
+          variableValues: {
+            input: 'Variable value',
+          },
+        });
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: '"Variable value"',
+          },
+        });
+      });
+
+      it('uses default value when explicit null value provided', async () => {
+        const withDefaultsAST = parse(`
+          query q($input: String = "Default value") {
+            fieldWithNullableStringInput(input: $input)
+          }
+        `);
+
+        const result = await execute({
+          schema,
+          document: withDefaultsAST,
+          variableValues: {
+            input: null,
+          },
+        });
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: '"Default value"',
+          },
+        });
+      });
+
+      it('uses null default value when not provided', async () => {
+        const withDefaultsAST = parse(`
+          query q($input: String = null) {
+            fieldWithNullableStringInput(input: $input)
+          }
+        `);
+
+        const result = await execute({
+          schema,
+          document: withDefaultsAST,
+          variableValues: {
+            // Intentionally missing value.
+          },
+        });
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: null,
+          },
+        });
+      });
+
       it('properly parses single value to list', async () => {
         const params = { input: { a: 'foo', b: 'bar', c: 'baz' } };
         const result = await execute(schema, ast, null, null, params);
@@ -534,8 +610,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$value" got invalid value null; ' +
-              'Expected non-nullable type String! not to be null.',
+              'Variable "$value" of required type "String!" was not provided.',
             locations: [{ line: 2, column: 31 }],
             path: undefined,
           },
@@ -733,8 +808,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String]! not to be null.',
+              'Variable "$input" of required type "[String]!" was not provided.',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
@@ -846,8 +920,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String!]! not to be null.',
+              'Variable "$input" of required type "[String!]!" was not provided.',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
@@ -985,5 +1058,26 @@ describe('Execute: Handles inputs', () => {
         ],
       });
     });
+
+    it('not when argument type is non-null', async () => {
+      const ast = parse(`query optionalVariable($optional: String) {
+        fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $optional)
+      }`);
+
+      expect(await execute(schema, ast)).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "input" of required type "String!" was provided the ' +
+              'variable "$optional" which was not provided a runtime value.',
+            locations: [{ line: 2, column: 71 }],
+            path: ['fieldWithNonNullableStringInputAndDefaultArgumentValue'],
+          },
+        ],
+      });
+    });
   });
 });