fix(parser,compiler): fix string serialization bugs found by fuzzing (#774)

* fuzz: add a fuzz target for string encoding/parsing

* Update to use compiler serialization

* Fix serializing single-line strings with leading whitespace

When parsing a block string, the first line does not affect the common
indent, as it would normally have no indent at all. But when
serializing a string to a block string, the first line *is* important,
namely for a single-line string that starts with whitespace.

Assume a text like this:
```
    Starts with "spaces"
```

If we skip the first line, the common indent is detected as 0,
producing this output:
```graphql
"""
    Starts with "spaces"
"""
```

Now when reparsing, the common indent is detected as 4, and you get
back:
```
Starts with "spaces"
```
which is wrong.

* Add failing test for \\""" in string

* enable debug log in fuzz test

* fix(lexer): lex \\""" as \ + \""" in block strings

Backslash is not special in block strings except in the exact sequence
`\"""`. So if you write `\\"""`, that sequence should still be treated
as the escape sequence for `"""`. This patch fixes that.

Previously, we would return to the BlockStringLiteral state on the
second `\`, and interpret the `"""` as the end of the string.

* fix rebase mistake

* changelogs

* pr number

Author: goto-bus-stop

crates/apollo-compiler/CHANGELOG.md

@@ -22,8 +22,13 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Features
 - **Add `NodeStr::from(Name)` - [goto-bus-stop], [pull/773]**
 
+## Fixes
+- **Fix serializing single-line strings with leading whitespace - [goto-bus-stop], [pull/774]**
+  Previously, the leading whitespace would get lost.
+
 [goto-bus-stop]: https://github.com/goto-bus-stop]
 [pull/773]: https://github.com/apollographql/apollo-rs/pull/773
+[pull/774]: https://github.com/apollographql/apollo-rs/pull/774
 
 # [1.0.0-beta.10](https://crates.io/crates/apollo-compiler/1.0.0-beta.10) - 2023-12-04
 

crates/apollo-compiler/src/ast/serialize.rs

@@ -918,10 +918,7 @@ fn can_be_block_string(value: &str) -> bool {
     }
 
     let common_indent = {
-        let mut lines = value.split('\n');
-        // Skip the first line, the one following """
-        lines.next();
-
+        let lines = value.split('\n');
         let each_line_indent_utf8_len = lines.filter_map(|line| {
             let after_indent = trim_start_graphql_whitespace(line);
             if !after_indent.is_empty() {

crates/apollo-compiler/test_data/ok/0039_string_literals.graphql

@@ -48,4 +48,16 @@ type Query {
 
   "Trailing quote\""
   f15: Int
+
+  "   Leading whitespace on a single line to preserve"
+  f16: Int
+
+  "   Leading whitespace in multi-line string to preserve\nNo leading whitespace on second line"
+  f17: Int
+
+  "\n   Leading empty line + indent to preserve"
+  f18: Int
+
+  "When serialized as a block string, \\\"\"\" outputs \\ in front of the escaped triple quote"
+  f19: Int
 }

crates/apollo-compiler/test_data/ok/0039_string_literals.txt

@@ -6,7 +6,7 @@ Schema {
         },
         38: SourceFile {
             path: "0039_string_literals.graphql",
-            source_text: "type Query {\n  \"LF: a\\nb\"\n  f1: Int\n\n  \"CRLF: a\\r\\nb\"\n  f2: Int\n\n  \"\"\"\n    a\n\n    b\n  \n\n  \"\"\"\n  f3: Int\n\n  \"a \\\"b\\\" c\"\n  f4: Int\n\n  \"\"\"a \\\"\"\"b\\\"\"\" c\"\"\"\n  f5: Int\n\n  \"\"\"\n  regex: \\d+\n  \"\"\"\n  f6: Int\n\n  \"\\nLeading empty line to preserve\"\n  f7: Int\n\n  \" \\nLeading whitespace-only line to preserve\"\n  f8: Int\n\n  \"Trailing empty line to preserve\\n\"\n  f9: Int\n\n  \"Trailing whitespace-only line to preserve\\n\\t\"\n  f10: Int\n\n  f11(arg: String = \"a\\nb\"): Int\n\n  f12(arg: String = \"a \\\"b\\\" c\"): Int\n\n  f13(arg: String = \"regex: \\\\d+\"): Int\n\n  \"Trailing backslash \\\\\"\n  f14: Int\n\n  \"Trailing quote\\\"\"\n  f15: Int\n}\n",
+            source_text: "type Query {\n  \"LF: a\\nb\"\n  f1: Int\n\n  \"CRLF: a\\r\\nb\"\n  f2: Int\n\n  \"\"\"\n    a\n\n    b\n  \n\n  \"\"\"\n  f3: Int\n\n  \"a \\\"b\\\" c\"\n  f4: Int\n\n  \"\"\"a \\\"\"\"b\\\"\"\" c\"\"\"\n  f5: Int\n\n  \"\"\"\n  regex: \\d+\n  \"\"\"\n  f6: Int\n\n  \"\\nLeading empty line to preserve\"\n  f7: Int\n\n  \" \\nLeading whitespace-only line to preserve\"\n  f8: Int\n\n  \"Trailing empty line to preserve\\n\"\n  f9: Int\n\n  \"Trailing whitespace-only line to preserve\\n\\t\"\n  f10: Int\n\n  f11(arg: String = \"a\\nb\"): Int\n\n  f12(arg: String = \"a \\\"b\\\" c\"): Int\n\n  f13(arg: String = \"regex: \\\\d+\"): Int\n\n  \"Trailing backslash \\\\\"\n  f14: Int\n\n  \"Trailing quote\\\"\"\n  f15: Int\n\n  \"   Leading whitespace on a single line to preserve\"\n  f16: Int\n\n  \"   Leading whitespace in multi-line string to preserve\\nNo leading whitespace on second line\"\n  f17: Int\n\n  \"\\n   Leading empty line + indent to preserve\"\n  f18: Int\n\n  \"When serialized as a block string, \\\\\\\"\\\"\\\" outputs \\\\ in front of the escaped triple quote\"\n  f19: Int\n}\n",
         },
     },
     schema_definition: SchemaDefinition {
@@ -42,7 +42,7 @@ Schema {
         "Boolean": built_in_type!("Boolean"),
         "ID": built_in_type!("ID"),
         "Query": Object(
-            0..602 @38 ObjectType {
+            0..947 @38 ObjectType {
                 description: None,
                 name: "Query",
                 implements_interfaces: {},
@@ -294,6 +294,62 @@ Schema {
                             directives: [],
                         },
                     },
+                    "f16": Component {
+                        origin: Definition,
+                        node: 604..667 @38 FieldDefinition {
+                            description: Some(
+                                "   Leading whitespace on a single line to preserve",
+                            ),
+                            name: "f16",
+                            arguments: [],
+                            ty: Named(
+                                "Int",
+                            ),
+                            directives: [],
+                        },
+                    },
+                    "f17": Component {
+                        origin: Definition,
+                        node: 671..776 @38 FieldDefinition {
+                            description: Some(
+                                "   Leading whitespace in multi-line string to preserve\nNo leading whitespace on second line",
+                            ),
+                            name: "f17",
+                            arguments: [],
+                            ty: Named(
+                                "Int",
+                            ),
+                            directives: [],
+                        },
+                    },
+                    "f18": Component {
+                        origin: Definition,
+                        node: 780..837 @38 FieldDefinition {
+                            description: Some(
+                                "\n   Leading empty line + indent to preserve",
+                            ),
+                            name: "f18",
+                            arguments: [],
+                            ty: Named(
+                                "Int",
+                            ),
+                            directives: [],
+                        },
+                    },
+                    "f19": Component {
+                        origin: Definition,
+                        node: 841..945 @38 FieldDefinition {
+                            description: Some(
+                                "When serialized as a block string, \\\"\"\" outputs \\ in front of the escaped triple quote",
+                            ),
+                            name: "f19",
+                            arguments: [],
+                            ty: Named(
+                                "Int",
+                            ),
+                            directives: [],
+                        },
+                    },
                 },
             },
         ),
@@ -307,7 +363,7 @@ ExecutableDocument {
         },
         38: SourceFile {
             path: "0039_string_literals.graphql",
-            source_text: "type Query {\n  \"LF: a\\nb\"\n  f1: Int\n\n  \"CRLF: a\\r\\nb\"\n  f2: Int\n\n  \"\"\"\n    a\n\n    b\n  \n\n  \"\"\"\n  f3: Int\n\n  \"a \\\"b\\\" c\"\n  f4: Int\n\n  \"\"\"a \\\"\"\"b\\\"\"\" c\"\"\"\n  f5: Int\n\n  \"\"\"\n  regex: \\d+\n  \"\"\"\n  f6: Int\n\n  \"\\nLeading empty line to preserve\"\n  f7: Int\n\n  \" \\nLeading whitespace-only line to preserve\"\n  f8: Int\n\n  \"Trailing empty line to preserve\\n\"\n  f9: Int\n\n  \"Trailing whitespace-only line to preserve\\n\\t\"\n  f10: Int\n\n  f11(arg: String = \"a\\nb\"): Int\n\n  f12(arg: String = \"a \\\"b\\\" c\"): Int\n\n  f13(arg: String = \"regex: \\\\d+\"): Int\n\n  \"Trailing backslash \\\\\"\n  f14: Int\n\n  \"Trailing quote\\\"\"\n  f15: Int\n}\n",
+            source_text: "type Query {\n  \"LF: a\\nb\"\n  f1: Int\n\n  \"CRLF: a\\r\\nb\"\n  f2: Int\n\n  \"\"\"\n    a\n\n    b\n  \n\n  \"\"\"\n  f3: Int\n\n  \"a \\\"b\\\" c\"\n  f4: Int\n\n  \"\"\"a \\\"\"\"b\\\"\"\" c\"\"\"\n  f5: Int\n\n  \"\"\"\n  regex: \\d+\n  \"\"\"\n  f6: Int\n\n  \"\\nLeading empty line to preserve\"\n  f7: Int\n\n  \" \\nLeading whitespace-only line to preserve\"\n  f8: Int\n\n  \"Trailing empty line to preserve\\n\"\n  f9: Int\n\n  \"Trailing whitespace-only line to preserve\\n\\t\"\n  f10: Int\n\n  f11(arg: String = \"a\\nb\"): Int\n\n  f12(arg: String = \"a \\\"b\\\" c\"): Int\n\n  f13(arg: String = \"regex: \\\\d+\"): Int\n\n  \"Trailing backslash \\\\\"\n  f14: Int\n\n  \"Trailing quote\\\"\"\n  f15: Int\n\n  \"   Leading whitespace on a single line to preserve\"\n  f16: Int\n\n  \"   Leading whitespace in multi-line string to preserve\\nNo leading whitespace on second line\"\n  f17: Int\n\n  \"\\n   Leading empty line + indent to preserve\"\n  f18: Int\n\n  \"When serialized as a block string, \\\\\\\"\\\"\\\" outputs \\\\ in front of the escaped triple quote\"\n  f19: Int\n}\n",
         },
     },
     anonymous_operation: None,

crates/apollo-compiler/test_data/serializer/ok/0039_string_literals.graphql

@@ -37,4 +37,17 @@ type Query {
   Trailing quote"
   """
   f15: Int
+  "   Leading whitespace on a single line to preserve"
+  f16: Int
+  """
+     Leading whitespace in multi-line string to preserve
+  No leading whitespace on second line
+  """
+  f17: Int
+  "\n   Leading empty line + indent to preserve"
+  f18: Int
+  """
+  When serialized as a block string, \\""" outputs \ in front of the escaped triple quote
+  """
+  f19: Int
 }

crates/apollo-parser/CHANGELOG.md

@@ -16,6 +16,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Maintenance
 
 ## Documentation -->
+
+# [x.x.x] (unreleased) - 2023-xx-xx
+
+## Fixes
+- **fix parsing `\\"""` in block string - [goto-bus-stop], [pull/774]**
+  Previously this was parsed as `\` followed by the end of the string,
+  now it's correctly parsed as `\` followed by an escaped `"""`.
+
+[goto-bus-stop]: https://github.com/goto-bus-stop
+[pull/774]: https://github.com/apollographql/apollo-rs/pull/774
+
 # [0.7.4](https://crates.io/crates/apollo-parser/0.7.4) - 2023-11-17
 
 ## Features

crates/apollo-parser/src/lexer/mod.rs

@@ -375,6 +375,11 @@ impl<'a> Cursor<'a> {
 
                         state = State::BlockStringLiteral;
                     }
+                    '\\' => {
+                        // We need to stay in the backslash state:
+                        // it's legal to write \\\""" with two literal backslashes
+                        // and then the escape sequence.
+                    }
                     _ => {
                         state = State::BlockStringLiteral;
                     }

crates/apollo-parser/test_data/lexer/ok/0008_block_string.graphql

@@ -11,4 +11,9 @@ input Filter {
 """
 input Filter {
     title: String
-}
+}
+
+"""
+\\"""
+"""
+scalar LiteralBackslashThenEscape

crates/apollo-parser/test_data/lexer/ok/0008_block_string.txt

@@ -29,4 +29,11 @@ WHITESPACE@162:163 " "
 NAME@163:169 "String"
 WHITESPACE@169:170 "\n"
 R_CURLY@170:171 "}"
-EOF@171:171
+WHITESPACE@171:173 "\n\n"
+STRING_VALUE@173:186 "\"\"\"\n\\\\\"\"\"\n\"\"\""
+WHITESPACE@186:187 "\n"
+NAME@187:193 "scalar"
+WHITESPACE@193:194 " "
+NAME@194:220 "LiteralBackslashThenEscape"
+WHITESPACE@220:221 "\n"
+EOF@221:221

fuzz/Cargo.toml

@@ -17,14 +17,12 @@ cargo-fuzz = true
 [dependencies]
 libfuzzer-sys = "0.4"
 apollo-compiler = { path = "../crates/apollo-compiler" }
+apollo-parser = { path = "../crates/apollo-parser" }
 apollo-smith = { path = "../crates/apollo-smith" }
 env_logger = "0.10.0"
 log = "0.4.14"
 similar-asserts = "1.5.0"
 
-[dependencies.apollo-parser]
-path = "../crates/apollo-parser"
-
 [[bin]]
 name = "parser"
 path = "fuzz_targets/parser.rs"
@@ -42,3 +40,9 @@ name = "reparse"
 path = "fuzz_targets/reparse.rs"
 test = false
 doc = false
+
+[[bin]]
+name = "strings"
+path = "fuzz_targets/strings.rs"
+test = false
+doc = false

fuzz/fuzz_targets/strings.rs

@@ -0,0 +1,36 @@
+#![no_main]
+use apollo_compiler::{name, Schema};
+use libfuzzer_sys::{
+    arbitrary::{Arbitrary, Unstructured},
+    fuzz_target,
+};
+use log::debug;
+
+fuzz_target!(|data: &[u8]| {
+    let _ = env_logger::try_init();
+
+    let mut u = Unstructured::new(data);
+    let string = String::arbitrary(&mut u).unwrap();
+    let mut input = Schema::new();
+    let def = input.schema_definition.make_mut();
+    def.description = Some(string.into());
+    // We can refer to a type that doesn't exist as we won't run validation
+    def.query = Some(name!("Dangling").into());
+    let doc_generated = input.to_string();
+
+    debug!("INPUT STRING: {:?}", input.schema_definition.description);
+    debug!("==== WHOLE DOCUMENT ====");
+    debug!("{doc_generated}");
+    debug!("========================");
+
+    let reparse = Schema::parse(doc_generated, "").unwrap();
+    debug!(
+        "REPARSED STRING: {:?}",
+        reparse.schema_definition.description
+    );
+
+    assert_eq!(
+        reparse.schema_definition.description,
+        input.schema_definition.description
+    );
+});