Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dangerous and incorrect int and float validation and parsing #921

Open
Jiia opened this issue Jun 21, 2021 · 2 comments
Open

Dangerous and incorrect int and float validation and parsing #921

Jiia opened this issue Jun 21, 2021 · 2 comments

Comments

@Jiia
Copy link

Jiia commented Jun 21, 2021

The int and float validation made by this library are not working correctly. They not only fail to validate the input values but they also incorrectly parse and modify the input values.

https://github.com/Urigo/graphql-scalars/blob/master/src/scalars/utilities.ts#L100

For example if your field is of type float and your input value is eg string 0,8 what would you expect to happen?

You probably would expect that a string doesn't pass validation for a field of type float, but you'd be wrong. The string 0,8 passes validation.

Because the library uses parseFloat the input value will be changed to the number 0 once the library is done with it.

I think the int and float validation should be reworked to not use any sort of parsing.

@ardatan
Copy link
Collaborator

ardatan commented Jun 21, 2021

The scenario and your request are different. We can solve that issue by comparing the final coerced value with the input value but this is completely unrelated to removing the parsing stuff.
Even if we do the solution I said above, I want to stick with graphql-js scalars' logic
https://github.com/graphql/graphql-js/blob/df1bddac6e7f2cef730b5c3695126820d075bbfd/src/type/scalars.ts#L118

@robross0606
Copy link

robross0606 commented Jul 20, 2021

I touched on this with #479 but was told it doesn't exist. lt does, but I haven't had time to produce a unit test to prove it. I'm not sure about things like "0,8", but I can say for sure that it is not doing "strong" typing as you would want with GraphQL. Instead, it is using JavaScript type coercion which cuts the legs out from under GraphQL typing a bit. A value like "3.1415" submitted on a PositiveInt type would be allowed and become a number type containing 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants