Repository: jensneuse/graphql-go-tools (go)

Issue:
AST printer produces invalid output when printing strings that contain unescaped special characters
The [GraphQL spec](https://spec.graphql.org/June2018/#sec-String-Value) has two types of string literals: double-quoted strings, "hello", and triple-quoted raw strings, """hello""". Tripe-quote strings can contain unescaped white space, line terminators, quote, and backslash characters. Both styles of string can be used in a document anywhere a string is valid.

In our schemas, triple-quoted strings are sometimes used for deprecation reasons, e.g.
```
type User {
    fooField: String @deprecated(reason: """
        fooField has been replaced with barField. fooField
        has the following issues: x, y, z.
    """)
}
```
The AST printer in `astprinter` prints this as:
```
type User {fooField: String @deprecated(reason: "fooField has been replaced with barField. fooField
        has the following issues: x, y, z.")}
```
Notice the single quotes. This string is invalid.

The string needs to be either printed with triple quotes or the newline needs to be escaped, i.e the expected output is either something like:
```
type User {fooField: String @deprecated(reason: """fooField has been replaced with barField. fooField
        has the following issues: x, y, z.""")}
```
or:
```
type User {fooField: String @deprecated(reason: """fooField has been replaced with barField. fooField\nhas the following issues: x, y, z.""")}
```
Note that according to the GraphQL spec "there is no single canonical printed block string for a given value". It also states that leading empty lines, trailing empty lines and trailing indentation are stripped. The exact formatting of multi-line strings isn't the concern of this issue. (See also #105).

The AST printer is leveraged by code like `sdlmerge.MergeSDLs`. That function is unable to merge schemas with multi-line triple-quoted strings because of this bug.

Also note that newlines aren't the only characters that need to be escaped. See https://spec.graphql.org/June2018/#sec-String-Value for a complete list of characters.

Pull request description:
Update Document.PrintValue to print block strings using triple quotes
Fixes #292.

GraphQL has two types of string literals: double-quoted strings and
triple-quoted "block" strings. Block strings can include unescaped white
space, lint terminators, quote and backslash characters. (These
characters must be escaped in double-quoted strings). See the String
Value section of the GraphQL spec[1] for details.

[1] https://spec.graphql.org/June2018/#sec-String-Value

This PR updates the Document.PrintValue function so it emits a
triple-quoted string for block strings in the AST. That way printed
block strings, which may contain unescape characters, will always be
valid. This assumes the string content was valid when the AST was
constructed (which seems like a reasonable assumption).

While working on this PR I realized the Document.ValueToJSON method also
has a bug. Raw string content in the AST is unquoted in block strings,
but the ValueToJSON code assumes string content is quoted. This means
invalid JSON may be produced when block strings are serialized to JSON.
This bug IS NOT addressed in this PR. Instead, I'll create a separate
issue to address this bug.

Patch:
diff --git a/pkg/ast/ast_value.go b/pkg/ast/ast_value.go
index 78e553b7..31605de3 100644
--- a/pkg/ast/ast_value.go
+++ b/pkg/ast/ast_value.go
@@ -118,9 +118,24 @@ func (d *Document) PrintValue(value Value, w io.Writer) (err error) {
 			_, err = w.Write(literal.FALSE)
 		}
 	case ValueKindString:
+		// This code assumes string content is valid for the associated string
+		// type (block/non-block) according to the GraphQL spec. Content IS NOT
+		// processed to quote characters that are invalid for the associated
+		// type.
+		//
+		// GraphQL spec: https://spec.graphql.org/June2018/#StringValue
+		isBlockString := d.StringValues[value.Ref].BlockString
 		_, err = w.Write(literal.QUOTE)
+		if isBlockString {
+			_, err = w.Write(literal.QUOTE)
+			_, err = w.Write(literal.QUOTE)
+		}
 		_, err = w.Write(d.Input.ByteSlice(d.StringValues[value.Ref].Content))
 		_, err = w.Write(literal.QUOTE)
+		if isBlockString {
+			_, err = w.Write(literal.QUOTE)
+			_, err = w.Write(literal.QUOTE)
+		}
 	case ValueKindInteger:
 		if d.IntValues[value.Ref].Negative {
 			_, err = w.Write(literal.SUB)


Test patch:
diff --git a/pkg/ast/ast_value_test.go b/pkg/ast/ast_value_test.go
index 15ac81a5..1ca6a6e2 100644
--- a/pkg/ast/ast_value_test.go
+++ b/pkg/ast/ast_value_test.go
@@ -1,6 +1,7 @@
 package ast
 
 import (
+	"bytes"
 	"strconv"
 	"testing"
 
@@ -173,3 +174,34 @@ func TestDocument_ValueToJSON(t *testing.T) {
 		}
 	}, `{"foo":"bar","baz":{"bat":"bal"},"list":[1,2,3]}`))
 }
+
+func TestDocument_PrintValue(t *testing.T) {
+	run := func(prepareDoc func(doc *Document) Value, expectedOutput string) func(t *testing.T) {
+		operation := NewDocument()
+		return func(t *testing.T) {
+			buf := new(bytes.Buffer)
+			err := operation.PrintValue(prepareDoc(operation), buf)
+			assert.NoError(t, err)
+			assert.Equal(t, expectedOutput, string(buf.Bytes()))
+		}
+	}
+	t.Run("ValueKindString - non-block", run(func(doc *Document) Value {
+		doc.StringValues = append(doc.StringValues, StringValue{
+			Content: doc.Input.AppendInputString("foo"),
+		})
+		return Value{
+			Kind: ValueKindString,
+			Ref:  0,
+		}
+	}, `"foo"`))
+	t.Run("ValueKindString - block", run(func(doc *Document) Value {
+		doc.StringValues = append(doc.StringValues, StringValue{
+			BlockString: true,
+			Content:     doc.Input.AppendInputString("foo"),
+		})
+		return Value{
+			Kind: ValueKindString,
+			Ref:  0,
+		}
+	}, `"""foo"""`))
+}