From 52088c8b8ee86e431107d9281b97ac4d6415ff3a Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Fri, 1 Apr 2022 21:36:56 +0300 Subject: [PATCH 1/2] Accept a cdata number as value of a Float variable It is quite usual that a floating point number is encoded using JSON, goes over HTTP, received on tarantool server, decoded from JSON and passed as a variable into the GraphQL executor. A number out of the [-10^14+1; 10^14-1] range is decoded into Lua as cdata number[^1] to don't loss the precision. A JSON decoder don't know that we intend to use this value as the floating point one and chooses the conservative option: decode the value into cdata number to don't loss the precision. In GraphQL we know that it is actually the floating point value (disregarding its representation in Lua). So it looks correct to accept a cdata number as value of a Float variable. The similar idea was expressed in [1] against tarantool itself: let it accept a number without fractional part as a value suitable for the `float` field type. [1]: https://github.com/tarantool/tarantool/issues/5933 [^1]: `cdata` or `cdata` Fixes #47 --- graphql/types.lua | 10 ++++- test/integration/graphql_test.lua | 61 +++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/graphql/types.lua b/graphql/types.lua index 8b21282..04cedc1 100644 --- a/graphql/types.lua +++ b/graphql/types.lua @@ -318,7 +318,15 @@ types.long = types.scalar({ }) local function isFloat(value) - return type(value) == 'number' + if type(value) == 'number' then + return true + end + + if type(value) == 'cdata' then + return ffi.istype('int64_t', value) or ffi.istype('uint64_t', value) + end + + return false end local function coerceFloat(value) diff --git a/test/integration/graphql_test.lua b/test/integration/graphql_test.lua index be7d57e..0bf698c 100644 --- a/test/integration/graphql_test.lua +++ b/test/integration/graphql_test.lua @@ -2051,3 +2051,64 @@ g.test_propagate_defaults_to_callback = function() t.assert_equals(errors, nil) t.assert_items_equals(json.decode(data.prefix.test_mutation), result) end + +-- gh-47: accept a cdata number as a value of a Float variable. +function g.test_cdata_number_as_float() + local query = [[ + query ($x: Float!) { test(arg: $x) } + ]] + + local function callback(_, args) + return args[1].value + end + + local query_schema = { + ['test'] = { + kind = types.float.nonNull, + arguments = { + arg = types.float.nonNull, + }, + resolve = callback, + } + } + + -- 2^64-1 + local variables = {x = 18446744073709551615ULL} + local res = check_request(query, query_schema, nil, nil, {variables = variables}) + t.assert_type(res, 'table') + t.assert_almost_equals(res.test, 18446744073709551615) +end + +-- Accept a large number in a Float argument. +-- +-- The test is created in the scope of gh-47, but it is not +-- strictly related to it: the issue is about interpreting +-- a cdata number in a **variable** as a `Float` value. +-- +-- Here we check a large number, which is written verbatim as +-- an argument in a query. Despite that it is not what is +-- described in gh-47, it worth to have such a test. +function g.test_large_float_argument() + -- 2^64-1 + local query = [[ + { test(arg: 18446744073709551615) } + ]] + + local function callback(_, args) + return args[1].value + end + + local query_schema = { + ['test'] = { + kind = types.float.nonNull, + arguments = { + arg = types.float.nonNull, + }, + resolve = callback, + } + } + + local res = check_request(query, query_schema) + t.assert_type(res, 'table') + t.assert_almost_equals(res.test, 18446744073709551615) +end From ec48eac27bcd8b85b132c30113b7404e5bf1bbf4 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Sat, 2 Apr 2022 00:27:11 +0300 Subject: [PATCH 2/2] Forbid NaN and Infinity as a value Float type The GraphQL specification explicitly forbids it: | Non-finite floating-point internal values (NaN and Infinity) cannot | be coerced to Float and must raise a field error. http://spec.graphql.org/October2021/#sec-Float Follows up #47 --- graphql/types.lua | 13 +++++++++++- test/integration/graphql_test.lua | 35 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/graphql/types.lua b/graphql/types.lua index 04cedc1..77db320 100644 --- a/graphql/types.lua +++ b/graphql/types.lua @@ -317,9 +317,20 @@ types.long = types.scalar({ isValueOfTheType = isLong, }) +-- Return `false` for NaN, Negative Infinity or Positive Infinity. +-- Return `true` otherwise. +local function isfinite(n) + local d = n - n + return n == n and d == d +end + local function isFloat(value) + -- http://spec.graphql.org/October2021/#sec-Float + -- + -- > Non-finite floating-point internal values (NaN and Infinity) cannot be + -- > coerced to Float and must raise a field error. if type(value) == 'number' then - return true + return isfinite(value) end if type(value) == 'cdata' then diff --git a/test/integration/graphql_test.lua b/test/integration/graphql_test.lua index 0bf698c..6462450 100644 --- a/test/integration/graphql_test.lua +++ b/test/integration/graphql_test.lua @@ -2112,3 +2112,38 @@ function g.test_large_float_argument() t.assert_type(res, 'table') t.assert_almost_equals(res.test, 18446744073709551615) end + +-- http://spec.graphql.org/October2021/#sec-Float +-- +-- > Non-finite floating-point internal values (NaN and Infinity) cannot be +-- > coerced to Float and must raise a field error. +function g.test_non_finite_float() + local query = [[ + query ($x: Float!) { test(arg: $x) } + ]] + + local function callback(_, args) + return args[1].value + end + + local query_schema = { + ['test'] = { + kind = types.float.nonNull, + arguments = { + arg = types.float.nonNull, + }, + resolve = callback, + } + } + + local nan = 0 / 0 + local inf = 1 / 0 + local ninf = -inf + + for _, x in pairs({nan, inf, ninf}) do + local variables = {x = x} + t.assert_error_msg_content_equals( + 'Wrong variable "x" for the Scalar "Float"', check_request, query, + query_schema, nil, nil, {variables = variables}) + end +end