Query variable scalar type coercion

[dereklavigne18]

The framework currently coerces query variables similar to the way it
treats output values, which means it attempts to coerce the value into
the field's corresponding data type regardless of the received value.
According to items 3f and 3g in section 6.1.2
(http://facebook.github.io/graphql/#sec-Validating-Requests) of
Facebook's GraphQL specification query variables should be coerced
according to their type's input coercion rules laid out in section
3.1.1 (http://facebook.github.io/graphql/#sec-Scalars). If the value
can not be coerced into the correct type according the the input
coercion rules for the type a query error should be thrown. This
ensures that client provided query variables were of the correct format
and will be a valid format and type by the time they are passed into an
implementing resolver.

One instance where we have seen this cause trouble before is when boolean query variable is passed a string. This should throw a query error according to the specification, but instead "false" was being coerced to true because the string was not empty. This then caused unexpected behavior during the execution of the resolver taking the query variable as an argument.

[vladar]

Thanks for reporting. Can you provide an example query which failed for your case?

I see your PR, but I must review it carefully before merging. And having more context/examples would be very useful.

[dereklavigne18]

The most common case I have noticed is the coercion of boolean query variables. Since there are not any checks on the value passed into a query variable declared as boolean (https://github.com/webonyx/graphql-php/blob/master/src/Type/Definition/BooleanType.php#L37) and the coercion relies entirely on what PHP interprets as truthy and falsey some unexpected values can be passed into the resolver. For instance, "false" and "0.0" are interpreted as truthy while "0" and 0.0 are interpreted as falsey. A client application of ours had at one point been sending "false" in a boolean query variable, which was then coerced to true and then passed off to the resolve function. While the client application should not be passing arguments of the wrong type (especially since there is no way to know how they will be handled) the spec says it should raise a query error so the client knows it has done something wrong.

Here's a quick PHP script I wrote to show how some edge case boolean query variables are being handled (apparently I can't upload .php files, so it's a .txt)
boolCoercionExample.txt.

I also included some tests in my PR that should confirm that the query variable type coercion meets the standards outlined in the specification. If you need anything else from me please let me know.

[vladar]

Looks like boolean coercion is broken because it mirrors graphql-js. And here is a related bug in reference js implementation.

[vladar]

Thanks a lot for your work! I merged your PR and will release it in 0.11.0 version. Unfortunately, I can't release in 0.10.x series because it may potentially break existing apps (even if it is a bugfix).

[vladar]

FYI, released v0.11.0