From 768eddddbb132c2a7fbc32f166763a740d485622 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 11 Feb 2024 15:44:23 +1100 Subject: [PATCH 1/5] Tidy up wording ahead of minor spec improvement https://github.com/graphql/graphql-spec/pull/1073 --- src/main/java/graphql/GraphQLError.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/GraphQLError.java b/src/main/java/graphql/GraphQLError.java index b4fc6fa600..7cd1f3a934 100644 --- a/src/main/java/graphql/GraphQLError.java +++ b/src/main/java/graphql/GraphQLError.java @@ -39,8 +39,8 @@ 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 https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format * * @return the path in list format */ From 8e5250b678273e5dbd6954c779d6d7dc061423f0 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 11 Feb 2024 16:15:59 +1100 Subject: [PATCH 2/5] Add JSON escape for single line description and test --- .../java/graphql/language/AstPrinter.java | 2 +- .../groovy/graphql/parser/ParserTest.groovy | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/language/AstPrinter.java b/src/main/java/graphql/language/AstPrinter.java index 218dabd59e..5070a4f139 100644 --- a/src/main/java/graphql/language/AstPrinter.java +++ b/src/main/java/graphql/language/AstPrinter.java @@ -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; } diff --git a/src/test/groovy/graphql/parser/ParserTest.groovy b/src/test/groovy/graphql/parser/ParserTest.groovy index 4484eb9e58..00b8fef455 100644 --- a/src/test/groovy/graphql/parser/ParserTest.groovy +++ b/src/test/groovy/graphql/parser/ParserTest.groovy @@ -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 @@ -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 + } } From 3c2de830ae84fd0ddefbdd3c960827c89e550f49 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 11 Feb 2024 16:29:58 +1100 Subject: [PATCH 3/5] Tidy error comment --- src/main/java/graphql/GraphQLError.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/GraphQLError.java b/src/main/java/graphql/GraphQLError.java index 7cd1f3a934..c18752b14a 100644 --- a/src/main/java/graphql/GraphQLError.java +++ b/src/main/java/graphql/GraphQLError.java @@ -39,8 +39,10 @@ public interface GraphQLError extends Serializable { ErrorClassification getErrorType(); /** - * The graphql spec says that the (optional) path field of any error must be a list - * of path entries https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format + * 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 * * @return the path in list format */ From 9d469579b6ebee44a87aa61f7d2d94b65718886d Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 18 Feb 2024 22:31:01 +1100 Subject: [PATCH 4/5] Add more test cases --- src/test/groovy/graphql/parser/ParserTest.groovy | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/groovy/graphql/parser/ParserTest.groovy b/src/test/groovy/graphql/parser/ParserTest.groovy index 00b8fef455..d76da7b63a 100644 --- a/src/test/groovy/graphql/parser/ParserTest.groovy +++ b/src/test/groovy/graphql/parser/ParserTest.groovy @@ -1154,8 +1154,6 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases""" def "escape characters correctly printed when printing AST"() { given: - def src = "\"\\\"\" scalar A" - def env = newParserEnvironment() .document(src) .parserOptions( @@ -1178,9 +1176,17 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases""" when: def reparsedPrinted = AstPrinter.printAst(reparsed) - + then: reparsedPrinted == printed // Re-parsing and re-printing produces the same result + + where: + src | _ + "\"\\\"\" scalar A" | _ + "\"\f\" scalar A" | _ + "\"\b\" scalar A" | _ + "\"\t\" scalar A" | _ + "\"\\\" scalar A" | _ } } From 7ee3cc5af82f7fd86bfb3bc9f78a59d8dfeaddea Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 18 Feb 2024 22:39:23 +1100 Subject: [PATCH 5/5] Remove invalid schema example --- src/test/groovy/graphql/parser/ParserTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/groovy/graphql/parser/ParserTest.groovy b/src/test/groovy/graphql/parser/ParserTest.groovy index d76da7b63a..42e604e9a6 100644 --- a/src/test/groovy/graphql/parser/ParserTest.groovy +++ b/src/test/groovy/graphql/parser/ParserTest.groovy @@ -1186,7 +1186,6 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases""" "\"\f\" scalar A" | _ "\"\b\" scalar A" | _ "\"\t\" scalar A" | _ - "\"\\\" scalar A" | _ } }