Unicode characters get transformed into surrogate pairs by graphql.print_ast()

[dkbarn]

I've run into an issue where serializing a GraphQL DocumentNode into a string, and then parsing that back into a DocumentNode transforms certain unicode characters into surrogate pairs, which make them no longer UTF-8 encodeable.

This code snippet demonstrates the problem:

import graphql

value = "\U000a90e5"
print(f"Value before serializing: {value!r}")

encoded = value.encode("utf8")
print(f"UTF-8 encoded: {encoded!r}")

query = graphql.DocumentNode(
    definitions=[
        graphql.OperationDefinitionNode(
            operation=graphql.OperationType.QUERY,
            selection_set=graphql.SelectionSetNode(
                selections=[
                    graphql.FieldNode(
                        name=graphql.NameNode(
                            kind="name",
                            value="hello",
                        ),
                        arguments=[
                            graphql.ArgumentNode(
                                name=graphql.NameNode(value="user"),
                                value=graphql.StringValueNode(value=value)
                            )
                        ]
                    )
                ]
            )
        )
    ]
)

serialized_query = graphql.print_ast(query)

print(f"Serialized query: {serialized_query}")

parsed_query = graphql.parse(serialized_query)

value = parsed_query.definitions[0].selection_set.selections[0].arguments[0].value.value
print(f"Value after serializing: {value!r}")

encoded = value.encode("utf8")
print(f"UTF-8 encoded: {encoded!r}")

Given the unicode character \U000a90e5 which is UTF-8 encodeable, passing this value to a DocumentNode tree and serializing the AST into text transforms the character into the surrogate pair \uda64\udce5. Converting this back into a DocumentNode via graphql.parse() and then extracting the argument value shows that it has been modified. And it is no longer UTF-8 encodeable. The last line in this snippet produces the error: UnicodeEncodeError: 'utf-8' codec can't encode characters in position 0-1: surrogates not allowed

Environment:

• Python 3.8.5
• graphql-core 3.1.4

[dkbarn]

It looks like there may be an asymmetry in the serialization / deserialization functions.

Serialization involves calling json.dumps, which results in the surrogate pair:

>>> json.dumps('\U000a90e5')
'"\\uda64\\udce5"'

If deserialization were to usejson.loads, then the surrogate pair would be converted back into the original character:

>>> json.loads('"\\uda64\\udce5"')
'\U000a90e5'

[Cito]

Thanks for reporting. As always, I try to compare with what GraphQL.js is doing here:

import { parse, print } from 'graphql';

let value = "\u{000a90e5}";

const query = {
  kind: "Document",
  definitions: [
    {
      kind: "OperationDefinition",
      operation: "query",
      selectionSet: {
        kind: "SelectionSet",
        selections: [
          {
            kind: "Field",
            name: {
              kind: "Name",
              value: "hello",
            },
            arguments: [
              {
                kind: "Argument",
                name: {
                  kind: "Name",
                  value: "user",
                },
                value: {
                  kind: "StringValue",
                  value,
                },
              },
            ],
          },
        ],
      },
    },
  ],
};

const serializedQuery = print(query);
const parsedQuery = parse(serializedQuery);

value = parsedQuery.definitions[0].selectionSet.selections[0].arguments[0].value.value;

console.log(value.charCodeAt(0).toString(16), value.charCodeAt(1).toString(16));

This gives: 'da64' 'dce5'. So the value in Python is the same as in JavaScript, using surrogates.

You can avoid the UnicodeEncodeError by passing errors="surrogatepass" to the encode method.

But if you really think this is the wrong behavior, we should file an issue upstream to clarify and to achieve consistency.

Not sure what should be changed, though:

• should the print (printAst) method not create surrogates in the first place (maybe create \u{a90e5} instead)?
• should the parse method convert back to the non-surrogate representation?

I think this is relevant here, but I'm really not a Unicode expert and haven't studied the GraphQL specs in that regard yet.

[dkbarn]

I'm definitely not a unicode expert either, but having done a bit of reading up on it, and going through the graphql-spec ticket you linked, along with its links, here are my main takeaways:

• The GraphQL spec does not yet formally support unicode characters outside the Basic Multilingual Plane, as mentioned in allow full unicode range graphql/graphql-spec#687
• However, there are implementations of GraphQL that do support all unicode characters, such as graphql-ruby and GraphQL Java. See: Allow unicode characters outside the Basic Multilingual Plane graphql/graphql-spec#214 (comment)
• I found this comment particularly helpful in explaining how the GraphQL spec's treatment of escaping unicode characters is likely inspired by the JSON spec, which is restricted to \u#### syntax (as opposed to the newer \U######## and \u{...} syntaxes), thereby requiring the use of surrogate pairs. See also https://stackoverflow.com/a/38552626/6686740

In short, by opting to use json.dumps() in graphql-core, you are essentially adopting JSON's method of serializing unicode characters, which involves the use of surrogate pairs for anything above U+FFFF. So this already deviates from the official GraphQL spec, because it does not yet include official support for anything above U+FFFF.

It seems that treatment of these characters is being left to the individual implementations to decide at this point in time. The authors of graphql-ruby and GraphQL Java have opted to support all unicode characters. It would be great if graphql-core could do the same. Maybe worth looking at these other implementations to see how they're doing it?

[dkbarn]

I know that you are using GraphQL.js as your guide, so just wanted to point out that there is an outstanding PR which will add full unicode support to it as well: graphql/graphql-js#2449

To follow suit in graphql-core I think would require a tweak to the logic in the lexer here: https://github.com/graphql-python/graphql-core/blob/main/src/graphql/language/lexer.py#L296-L307

This would mean jumping ahead of what the spec formally recognizes, but would match the direction that the other implementations are moving in. If you want to stick to the letter of the spec, then I think there needs to be validation added to explicitly raise an exception when encountering a unicode character above U+FFFF rather than just unconditionally calling json.dumps here:

graphql-core/src/graphql/language/printer.py

Line 162 in 4dbb9d4

return dumps(node.value)

[Cito]

Thanks for these helpful remarks. When that GraphQL.js PR gets merged, I will of course happily port these changes to GraphQL-core. But this will only solve the deserialization issue. The json.dumps in the printer is a different issue. Again, I would prefer if we can come up with a suggestion for a change in GraphQL.js (which does exactly the same), and from there port it here.

[dkbarn]

Those two tasks are alternative options, not things that would need to be done together. In other words, porting the changes from that GraphQL.js PR would add full support for the whole unicode range, and eliminate the need to make any changes to json.dumps in the printer. The validation I was suggesting in the printer was to provide more explicitly defined behaviour right now, while the full unicode range is not supported, by checking for unsupported characters and raising an exception.

Is there a particular reason why graphql-js is the guiding force behind this repo, as opposed to say graphql-ruby or graphql-java? They're all just implementations of the spec, so it seems somewhat arbitrary to latch onto graphql-js and never stray from its exact behaviour.

[Cito]

Regarding your question, there are good reasons (see also "Goals and Restrictions" in the README): First, GraphQL.js was originally made by Facebook and was written by Lee Byron who is also the co-author of GraphQL. Meanwhile, it is developed as part of the GraphQL foundation. So this library is probably the closest to the specs and it is also continually updated. Second, I needed to restrict the scope of the project in order to be able to maintain it for a long time in a sustainable way, since I work on this only in my spare time. It surely is possible to create a GraphQL implementation in Python that is more performant, more Pythonic or has more features, but that's outside the declared scope of this project. Maybe an idea for another project.

[Cito]

The PR mentioned here has meanwhile be ported to GraphQL-core and is available since v3.2.0rc1.