refactor(tvix/castore): add PathComponent type for checked components r/8506

This encodes a verified component on the type level. Internally, it
contains a bytes::Bytes.

The castore Path/PathBuf component() and file_name() methods now
return this type, the old ones returning bytes were renamed to
component_bytes() and component_file_name() respectively.

We can drop the directory_reject_invalid_name test - it's not possible
anymore to pass an invalid name to Directories::add.
Invalid names in the Directory proto are still being tested to be
rejected in the validate_invalid_names tests.

Change-Id: Ide4d16415dfd50b7e2d7e0c36d42a3bbeeb9b6c5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12217
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI

1 files changed, 17 insertions, 47 deletions

diff --git a/tvix/castore/src/nodes/directory.rs b/tvix/castore/src/nodes/directory.rs
index a2c520358929..03db30b52408 100644
--- a/tvix/castore/src/nodes/directory.rs
+++ b/tvix/castore/src/nodes/directory.rs
@@ -1,6 +1,6 @@
 use std::collections::BTreeMap;
 
-use crate::{errors::DirectoryError, proto, B3Digest, Node};
+use crate::{errors::DirectoryError, path::PathComponent, proto, B3Digest, Node};
 
 /// A Directory contains nodes, which can be Directory, File or Symlink nodes.
 /// It attached names to these nodes, which is the basename in that directory.
@@ -10,7 +10,7 @@ use crate::{errors::DirectoryError, proto, B3Digest, Node};
 ///  - MUST be unique across all three lists
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct Directory {
-    nodes: BTreeMap<bytes::Bytes, Node>,
+    nodes: BTreeMap<PathComponent, Node>,
 }
 
 impl Directory {
@@ -46,31 +46,17 @@ impl Directory {
     /// Allows iterating over all nodes (directories, files and symlinks)
     /// For each, it returns a tuple of its name and node.
     /// The elements are sorted by their names.
-    pub fn nodes(&self) -> impl Iterator<Item = (&bytes::Bytes, &Node)> + Send + Sync + '_ {
+    pub fn nodes(&self) -> impl Iterator<Item = (&PathComponent, &Node)> + Send + Sync + '_ {
         self.nodes.iter()
     }
 
-    /// Checks a Node name for validity as a directory entry
-    /// We disallow slashes, null bytes, '.', '..' and the empty string.
-    pub(crate) fn is_valid_name(name: &[u8]) -> bool {
-        !(name.is_empty()
-            || name == b".."
-            || name == b"."
-            || name.contains(&0x00)
-            || name.contains(&b'/'))
-    }
-
     /// Adds the specified [Node] to the [Directory] with a given name.
     ///
     /// Inserting an element that already exists with the same name in the directory will yield an
     /// error.
     /// Inserting an element will validate that its name fulfills the
     /// requirements for directory entries and yield an error if it is not.
-    pub fn add(&mut self, name: bytes::Bytes, node: Node) -> Result<(), DirectoryError> {
-        if !Self::is_valid_name(&name) {
-            return Err(DirectoryError::InvalidName(name));
-        }
-
+    pub fn add(&mut self, name: PathComponent, node: Node) -> Result<(), DirectoryError> {
         // Check that the even after adding this new directory entry, the size calculation will not
         // overflow
         // FUTUREWORK: add some sort of batch add interface which only does this check once with
@@ -91,7 +77,7 @@ impl Directory {
                 Ok(())
             }
             std::collections::btree_map::Entry::Occupied(occupied) => {
-                Err(DirectoryError::DuplicateName(occupied.key().to_vec()))
+                Err(DirectoryError::DuplicateName(occupied.key().to_owned()))
             }
         }
     }
@@ -112,7 +98,7 @@ mod test {
         let mut d = Directory::new();
 
         d.add(
-            "b".into(),
+            "b".try_into().unwrap(),
             Node::Directory {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -120,7 +106,7 @@ mod test {
         )
         .unwrap();
         d.add(
-            "a".into(),
+            "a".try_into().unwrap(),
             Node::Directory {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -128,7 +114,7 @@ mod test {
         )
         .unwrap();
         d.add(
-            "z".into(),
+            "z".try_into().unwrap(),
             Node::Directory {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -137,7 +123,7 @@ mod test {
         .unwrap();
 
         d.add(
-            "f".into(),
+            "f".try_into().unwrap(),
             Node::File {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -146,7 +132,7 @@ mod test {
         )
         .unwrap();
         d.add(
-            "c".into(),
+            "c".try_into().unwrap(),
             Node::File {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -155,7 +141,7 @@ mod test {
         )
         .unwrap();
         d.add(
-            "g".into(),
+            "g".try_into().unwrap(),
             Node::File {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -165,21 +151,21 @@ mod test {
         .unwrap();
 
         d.add(
-            "t".into(),
+            "t".try_into().unwrap(),
             Node::Symlink {
                 target: "a".try_into().unwrap(),
             },
         )
         .unwrap();
         d.add(
-            "o".into(),
+            "o".try_into().unwrap(),
             Node::Symlink {
                 target: "a".try_into().unwrap(),
             },
         )
         .unwrap();
         d.add(
-            "e".into(),
+            "e".try_into().unwrap(),
             Node::Symlink {
                 target: "a".try_into().unwrap(),
             },
@@ -197,7 +183,7 @@ mod test {
 
         assert_eq!(
             d.add(
-                "foo".into(),
+                "foo".try_into().unwrap(),
                 Node::Directory {
                     digest: DUMMY_DIGEST.clone(),
                     size: u64::MAX
@@ -212,7 +198,7 @@ mod test {
         let mut d = Directory::new();
 
         d.add(
-            "a".into(),
+            "a".try_into().unwrap(),
             Node::Directory {
                 digest: DUMMY_DIGEST.clone(),
                 size: 1,
@@ -223,7 +209,7 @@ mod test {
             format!(
                 "{}",
                 d.add(
-                    "a".into(),
+                    "a".try_into().unwrap(),
                     Node::File {
                         digest: DUMMY_DIGEST.clone(),
                         size: 1,
@@ -235,20 +221,4 @@ mod test {
             "\"a\" is a duplicate name"
         );
     }
-
-    /// Attempt to add a directory entry with a name which should be rejected.
-    #[tokio::test]
-    async fn directory_reject_invalid_name() {
-        let mut dir = Directory::new();
-        assert!(
-            dir.add(
-                "".into(), // wrong! can not be added to directory
-                Node::Symlink {
-                    target: "doesntmatter".try_into().unwrap(),
-                },
-            )
-            .is_err(),
-            "invalid symlink entry be rejected"
-        );
-    }
 }