refactor(tvix/castore): add try_into_anonymous_node, rename to try_* r/8836

We have two places where we parse protos and want their names to be
empty:

 - Receiving a root node in a nar-bridge NAR request
 - Processing the CalculateNAR gRPC call

We don't have any place where we want to keep a name as bytes::Bytes
around, yet we used the `into_name_bytes_and_node` method.

It was also a bit wrongly named - it wasn't very clear the name was
not validated, and that the function may fail.

This moves the "splitting off the name as bytes::Bytes" part into a
private helper, only leaving the `try_into_name_and_node` and
`try_into_anonymous_node` methods around.

Change-Id: I2c7fd9871d49ec67450d7efa6a30d96197fb319c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12664
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Marijan Petričević <marijan.petricevic94@gmail.com>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>

3 files changed, 49 insertions, 16 deletions

diff --git a/tvix/castore/src/errors.rs b/tvix/castore/src/errors.rs
index bf8fd236dbef..1c4605200842 100644
--- a/tvix/castore/src/errors.rs
+++ b/tvix/castore/src/errors.rs
@@ -52,6 +52,10 @@ pub enum DirectoryError {
     /// Invalid name encountered
     #[error("Invalid name: {0}")]
     InvalidName(PathComponentError),
+    /// This can occur if a protobuf node with a name is passed where we expect
+    /// it to be anonymous.
+    #[error("Name is set when it shouldn't")]
+    NameInAnonymousNode,
     /// Elements are not in sorted order. Can only happen on protos
     #[error("{:?} is not sorted", .0.as_bstr())]
     WrongSorting(bytes::Bytes),
diff --git a/tvix/castore/src/proto/mod.rs b/tvix/castore/src/proto/mod.rs
index 15e55ad049cf..89c68a4ad97b 100644
--- a/tvix/castore/src/proto/mod.rs
+++ b/tvix/castore/src/proto/mod.rs
@@ -124,7 +124,7 @@ impl TryFrom<Directory> for crate::Directory {
                 Node {
                     node: Some(node::Node::Directory(e)),
                 }
-                .into_name_and_node()?,
+                .try_into_name_and_node()?,
             );
         }
 
@@ -133,7 +133,7 @@ impl TryFrom<Directory> for crate::Directory {
                 Node {
                     node: Some(node::Node::File(e)),
                 }
-                .into_name_and_node()?,
+                .try_into_name_and_node()?,
             )
         }
 
@@ -142,7 +142,7 @@ impl TryFrom<Directory> for crate::Directory {
                 Node {
                     node: Some(node::Node::Symlink(e)),
                 }
-                .into_name_and_node()?,
+                .try_into_name_and_node()?,
             )
         }
 
@@ -191,22 +191,20 @@ impl From<crate::Directory> for Directory {
 }
 
 impl Node {
-    /// Converts a proto [Node] to a [crate::Node], and splits off the name.
-    pub fn into_name_and_node(self) -> Result<(PathComponent, crate::Node), DirectoryError> {
-        let (unvalidated_name, node) = self.into_name_bytes_and_node()?;
+    /// Converts a proto [Node] to a [crate::Node], and splits off the name as a [PathComponent].
+    pub fn try_into_name_and_node(self) -> Result<(PathComponent, crate::Node), DirectoryError> {
+        let (name_bytes, node) = self.try_into_unchecked_name_and_checked_node()?;
         Ok((
-            unvalidated_name
-                .try_into()
-                .map_err(DirectoryError::InvalidName)?,
+            name_bytes.try_into().map_err(DirectoryError::InvalidName)?,
             node,
         ))
     }
 
-    /// Converts a proto [Node] to a [crate::Node], and splits off the name and returns it as a
-    /// [bytes::Bytes].
-    ///
-    /// Note: the returned name is not validated.
-    pub fn into_name_bytes_and_node(self) -> Result<(bytes::Bytes, crate::Node), DirectoryError> {
+    /// Converts a proto [Node] to a [crate::Node], and splits off the name as a
+    /// [bytes::Bytes] without doing any checking of it.
+    fn try_into_unchecked_name_and_checked_node(
+        self,
+    ) -> Result<(bytes::Bytes, crate::Node), DirectoryError> {
         match self.node.ok_or_else(|| DirectoryError::NoNodeSet)? {
             node::Node::Directory(n) => {
                 let digest = B3Digest::try_from(n.digest)
@@ -247,6 +245,20 @@ impl Node {
         }
     }
 
+    /// Converts a proto [Node] to a [crate::Node], and splits off the name and returns it as a
+    /// [bytes::Bytes].
+    ///
+    /// The name must be empty.
+    pub fn try_into_anonymous_node(self) -> Result<crate::Node, DirectoryError> {
+        let (name, node) = Self::try_into_unchecked_name_and_checked_node(self)?;
+
+        if !name.is_empty() {
+            return Err(DirectoryError::NameInAnonymousNode);
+        }
+
+        Ok(node)
+    }
+
     /// Constructs a [Node] from a name and [crate::Node].
     /// The name is a [bytes::Bytes], not a [PathComponent], as we have use an
     /// empty name in some places.
diff --git a/tvix/castore/src/proto/tests/mod.rs b/tvix/castore/src/proto/tests/mod.rs
index 8efb92870374..9f6330914bff 100644
--- a/tvix/castore/src/proto/tests/mod.rs
+++ b/tvix/castore/src/proto/tests/mod.rs
@@ -1,4 +1,5 @@
 use super::{node, Node, SymlinkNode};
+use crate::DirectoryError;
 
 mod directory;
 
@@ -11,7 +12,7 @@ fn convert_symlink_empty_target_invalid() {
             target: "".into(),
         })),
     }
-    .into_name_and_node()
+    .try_into_name_and_node()
     .expect_err("must fail validation");
 }
 
@@ -25,6 +26,22 @@ fn convert_symlink_target_null_byte_invalid() {
             target: "foo\0".into(),
         })),
     }
-    .into_name_and_node()
+    .try_into_name_and_node()
     .expect_err("must fail validation");
 }
+
+/// Create a node with a name, and ensure our ano
+#[test]
+fn convert_anonymous_with_name_fail() {
+    assert_eq!(
+        DirectoryError::NameInAnonymousNode,
+        Node {
+            node: Some(node::Node::Symlink(SymlinkNode {
+                name: "foo".into(),
+                target: "somewhereelse".into(),
+            })),
+        }
+        .try_into_anonymous_node()
+        .expect_err("must fail")
+    )
+}