Passing JSONString with value "null" as mutation input leads to an error.

[sergey-komissarov]

Hello, graphql-core 2.1 is unable to parse such values as argument.

Result: Argument "value" has invalid value "null". Expected type "JSONString", found "null".

Seems that None value is invalid regardless of the argument type. Of course we can handle it by passing null via query argument but I still think this is a bug.

Exception stack trace:

File "/venv/lib/python3.7/site-packages/graphql/graphql.py", line 44, in graphql
     return execute_graphql(*args, **kwargs)
File "/venv/lib/python3.7/site-packages/graphql/graphql.py", line 70, in execute_graphql
     **execute_options
File "/venv/lib/python3.7/site-packages/graphql/backend/core.py", line 30, in execute_and_validate
     validation_errors = validate(schema, document_ast)
File "/venv/lib/python3.7/site-packages/graphql/validation/validation.py", line 33, in validate
     return visit_using_rules(schema, type_info, ast, rules)
File "/venv/lib/python3.7/site-packages/graphql/validation/validation.py", line 40, in visit_using_rules
     visit(ast, TypeInfoVisitor(type_info, ParallelVisitor(visitors)))
File "/venv/lib/python3.7/site-packages/graphql/language/visitor.py", line 128, in visit
     result = enter(node, key, parent, path, ancestors)
File "/venv/lib/python3.7/site-packages/graphql/language/visitor.py", line 279, in enter
     result = self.visitor.enter(node, key, parent, path, ancestors)
File "/venv/lib/python3.7/site-packages/graphql/language/visitor.py", line 231, in enter
     result = visitor.enter(node, key, parent, path, ancestors)
File "/venv/lib/python3.7/site-packages/graphql/language/visitor.py", line 191, in enter
     return method(self, node, key, parent, path, ancestors)
File "/venv/lib/python3.7/site-packages/graphql/validation/rules/arguments_of_correct_type.py", line 24, in enter_Argument
     errors = is_valid_literal_value(arg_def.type, node.value)
File "/venv/lib/python3.7/site-packages/graphql/utils/is_valid_literal_value.py", line 86, in is_valid_literal_value
     if parse_result is None:

Simple way to reproduce:

Query example:

mutation {
  testNull(value: "null") { value }
}

GraphQL schema:

TestNull:
  value: JSONString

testNull(value: JSONString): TestNull

Graphene code:

class TestNull(graphene.Mutation):
    class Input:
        value = graphene.JSONString()

    value = graphene.JSONString()

    @classmethod
    def mutate(cls, instance, input, info):
        value = input.get('value')
        print(f"VALUE={value}")
        return TestNull(value=value)

[jkimbo]

@sergey-komissarov this doesn't look like a bug because you're trying to pass the string null to the JSON parser and so it's going to error out. If you have no valid json to pass to the graphene then you shouldn't be passing a value at all.

[sergey-komissarov]

@sergey-komissarov this doesn't look like a bug because you're trying to pass the string null to the JSON parser and so it's going to error out. If you have no valid json to pass to the graphene then you shouldn't be passing a value at all.

I can not agree with you, "null" is a valid JSON and parsed without errors. But core itself does not allow argument field to be None.
Also passing None as member value or not passing member at all have different meaning for our graphql api, hacking it with null in query argument works but smells badly.

[jkimbo]

@sergey-komissarov my apologies you're absolutely right. It looks like the bug is here: https://github.com/graphql-python/graphql-core/blob/fa4eeda36029680205e20059379e89189b946032/graphql/utils/is_valid_value.py#L77-L79

Because json.loads("null") returns None it's tripping up the validation and throwing the error.

Unfortunately I can't see an easy fix. You might want to try https://github.com/graphql-python/graphql-core-next which is a more recent port of the graphql-js reference library and so probably doesn't suffer from the same bug. I don't think it's compatible with Graphene yet but you can still use it on its own. Also @Cito is a much more responsive maintainer 🙂

[dperetti]

I don't understand why it can't be fixed? 🤔

[AlexzAK]

I'd like to see that bug fixed too 🥲