simd-doc: verify newlines and proper end-of-endpoint in fallbacks

We weren't properly verifying that the full deserialization buffer was
consumed through to the newline pivot in fallback handlers.
Do so, and improve test coverage slightly.

Also update simdjson usage to verify that a document extent ends in a
newline. This is to catch cases like `{"doc":1} {"doc":2}\n` which are
allowed by simd-json normally, but which are reject by fallback parsing.

Note this will cause the fallback handler to be used without causing an
explicit error if leading whitespace appears in a buffer,
such as `{"doc":1}\n {"doc":2}\n".

Other updates to the garden-path simdjson flow are not required because we
give simdjson only complete chunks of documents with a trailing newline:
it parses all documents of the input as a single operation and errors
out if *any* documents fail to parse, triggering fallback handling.

Author: jgraettinger

crates/simd-doc/src/ffi/simd-doc.hpp

@@ -454,7 +454,7 @@ void parse_node(const Allocator &alloc, dom::element_type typ, dom::element elem
 class Parser
 {
 public:
-    Parser(size_t capacity) : parser(capacity){};
+    Parser(size_t capacity) : parser(capacity) {};
     void parse(const rust::Slice<const uint8_t> input, int64_t offset, const Allocator &alloc, HeapNode &node, Parsed &output);
     void transcode(const rust::Slice<const uint8_t> input, Transcoded &output);
 
@@ -478,7 +478,13 @@ inline void Parser::parse(const rust::Slice<const uint8_t> input, int64_t offset
     {
         dom::element elem = *it;
         parse_node(alloc, elem.type(), elem, node);
-        ++it; // Step to next document.
+
+        ++it; // Step to the next document and verify trailing newline.
+        if (input.data()[it.current_index() - 1] != '\n')
+        {
+            throw std::out_of_range("missing trailing newline");
+        }
+
         complete(output, node, offset + static_cast<int64_t>(it.current_index()));
     }
 
@@ -511,7 +517,12 @@ inline void Parser::transcode(const rust::Slice<const uint8_t> input, Transcoded
         dom::element elem = *it;
         pnode root = transcode_node(buf, elem.type(), elem);
         place_array(buf, &root, 1);
-        ++it; // Step to next document.
+
+        ++it; // Step to the next document and verify trailing newline.
+        if (input.data()[it.current_index() - 1] != '\n')
+        {
+            throw std::out_of_range("missing trailing newline");
+        }
 
         // Update and re-write header now that we know the next offset and length.
         header.u32.l = static_cast<uint32_t>(it.current_index());

crates/simd-doc/src/lib.rs

@@ -72,13 +72,15 @@ impl Parser {
         );
         let mut buf = std::mem::take(&mut self.whole);
         buf.extend_from_slice(input);
+        buf.push(b'\n');
 
         if let Err(err) = parse_simd(&mut buf, 0, alloc, &mut self.parsed, &mut self.ffi) {
             self.parsed.clear(); // Clear a partial simd parsing.
             tracing::debug!(%err, "simdjson JSON parse-one failed; using fallback");
 
             let mut de = serde_json::Deserializer::from_slice(&buf);
             let node = doc::HeapNode::from_serde(&mut de, &alloc)?;
+            () = de.end()?;
             self.parsed.push((node, 0));
         }
         let mut parsed = self.parsed.drain(..);
@@ -292,7 +294,10 @@ fn parse_fallback<'a>(
         let pivot = memchr::memchr(b'\n', &input).expect("input always ends with newline") + 1;
 
         let mut de = serde_json::Deserializer::from_slice(&input[..pivot]);
-        match doc::HeapNode::from_serde(&mut de, &alloc) {
+        match doc::HeapNode::from_serde(&mut de, &alloc).and_then(|node| {
+            () = de.end()?;
+            Ok(node)
+        }) {
             Ok(node) => {
                 input = &input[pivot..];
                 consumed += pivot;
@@ -327,7 +332,10 @@ fn transcode_fallback(
         let pivot = memchr::memchr(b'\n', &input).expect("input always ends with newline") + 1;
 
         let mut de = serde_json::Deserializer::from_slice(&input[..pivot]);
-        match doc::HeapNode::from_serde(&mut de, &alloc) {
+        match doc::HeapNode::from_serde(&mut de, &alloc).and_then(|node| {
+            () = de.end()?;
+            Ok(node)
+        }) {
             Ok(node) => {
                 input = &input[pivot..];
                 consumed += pivot;

crates/simd-doc/src/tests/fixtures.rs

@@ -108,7 +108,8 @@ fn test_incomplete_row_parsing() {
     let mut inputs = Vec::new();
     inputs.push(r###"st": {"this": "is", "a": "doc"}}"###.as_bytes());
     inputs.push(r###""test": {"this": "is", "a": "doc"}}"###.as_bytes());
-    inputs.push(r###"{"test": 5}, {"test": 6"}]"###.as_bytes());
+    inputs.push(r###"{"test": 5}, {"test": 6}]"###.as_bytes());
+    inputs.push(r###"{"test": 7}  {"test": 8}"###.as_bytes());
     inputs.push(r###"{"real": "object"}"###.as_bytes());
 
     let real = r###"{"this": "is a real doc"}"###;
@@ -166,76 +167,91 @@ fn test_incomplete_row_parsing() {
             }
         }
 
-        debug_assert_eq!(parse_snap, transcode_snap);
+        assert_eq!(parse_snap, transcode_snap);
 
         snaps.push((String::from_utf8(real_input).unwrap(), parse_snap));
     }
 
     insta::assert_debug_snapshot!(snaps, @r###"
-      [
-          (
-              "st\": {\"this\": \"is\", \"a\": \"doc\"}}\n{\"this\": \"is a real doc\"}\n",
-              [
-                  (
-                      -1,
-                      String("expected value at line 1 column 1 @ 0..33"),
-                  ),
-                  (
-                      59,
-                      Object {
-                          "this": String("is a real doc"),
-                      },
-                  ),
-              ],
-          ),
-          (
-              "\"test\": {\"this\": \"is\", \"a\": \"doc\"}}\n{\"this\": \"is a real doc\"}\n",
-              [
-                  (
-                      -1,
-                      String("incomplete row @ 0..45"),
-                  ),
-                  (
-                      62,
-                      Object {
-                          "this": String("is a real doc"),
-                      },
-                  ),
-              ],
-          ),
-          (
-              "{\"test\": 5}, {\"test\": 6\"}]\n{\"this\": \"is a real doc\"}\n",
-              [
-                  (
-                      -1,
-                      String("incomplete row @ 0..33"),
-                  ),
-                  (
-                      53,
-                      Object {
-                          "this": String("is a real doc"),
-                      },
-                  ),
-              ],
-          ),
-          (
-              "{\"real\": \"object\"}\n{\"this\": \"is a real doc\"}\n",
-              [
-                  (
-                      19,
-                      Object {
-                          "real": String("object"),
-                      },
-                  ),
-                  (
-                      45,
-                      Object {
-                          "this": String("is a real doc"),
-                      },
-                  ),
-              ],
-          ),
-      ]
+    [
+        (
+            "st\": {\"this\": \"is\", \"a\": \"doc\"}}\n{\"this\": \"is a real doc\"}\n",
+            [
+                (
+                    -1,
+                    String("expected value at line 1 column 1 @ 0..33"),
+                ),
+                (
+                    59,
+                    Object {
+                        "this": String("is a real doc"),
+                    },
+                ),
+            ],
+        ),
+        (
+            "\"test\": {\"this\": \"is\", \"a\": \"doc\"}}\n{\"this\": \"is a real doc\"}\n",
+            [
+                (
+                    -1,
+                    String("trailing characters at line 1 column 7 @ 0..36"),
+                ),
+                (
+                    62,
+                    Object {
+                        "this": String("is a real doc"),
+                    },
+                ),
+            ],
+        ),
+        (
+            "{\"test\": 5}, {\"test\": 6}]\n{\"this\": \"is a real doc\"}\n",
+            [
+                (
+                    -1,
+                    String("trailing characters at line 1 column 12 @ 0..26"),
+                ),
+                (
+                    52,
+                    Object {
+                        "this": String("is a real doc"),
+                    },
+                ),
+            ],
+        ),
+        (
+            "{\"test\": 7}  {\"test\": 8}\n{\"this\": \"is a real doc\"}\n",
+            [
+                (
+                    -1,
+                    String("trailing characters at line 1 column 14 @ 0..25"),
+                ),
+                (
+                    51,
+                    Object {
+                        "this": String("is a real doc"),
+                    },
+                ),
+            ],
+        ),
+        (
+            "{\"real\": \"object\"}\n{\"this\": \"is a real doc\"}\n",
+            [
+                (
+                    19,
+                    Object {
+                        "real": String("object"),
+                    },
+                ),
+                (
+                    45,
+                    Object {
+                        "this": String("is a real doc"),
+                    },
+                ),
+            ],
+        ),
+    ]
     "###);
 }
 
@@ -352,13 +368,26 @@ fn test_basic_parser_apis() {
     let doc = parser.parse_one(input.as_bytes(), &alloc).unwrap();
     snap.push((0, doc.to_debug_json_value()));
 
-    let input = input.repeat(3);
-    insta::assert_debug_snapshot!(parser.parse_one(input.as_bytes(), &alloc).unwrap_err(), @r###"
-    Custom {
-        kind: InvalidData,
-        error: "expected one document, but parsed 3",
+    {
+        let input = vec![input.as_str(), input.as_str(), input.as_str()].join("\n");
+        let input = input.as_bytes();
+
+        // Verify error handling of multiple valid documents in the input.
+        insta::assert_debug_snapshot!(parser.parse_one(input, &alloc).unwrap_err(), @r###"
+        Custom {
+            kind: InvalidData,
+            error: "expected one document, but parsed 3",
+        }
+        "###);
+
+        // Verify error handling of a trailing partial document.
+        insta::assert_debug_snapshot!(parser.parse_one(&input[..input.len()/2], &alloc).unwrap_err(), @r###"
+        Custom {
+            kind: InvalidData,
+            error: Error("trailing characters", line: 11, column: 1),
+        }
+        "###);
     }
-    "###);
 
     insta::assert_json_snapshot!(snap, @r###"
     [