GraphQL string semantics unclear and inconsistent in the presence of escape sequences

chewxy · December 22, 2020, 3:25am

Given a schema like this:

type Post {
    id: ID!
    raw: String!
}

Here’s a way to break a mutation:

mutation MyMutation {
  addPost(input: {raw: "\xaa Raw content"}) {
    numUids
  }
}

giving rise to the following error:

{
  "errors": [
    {
      "message": "Unexpected <Invalid>",
      "locations": [
        {
          "line": 2,
          "column": 26
        }
      ]
    }
  ]
}

This works:

mutation MyMutation {
  addPost(input: {raw: "\\xaa Raw content"}) {
    numUids
  }
}

Inconsistent Semantics

The Graphql spec defines a String as such: “String: A UTF‐8 character sequence.”. So far so good. Both sequences "\xaa" and "\\xaa" are valid UTF-8 character sequences. They have different meanings, however. The first one is basically another way of writting "ª", while the second would be represented as "\xaa".

Perhaps then there was some unstated requirement that escape sequences (\n, \t, \a, \xhh etc) should themselves be escaped. But that’s untrue. Our implementation is inconsistent.

To wit, this works too - the sequence is correctly stored.

mutation MyMutation {
  addPost(input: {raw: "\n Raw content"}) {
    numUids
  }
}

It would seem that some escape sequences are more privileged than others. This should not be the case. So what could they be? Perhaps control characters do not require escapes, while octal/hex literals require escapes. The following table enumerates what is OK and what is not:

Character	Type	OK?
`\0`	Control	No
`\a`	Control	No
`\b`	Control	Yes
`\t`	Control	Yes
`\n`	Control	Yes
`\v`	Control	No
`\f`	Control	Yes
`\r`	Control	Yes
`\x1a`	Control	No
`\e` or `\x1b`	Control	No
`\xHH`	Hex literal	No
`\OO`	Octal literal	No
`\uHHHH`	Unicode literal	No

It’s rather maddening. Especially when dealing with text corpuses in the large. For example, people on forums type/mistype \u all the time (e.g. a dyslexic user may type "me\u" instead of "me/u"). Ironically, this very post can’t go into Slash GraphQL without further processing.

What is the ideal way?

thisistheway

Jokes aside, strings are hard, man. I think we should follow the GraphQL spec and just accept a sequence of UTF8 characters. That means not expecting the inputs to be escaped. We should let people type whatever they want in strings.

JatinDevDG · December 22, 2020, 12:36pm

Hi @chewxy, nice finding !!
I am creating ticket for this and will make these enhancements in coming sprints after discussion with graphql team.

chewxy · December 22, 2020, 12:45pm

I’m putting in a PR to vektah/gqlparser, which I have traced the errors to be.

chewxy · December 23, 2020, 12:41am

github.com/vektah/gqlparser

Better handling of string lexing

vektah:master ← chewxy:master

opened 12:40AM - 23 Dec 20 UTC

chewxy

+62 -25

# The Problem # Given a schema like this: ``` type Post { id: ID! … raw: String! } ``` Here's a way to break a mutation: ``` mutation MyMutation { addPost(input: {raw: "\xaa Raw content"}) { numUids } } ``` giving rise to the following error: ``` input:3: Unexpected <Invalid> ``` The issue is that `\xaa` is not parsed correctly. # The Solution # The solution is quite simple: add a case to the `readString()` method of the lexer to handle `x` as a escape character. # Side Note: How Parsing of Strings Should Work Please note that the handling of bad inputs for `\xHH` (where `H` is a meta-variable standing in for a hexadecimal number) is not quite the same as elsewhere in the package. I've got a good reason for this, and I am willing to make changes to the other escape sequences as well. With this PR, the bad inputs will lead to the literals being returned - so that `"\xZZ"` will return `"\xZZ"`, while good inputs such as `"\xaa"` will be parsed correctly, returning `"ª"`. I believe this is more user friendly. In the example I had listed, the scenario is one where the server is receiving a mutation. The input string, could be an end user input. And end users do often type the wrong things. For example, consider a dyslexic person trying to write the sentence "they will give it to me/u". Said person would often type something along the lines of "they will give it to me\u". In this case, the extra parsing for UTF-8 characters in the string will cause this input to fail. What the user meant to type, in a string representation, is `"they will give it to me\\u"`. An argument could be made that it is the onus of the user of this library to escape the string `"they will give it to me\u"` to `"they will give it to me\\u"`. My counter argument is that the role of a lexer is to simply find tokens. A string token that contains the literal bytes in `"they will give it to me\u"` would qualify as a string token. That gqlparser goes above and beyond in order to parse out the UTF8 characters in the contents of a string is most commendable. But it should not return an error. In the example I have given so far, it would be very unfriendly to the end user, as well as the user of the library. There can be a further argument to be made - that having the user of this library parse the string and escape any invalid sequences would be extra computation wasted. Now as a total, the program has to go through the string twice - once to escape bad sequences (done by the user of this library), and the second, to parse the correct escape sequences into UTF8 chars (done by this library). If we could save computation by doing all at once, we would make the world a much nicer place. As I mentioned, I am willing to put the changes in for handling the rest of the bad escape sequences. I am also OK if you want to just keep returning errors for string parsing, and will modify this PR. Your call @vektah . # End Notes This was originally filed as an issue in Dgraph's community forum: https://discuss.dgraph.io/t/graphql-string-semantics-unclear-and-inconsistent-in-the-presence-of-escape-sequences/12019

Topic		Replies	Views
Go Client Mutations Users	3	780	November 28, 2017
"set" mutation escape character in JSON error Users mutation	2	908	November 15, 2019
Naming rules - Dgraph vs GraphQL GraphQL kind:enhancement	5	985	August 24, 2020
[Bug] StringHashFilter of update mutation does not work correctly GraphQL status:accepted , kind:bug , ticket:created	2	1368	January 6, 2023
Mutations: GraphQL vs Dgraph format Users	4	1690	November 28, 2017

GraphQL string semantics unclear and inconsistent in the presence of escape sequences

Inconsistent Semantics

What is the ideal way?

Related topics