graph-gophers / graphql-go

GraphQL server with a focus on ease of use
BSD 2-Clause "Simplified" License
4.65k stars 492 forks source link

Issue with negative numbers #624

Closed otboss closed 8 months ago

otboss commented 1 year ago

With the graph-gophers/graphql-go package there seems to be an issue with negative number support. See the error message in the image attached:

image

As shown, sending a negative float number to a resolver result in an error message being returned.

otboss commented 1 year ago

This issue appears to be fixed, though the changes have not yet been released. (latest released version at this point in time is 1.5.0) image

otboss commented 1 year ago

I propose the following fix as to not dismiss the minimal boundary entirely:

index 4cdf274..1848058 100644
--- a/internal/validation/validation.go
+++ b/internal/validation/validation.go
@@ -890,7 +890,7 @@ func validateBuiltInScalar(v string, n string) bool {
                return f >= math.MinInt32 && f <= math.MaxInt32
        case "Float":
                f, fe := strconv.ParseFloat(v, 64)
-               return fe == nil && f <= math.MaxFloat64
+               return fe == nil && f >= -math.MaxFloat64 && f <= math.MaxFloat64
        case "String":
                vl := len(v)
                return vl >= 2 && v[0] == '"' && v[vl-1] == '"'

I am kindly asking that a new release be created with the latest changes. Also consider adding me as a collaborator.

pavelnikolov commented 8 months ago

Actually, I don't think we need to check the boundary at all since strcov.ParseFloat(v, 64) would return an error if that isn't a valid float64 number. No need to check for boundaries. If you still believe there are some problem, please send a failing unit test. Thanks.