From 3389c550b92d8b631f75e5a77e244fe698e4b4b2 Mon Sep 17 00:00:00 2001
From: sinavir <tvix@sinavir.fr>
Date: Sun, 22 Sep 2024 21:28:39 +0200
Subject: [PATCH] fix(nar_bridge): Don't check if root node has a name in nar
 generation

Nar-bridge tried to parse the name of the protobuf node encoded in the
URL into a PathComponent but this name was empty, leading to an error
when the user tried to retrieve the nar file. This commit removes this
useless step and fixes the errors caused by it

Change-Id: I2996cdd2e0107133e502748947298f512f1cc521
---
 tvix/castore/src/errors.rs    |  2 +-
 tvix/castore/src/proto/mod.rs | 40 ++++++++++++++++++++++++-----------
 tvix/nar-bridge/src/nar.rs    |  7 +-----
 3 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/tvix/castore/src/errors.rs b/tvix/castore/src/errors.rs
index 7b5d1a422..dff6e0712 100644
--- a/tvix/castore/src/errors.rs
+++ b/tvix/castore/src/errors.rs
@@ -46,7 +46,7 @@ pub enum DirectoryError {
     DuplicateName(PathComponent),
     /// Node failed validation
     #[error("invalid node with name {}: {:?}", .0, .1.to_string())]
-    InvalidNode(PathComponent, ValidateNodeError),
+    InvalidNode(String, ValidateNodeError),
     #[error("Total size exceeds u64::MAX")]
     SizeOverflow,
     /// Invalid name encountered
diff --git a/tvix/castore/src/proto/mod.rs b/tvix/castore/src/proto/mod.rs
index 8bc74b412..82e362b69 100644
--- a/tvix/castore/src/proto/mod.rs
+++ b/tvix/castore/src/proto/mod.rs
@@ -1,6 +1,9 @@
 use prost::Message;
+
 use std::cmp::Ordering;
 
+use bstr::ByteSlice;
+
 mod grpc_blobservice_wrapper;
 mod grpc_directoryservice_wrapper;
 
@@ -190,25 +193,40 @@ impl From<crate::Directory> for Directory {
 }
 
 impl Node {
+    /// Converts a proto [Node] to a [crate::Node], and splits off the name.
+    pub fn into_node(self) -> Result<crate::Node, DirectoryError> {
+        let (_, node) = self.into_unvalidated_name_and_node()?;
+        Ok(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_unvalidated_name_and_node()?;
+        Ok((
+            unvalidated_name
+                .try_into()
+                .map_err(DirectoryError::InvalidName)?,
+            node,
+        ))
+    }
+
+    fn into_unvalidated_name_and_node(self) -> Result<(bytes::Bytes, crate::Node), DirectoryError> {
         match self.node.ok_or_else(|| DirectoryError::NoNodeSet)? {
             node::Node::Directory(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
-                let digest = B3Digest::try_from(n.digest)
-                    .map_err(|e| DirectoryError::InvalidNode(name.clone(), e.into()))?;
+                let digest = B3Digest::try_from(n.digest).map_err(|e| {
+                    DirectoryError::InvalidNode(n.name.as_bstr().to_string(), e.into())
+                })?;
 
                 let node = crate::Node::Directory {
                     digest,
                     size: n.size,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
             node::Node::File(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
-                let digest = B3Digest::try_from(n.digest)
-                    .map_err(|e| DirectoryError::InvalidNode(name.clone(), e.into()))?;
+                let digest = B3Digest::try_from(n.digest).map_err(|e| {
+                    DirectoryError::InvalidNode(n.name.as_bstr().to_string(), e.into())
+                })?;
 
                 let node = crate::Node::File {
                     digest,
@@ -216,22 +234,20 @@ impl Node {
                     executable: n.executable,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
 
             node::Node::Symlink(n) => {
-                let name: PathComponent = n.name.try_into().map_err(DirectoryError::InvalidName)?;
-
                 let node = crate::Node::Symlink {
                     target: n.target.try_into().map_err(|e| {
                         DirectoryError::InvalidNode(
-                            name.clone(),
+                            n.name.as_bstr().to_string(),
                             crate::ValidateNodeError::InvalidSymlinkTarget(e),
                         )
                     })?,
                 };
 
-                Ok((name, node))
+                Ok((n.name, node))
             }
         }
     }
diff --git a/tvix/nar-bridge/src/nar.rs b/tvix/nar-bridge/src/nar.rs
index 70d9d644c..c683b0ef2 100644
--- a/tvix/nar-bridge/src/nar.rs
+++ b/tvix/nar-bridge/src/nar.rs
@@ -52,16 +52,11 @@ pub async fn get(
             StatusCode::NOT_FOUND
         })?;
 
-    let (root_name, root_node) = root_node.into_name_and_node().map_err(|e| {
+    let root_node = root_node.into_node().map_err(|e| {
         warn!(err=%e, "root node validation failed");
         StatusCode::BAD_REQUEST
     })?;
 
-    if !root_name.as_ref().is_empty() {
-        warn!("root node has name, which it shouldn't");
-        return Err(StatusCode::BAD_REQUEST);
-    }
-
     let (w, r) = tokio::io::duplex(1024 * 8);
 
     // spawn a task rendering the NAR to the client