wundergraph / graphql-go-tools

GraphQL Router / API Gateway framework written in Golang, focussing on correctness, extensibility, and high-performance. Supports Federation v1 & v2, Subscriptions & more.
https://graphql-api-gateway.com
MIT License
685 stars 129 forks source link

Error in datasource when querying using triple quotes syntax containing quotes or newlines #839

Closed mattjohnsonpint closed 2 months ago

mattjohnsonpint commented 3 months ago

Given a schema such as:

type Query {
    foo(bar: String):String
}

I can accept queries like this:

query {
  foo(bar: "abc")
}

or:

query {
  foo(bar: "abc\ndef")
}

or with variables (containing any newlines or not):

query MyQuery($bar: String){
  foo(bar: $bar)
}

But I believe I should be able to also do this, and it fails:

query {
  foo(bar: 
"""
abc
def
""")
}

I'm using a custom data source with a JsonVariableRenderer.

With some trace debugging, I believe the problem is here:

https://github.com/wundergraph/graphql-go-tools/blob/3c4cb175dafb214644c3eee89960808e03924d54/v2/pkg/ast/ast_value.go#L146-L147

Since the string value returned by d.StringValueContentBytes(value.Ref) contains a raw newline character, and it's only wrapping it in quotes, the resulting value doesn't have the newline escaped as "\n" to be valid JSON.

The failure occurs in my custom datasource at the start of the Load function when I try to unmarshal the input string. It should be valid JSON, but in this case it's malformed.

A minimal repro will take some time if you need one, but I thought I'd report it in the meantime. Thanks.

mattjohnsonpint commented 3 months ago

For now I've worked around it with:

input = bytes.ReplaceAll(input, []byte("\n"), []byte("\\n"))

... in my datasource before I parse. But this should probably be done earlier. Thanks.

mattjohnsonpint commented 3 months ago

An update, it's not just newlines. In a """ block I can also have raw quotes, and those don't get escaped properly either.

query {
  foo(bar: 
"""
a"bc
""")
}

I can't workaround this case easily either, because the outer JSON quotes would need to remain unescaped.

StarpTech commented 3 months ago

Hi @mattjohnsonpint, great catch! Would you be open to create a PR?

mattjohnsonpint commented 3 months ago

@StarpTech - PR created: #843