Graphql-js: Spec Compliance: Strict input coercion for scalars

[spawnia]

The GraphQL specification defines strict rules for input coercion, especially of scalar types: http://facebook.github.io/graphql/October2016/#sec-Scalars

However, inputs that are passed in as variables are not validated accordingly, while literals are. parseValue mostly behaves the same as serialize - which allows for some loose coercion.

If i understand the specification correctly, such loose coercion is only allowed for results - outbound information that the server sends to the client. It makes sense in this case, the server should not break the contract it defines in its schema.

Inputs, regardless if they are provided as literals or as variables, are inbound information which come from the client and are sent to the server. In that case, strict validation makes absolute sense - if the client sends bad data, the server should not quietly coerce the value but rather throw an error.

While literals and variables have to be handled differently at first, they both fall into the category of Input Coercion. The spec is really clear on how inputs should be handled, see Section 6.1.2:

If the operation has defined any variables, then the values for those variables need to be coerced using the input coercion rules of variable’s declared type.

What is currently happening is that the result coercion rules, which are different from input coercion rules, are applied to input variables. On the other hand, input literals are treated different than input variables - while according to the specification, they should be handled the same in regards to the coercion.

It seems like there is a fundamental misunderstanding here. If it is on my side, maybe you can clear it up for me? If not, i think resolving this issue to ensure spec compliance requires breaking changes to the implementation.

[IvanGoncharov]

@spawnia Can you please provide some examples?

[spawnia]

Sure. Given the following schema:

````graphql
type Query {
example(number: Int): Example
}

type Example {
number: Int
}
````

Now, if i send the following Query i get a type error:

graphql { example(number: "123"){ number } }

However, if i pass the same value as a variable, the value gets converted into a string:

graphql query ExampleQuery($value: Int){ example(number: $value){ number } }
json { "value": "123" }

I think the server should treat those values the same.

What actually happens can be seen in https://github.com/graphql/graphql-js/blob/master/src/type/scalars.js

````javascript
function coerceInt(value: mixed): ?number {
if (value === '') {
throw new TypeError(
'Int cannot represent non 32-bit signed integer value: (empty string)',
);
}
const num = Number(value);
if (num !== num || num > MAX_INT || num < MIN_INT) {
throw new TypeError(
'Int cannot represent non 32-bit signed integer value: ' + String(value),
);
}
const int = Math.floor(num);
if (int !== num) {
throw new TypeError(
'Int cannot represent non-integer value: ' + String(value),
);
}
return int;
}

export const GraphQLInt = new GraphQLScalarType({
name: 'Int',
description:
'The Int scalar type represents non-fractional signed whole numeric ' +
'values. Int can represent values between -(2^31) and 2^31 - 1. ',
serialize: coerceInt,
parseValue: coerceInt,
parseLiteral(ast) {
if (ast.kind === Kind.INT) {
const num = parseInt(ast.value, 10);
if (num <= MAX_INT && num >= MIN_INT) {
return num;
}
}
return undefined;
},
});
````

Integers are sent through the same function for coercion, no matter if it is being serialized (= result coercion) or parsed (= input coercion). Just the parsing of literals works differently and has the correct behaviour.

[IvanGoncharov]

It seems like there is a fundamental misunderstanding here. If it is on my side, maybe you can clear it up for me?

@spawnia You completely right, specification clear state that input coercion should be strict. Mover some behavior is ridiculous for example you can pass {} and it will be coerced to '[object Object]'.

If not, i think resolving this issue to ensure spec compliance requires breaking changes to the implementation.

Yes, it definitely a breaking change so it's important to include in 14.0.0 RC.
I will work on PR.

[leebyron]

Some great improves came from this issue, thanks for filing it!

[ahouzzer]

Hi, my company was using an old version of graphql.js and when we update to the new version, we encountered this error. Is there a way to enable the "incorrect" behavior so that my mobile client doesn't need to change their code? (Our mobile pass int input variables as strings)

[dchambers]

@ahouzzer, we had the same problem in my company, so I created a fork of graphql-js that re-introduces the old behaviour (for the most part), but where it now emits deprecation warnings that developers can fix in their own time:

• https://github.com/babylonhealth/graphql-js/commits/14.x.x

Please note, the more recent of the two commits also contains some company specific changes that you won't want, but it could be a good starting point for you.

[ahouzzer]

@dchambers LOL. Thanks. I have solved the problem by writing a small converter for input arguments.

[kohakade]

@ahouzzer I have the same issue. Could you please provide the resolve. It will help me alot.
Thanks in advance.

@dchambers LOL. Thanks. I have solved the problem by writing a small converter for input arguments.