Fix prototype-colliding names in execution values

Author: abishekgiri

src/execution/__tests__/variables-test.ts

@@ -129,6 +129,15 @@ const TestType = new GraphQLObjectType({
       type: TestNestedInputObject,
       defaultValue: 'Hello World',
     }),
+    fieldWithPrototypeNamedArgument: {
+      type: GraphQLString,
+      args: {
+        toString: { type: GraphQLString },
+      },
+      resolve(_, args) {
+        return args.toString === undefined ? 'missing' : inspect(args.toString);
+      },
+    },
     list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
     nnList: fieldWithInputArg({
       type: new GraphQLNonNull(new GraphQLList(GraphQLString)),
@@ -1064,6 +1073,20 @@ describe('Execute: Handles inputs', () => {
         },
       });
     });
+
+    it('does not expose prototype argument names when omitted', () => {
+      const result = executeQuery(`
+        {
+          fieldWithPrototypeNamedArgument
+        }
+      `);
+
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithPrototypeNamedArgument: 'missing',
+        },
+      });
+    });
   });
 
   describe('getVariableValues: limit maximum number of coercion errors', () => {
@@ -1136,4 +1159,45 @@ describe('Execute: Handles inputs', () => {
       });
     });
   });
+
+  describe('getVariableValues: own-property names', () => {
+    const doc = parse(`
+      query ($toString: String) {
+        fieldWithNullableStringInput(input: $toString)
+      }
+    `);
+
+    const operation = doc.definitions[0];
+    invariant(operation.kind === Kind.OPERATION_DEFINITION);
+    const { variableDefinitions } = operation;
+    invariant(variableDefinitions != null);
+
+    it('does not expose prototype variable names when omitted', () => {
+      const result = getVariableValues(schema, variableDefinitions, {});
+      if (!('coerced' in result)) {
+        throw new Error('Expected coerced variable values.');
+      }
+      const coerced = result.coerced as { toString?: unknown } | undefined;
+      if (coerced == null) {
+        throw new Error('Expected coerced variable values.');
+      }
+
+      expect(coerced.toString).to.equal(undefined);
+    });
+
+    it('still returns provided variables with colliding names', () => {
+      const result = getVariableValues(schema, variableDefinitions, {
+        toString: 'value',
+      });
+      if (!('coerced' in result)) {
+        throw new Error('Expected coerced variable values.');
+      }
+      const coerced = result.coerced as { toString?: unknown } | undefined;
+      if (coerced == null) {
+        throw new Error('Expected coerced variable values.');
+      }
+
+      expect(coerced.toString).to.equal('value');
+    });
+  });
 });

src/execution/values.ts

@@ -32,9 +32,8 @@ type CoercedVariableValues =
  * provided variable definitions and arbitrary input. If the input cannot be
  * parsed to match the variable definitions, a GraphQLError will be thrown.
  *
- * Note: The returned value is a plain Object with a prototype, since it is
- * exposed to user code. Care should be taken to not pull values from the
- * Object prototype.
+ * Note: The returned value uses a null prototype to avoid collisions with
+ * JavaScript's own property names.
  */
 export function getVariableValues(
   schema: GraphQLSchema,
@@ -138,16 +137,15 @@ function coerceVariableValues(
     );
   }
 
-  return { ...coercedValues };
+  return coercedValues;
 }
 
 /**
  * Prepares an object map of argument values given a list of argument
  * definitions and list of argument AST nodes.
  *
- * Note: The returned value is a plain Object with a prototype, since it is
- * exposed to user code. Care should be taken to not pull values from the
- * Object prototype.
+ * Note: The returned value uses a null prototype to avoid collisions with
+ * JavaScript's own property names.
  */
 export function getArgumentValues(
   def: GraphQLField<unknown, unknown> | GraphQLDirective,
@@ -222,7 +220,7 @@ export function getArgumentValues(
     }
     coercedValues[name] = coercedValue;
   }
-  return { ...coercedValues };
+  return coercedValues;
 }
 
 /**
@@ -232,9 +230,8 @@ export function getArgumentValues(
  *
  * If the directive does not exist on the node, returns undefined.
  *
- * Note: The returned value is a plain Object with a prototype, since it is
- * exposed to user code. Care should be taken to not pull values from the
- * Object prototype.
+ * Note: The returned value uses a null prototype to avoid collisions with
+ * JavaScript's own property names.
  */
 export function getDirectiveValues(
   directiveDef: GraphQLDirective,