Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid AST printer: add escape characters to single line descriptions #3443

Merged
merged 5 commits into from Feb 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/main/java/graphql/GraphQLError.java
Expand Up @@ -39,8 +39,10 @@ public interface GraphQLError extends Serializable {
ErrorClassification getErrorType();

/**
* The graphql spec says that the (optional) path field of any error should be a list
* of path entries https://spec.graphql.org/October2021/#sec-Handling-Field-Errors
* The graphql spec says that the (optional) path field of any error must be
* a list of path entries starting at the root of the response
* and ending with the field associated with the error
* https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to AST printing, there is a minor spec change coming soon that specifies the path (if present) must be a list from root to the field. As this engine is already compliant this is only a comment change graphql/graphql-spec#1073

*
* @return the path in list format
*/
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/language/AstPrinter.java
Expand Up @@ -534,7 +534,7 @@ private String description(Node<?> node) {
if (description.isMultiLine()) {
s = "\"\"\"" + (startNewLine ? "" : "\n") + description.getContent() + "\n\"\"\"\n";
} else {
s = "\"" + description.getContent() + "\"\n";
s = "\"" + escapeJsonString(description.getContent()) + "\"\n";
}
return s;
}
Expand Down
31 changes: 31 additions & 0 deletions src/test/groovy/graphql/parser/ParserTest.groovy
Expand Up @@ -4,6 +4,7 @@ package graphql.parser
import graphql.language.Argument
import graphql.language.ArrayValue
import graphql.language.AstComparator
import graphql.language.AstPrinter
import graphql.language.BooleanValue
import graphql.language.Description
import graphql.language.Directive
Expand Down Expand Up @@ -1151,5 +1152,35 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases"""
document.getDefinitions()[0].getSourceLocation() == SourceLocation.EMPTY
}

def "escape characters correctly printed when printing AST"() {
given:
def src = "\"\\\"\" scalar A"

def env = newParserEnvironment()
.document(src)
.parserOptions(
ParserOptions.newParserOptions()
.captureIgnoredChars(true)
.build()
)
.build()

when:
// Parse the original Document
def doc = Parser.parse(env)
// Print the AST
def printed = AstPrinter.printAst(doc)
// Re-parse printed AST
def reparsed = Parser.parse(printed)

then:
noExceptionThrown() // The printed AST was re-parsed without exception

when:
def reparsedPrinted = AstPrinter.printAst(reparsed)

then:
reparsedPrinted == printed // Re-parsing and re-printing produces the same result
}

}