Skip to content

GraphQL silently cast huge cdata numbers to null #17

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

Closed
olegrok opened this issue Oct 8, 2020 · 4 comments · Fixed by #55
Closed

GraphQL silently cast huge cdata numbers to null #17

olegrok opened this issue Oct 8, 2020 · 4 comments · Fixed by #55

Comments

@olegrok
Copy link
Collaborator

olegrok commented Oct 8, 2020

in code:

object.version = -1ULL

in query such field silently return null.

@olegrok olegrok changed the title GraphQL silently cast -1ULL to null GraphQL silently cast huge cdata numbers to null Nov 11, 2020
@rosik
Copy link

rosik commented Sep 6, 2021

Could you provide a more detailed description, please? Is it still relevant, by the way?

@olegrok olegrok transferred this issue from tarantool/cartridge Sep 6, 2021
@kyukhin kyukhin added the teamE label Oct 25, 2021
@Totktonada
Copy link
Member

@olegrok Friendly reminder. I don't understand the issue and have no idea how to reproduce it.

I made some attempts, but can't deduce anything useful from them.

diff --git a/test/integration/graphql_test.lua b/test/integration/graphql_test.lua
index 6462450..a79f17d 100644
--- a/test/integration/graphql_test.lua
+++ b/test/integration/graphql_test.lua
@@ -2147,3 +2147,60 @@ function g.test_non_finite_float()
             query_schema, nil, nil, {variables = variables})
     end
 end
+
+function g.test_huge_cdata_number()
+     local query = [[
+        query ($x: Long!) { test(arg: $x) }
+    ]]
+
+    local function callback(_, args)
+        return args[1].value
+    end
+
+    local query_schema = {
+        ['test'] = {
+            kind = types.long.nonNull,
+            arguments = {
+                arg = types.long.nonNull,
+            },
+            resolve = callback,
+        }
+    }
+
+    -- 2^64-1
+    --[[
+    local variables = {x = 18446744073709551615ULL}
+    local res = check_request(query, query_schema, nil, nil, {variables = variables})
+    -- Raises:
+    -- > Wrong variable "x" for the Scalar "Long"
+    t.assert_type(res, 'table')
+    t.assert_equals(res.test, 18446744073709551615ULL)
+    --]]
+
+    -- 2^52-1: maximum allowed by the specification of Long.
+    --
+    -- https://github.com/tarantool/graphql/wiki/Long
+    local variables = {x = 4503599627370495ULL}
+    local res = check_request(query, query_schema, nil, nil, {variables = variables})
+    t.assert_type(res, 'table')
+    t.assert_equals(res.test, 4503599627370495ULL)
+    -- Succeeds.
+
+    -- 2^63-1: de-facto maximum.
+    --
+    -- I don't know why something out of the specification is
+    -- accepted.
+    --
+    -- OTOH, I don't know why the specification shrinks the range
+    -- to [-2^52; 2^52 - 1]. I mean, well, `number` Lua type
+    -- represents integral numbers precisely only in this
+    -- range[^1], but larger values are representable with cdata
+    -- numbers.
+    --
+    -- [^1]: [-2^53; 2^53] in fact.
+    local variables = {x = 9223372036854775807ULL}
+    local res = check_request(query, query_schema, nil, nil, {variables = variables})
+    t.assert_type(res, 'table')
+    t.assert_equals(res.test, 9223372036854775807ULL)
+    -- Succeeds.
+end

(My version of the module is 0.1.4.)

@Totktonada Totktonada added the needs feedback Something is unclear with the issue label Apr 8, 2022
@olegrok
Copy link
Collaborator Author

olegrok commented Apr 9, 2022

My example will be related to TDG.
I have following schema:

local user_schema = types.object{
    name = 'UserSchema',
    fields = {
        uid = types.string.nonNull,
        email = types.string.nonNull,
        login = types.string.nonNull,
        username = types.string.nonNull,
        created_at = types.long.nonNull,
        last_login = types.long,
        state = types.string.nonNull,
        state_reason = types.string,
        role = types.string.nonNull,
        roles = types.list(types.string.nonNull),
        expires_in = types.long.nonNull,
        last_password_update_time = types.long,
        failed_login_attempts = types.long.nonNull,
        unblocked_at = types.long,
        tenant = types.string,
    },
    description = 'User info',
    schema = 'admin',
}

local self_user = types.object{
        name='GraphqlUser',
        description='Graphql authorization info',
        fields = {
            user = user_schema,
        },
    }

graphql.add_callback({
        schema='admin',
        prefix = 'user',
        name = 'self',
        doc = 'Returns if access granted and current logged user. ' ..
            'If user is null than access granted by anonymous or token',
        args = {},
        kind = self_user,
        callback = 'common.admin.users.self',
    })

Then I write simple function:

local function user_self(_, _)
    if true then
        return {
            user = { unblocked_at = 1000 },
        }
    end
end

It works fine:
image

But after I change 1000 to "-1ULL" (the same as 18446744073709551615ULL) I have following result:
image

Actually it works fine with "-1ULL / 2" but starts to return null since "-1ULL / 2 + 1"

@Totktonada Totktonada removed the needs feedback Something is unclear with the issue label Apr 9, 2022
@no1seman
Copy link

no1seman commented Apr 16, 2022

tarantool> ffi=require('ffi'); a=-1ULL; print(a); print(type(a)); print(ffi.istype('uint64_t', a)); tonumber64('-1ULL')
18446744073709551615ULL
cdata
true
---
...

seems that the problem is in tonumber64() because for unknown reason -1ULL becomes 'uint64_t' instead of 'int64_t'

also may be related to: tarantool/tarantool#4738

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

Successfully merging a pull request may close this issue.

5 participants