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:

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

type Example {
    number: Int
}

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

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

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

query ExampleQuery($value: Int){
	example(number: $value){
		number
	}
}

{
	"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

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!