fix: assure all kinds of trees can be parsed. (#1096)

Fix the slow and the fast-path tree-parsers to be able to cope
with a greater variety of trees.

Author: Byron

gix-object/benches/decode_objects.rs

@@ -19,10 +19,10 @@ fn parse_tag(c: &mut Criterion) {
 }
 
 fn parse_tree(c: &mut Criterion) {
-    c.bench_function("TreeRef(sig)", |b| {
+    c.bench_function("TreeRef()", |b| {
         b.iter(|| black_box(gix_object::TreeRef::from_bytes(TREE)).unwrap())
     });
-    c.bench_function("TreeRefIter(sig)", |b| {
+    c.bench_function("TreeRefIter()", |b| {
         b.iter(|| black_box(gix_object::TreeRefIter::from_bytes(TREE).count()))
     });
 }

gix-object/src/tree/ref_iter.rs

@@ -116,17 +116,9 @@ mod decode {
     use std::convert::TryFrom;
 
     use bstr::ByteSlice;
-    use winnow::{
-        combinator::{eof, repeat, terminated},
-        error::ParserError,
-        prelude::*,
-        stream::AsChar,
-        token::{take, take_while},
-    };
+    use winnow::{error::ParserError, prelude::*};
 
-    use crate::{parse::SPACE, tree, tree::EntryRef, TreeRef};
-
-    const NULL: &[u8] = b"\0";
+    use crate::{tree, tree::EntryRef, TreeRef};
 
     pub fn fast_entry(i: &[u8]) -> Option<(&[u8], EntryRef<'_>)> {
         let mut mode = 0u32;
@@ -157,24 +149,20 @@ mod decode {
         ))
     }
 
-    pub fn entry<'a, E: ParserError<&'a [u8]>>(i: &mut &'a [u8]) -> PResult<EntryRef<'a>, E> {
-        (
-            terminated(take_while(5..=6, AsChar::is_dec_digit), SPACE)
-                .verify_map(|mode| tree::EntryMode::try_from(mode).ok()),
-            terminated(take_while(1.., |b| b != NULL[0]), NULL),
-            take(20u8), // TODO(SHA256): make this compatible with other hash lengths
-        )
-            .map(|(mode, filename, oid): (_, &[u8], _)| EntryRef {
-                mode,
-                filename: filename.as_bstr(),
-                oid: gix_hash::oid::try_from_bytes(oid).expect("we counted exactly 20 bytes"),
-            })
-            .parse_next(i)
-    }
-
     pub fn tree<'a, E: ParserError<&'a [u8]>>(i: &mut &'a [u8]) -> PResult<TreeRef<'a>, E> {
-        terminated(repeat(0.., entry), eof)
-            .map(|entries| TreeRef { entries })
-            .parse_next(i)
+        let mut out = Vec::new();
+        let mut i = &**i;
+        while !i.is_empty() {
+            let Some((rest, entry)) = fast_entry(i) else {
+                #[allow(clippy::unit_arg)]
+                return Err(winnow::error::ErrMode::from_error_kind(
+                    &i,
+                    winnow::error::ErrorKind::Verify,
+                ));
+            };
+            i = rest;
+            out.push(entry);
+        }
+        Ok(TreeRef { entries: out })
     }
 }

gix-object/tests/fixtures/tree/special-1.tree

@@ -56,7 +56,7 @@ mod iter {
 }
 
 mod from_bytes {
-    use gix_object::{bstr::ByteSlice, tree, tree::EntryRef, TreeRef};
+    use gix_object::{bstr::ByteSlice, tree, tree::EntryRef, TreeRef, TreeRefIter};
 
     use crate::{fixture_name, hex_to_id};
 
@@ -108,24 +108,27 @@ mod from_bytes {
     }
 
     #[test]
-    fn maybe_special() -> crate::Result {
-        assert_eq!(
-            TreeRef::from_bytes(&fixture_name("tree", "maybe-special.tree"))?
-                .entries
-                .len(),
-            160
-        );
-        Ok(())
-    }
-
-    #[test]
-    fn definitely_special() -> crate::Result {
-        assert_eq!(
-            TreeRef::from_bytes(&fixture_name("tree", "definitely-special.tree"))?
-                .entries
-                .len(),
-            19
-        );
+    fn special_trees() -> crate::Result {
+        for (name, expected_entry_count) in [
+            ("maybe-special", 160),
+            ("definitely-special", 19),
+            ("special-1", 5),
+            ("special-2", 18),
+            ("special-3", 5),
+            ("special-4", 18),
+        ] {
+            let fixture = fixture_name("tree", &format!("{}.tree", name));
+            assert_eq!(
+                TreeRef::from_bytes(&fixture)?.entries.len(),
+                expected_entry_count,
+                "{name}"
+            );
+            assert_eq!(
+                TreeRefIter::from_bytes(&fixture).map(Result::unwrap).count(),
+                expected_entry_count,
+                "{name}"
+            );
+        }
         Ok(())
     }
 }