diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 133abfaacc..12b992fdea 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -164,6 +164,15 @@ const TestType = new GraphQLObjectType({ type: TestNestedInputObject, }), fieldWithJSONScalarInput: fieldWithInputArg({ type: TestJSONScalar }), + fieldWithPrototypeNamedArgument: { + type: GraphQLString, + args: { + toString: { type: GraphQLString }, + }, + resolve(_, args) { + return args.toString === undefined ? 'missing' : inspect(args.toString); + }, + }, list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }), nested: { type: NestedType, @@ -1225,6 +1234,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', () => { @@ -1661,4 +1684,31 @@ describe('Execute: Handles inputs', () => { expect(result).to.include.keys('initialResult', 'subsequentResults'); }); }); + + describe('getVariableValues: own-property names', () => { + const doc = parse(` + query ($toString: String) { + fieldWithNullableStringInput(input: $toString) + } + `); + + const operation = doc.definitions[0]; + assert(operation.kind === Kind.OPERATION_DEFINITION); + const { variableDefinitions } = operation; + assert(variableDefinitions != null); + + it('does not expose prototype variable names when omitted', () => { + const result = getVariableValues(schema, variableDefinitions, {}); + assert('variableValues' in result); + expect(result.variableValues.coerced.toString).to.equal(undefined); + }); + + it('still returns provided variables with colliding names', () => { + const result = getVariableValues(schema, variableDefinitions, { + toString: 'value', + }); + assert('variableValues' in result); + expect(result.variableValues.coerced.toString).to.equal('value'); + }); + }); }); diff --git a/src/execution/values.ts b/src/execution/values.ts index 623060203b..8b0c3c77e9 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -58,9 +58,8 @@ type VariableValuesOrErrors = * 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 `coerced` and `sources` properties of VariableValues use null + * prototype to avoid collisions with JavaScript's own property names. */ export function getVariableValues( schema: GraphQLSchema, @@ -195,9 +194,8 @@ export function getFragmentVariableValues( * 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 | GraphQLDirective, @@ -316,9 +314,8 @@ function coerceArgument( * * 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, @@ -326,7 +323,7 @@ export function getDirectiveValues( variableValues?: Maybe, fragmentVariableValues?: Maybe, hideSuggestions?: Maybe, -): undefined | { [argument: string]: unknown } { +): undefined | ObjMap { const directiveNode = node.directives?.find( (directive) => directive.name.value === directiveDef.name, );