diff options
-rw-r--r-- | tvix/build/src/buildservice/oci.rs | 2 | ||||
-rw-r--r-- | tvix/build/src/oci/spec.rs | 2 | ||||
-rw-r--r-- | tvix/build/src/proto/mod.rs | 2 | ||||
-rw-r--r-- | tvix/castore/src/errors.rs | 4 | ||||
-rw-r--r-- | tvix/castore/src/proto/mod.rs | 40 | ||||
-rw-r--r-- | tvix/castore/src/proto/tests/mod.rs | 21 | ||||
-rw-r--r-- | tvix/glue/src/tvix_store_io.rs | 6 | ||||
-rw-r--r-- | tvix/nar-bridge/src/nar.rs | 7 | ||||
-rw-r--r-- | tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs | 4 | ||||
-rw-r--r-- | tvix/store/src/proto/mod.rs | 2 |
10 files changed, 59 insertions, 31 deletions
diff --git a/tvix/build/src/buildservice/oci.rs b/tvix/build/src/buildservice/oci.rs index 7b88518e924f..89efbb4285d0 100644 --- a/tvix/build/src/buildservice/oci.rs +++ b/tvix/build/src/buildservice/oci.rs @@ -131,7 +131,7 @@ where let root_nodes: BTreeMap<PathComponent, Node> = BTreeMap::from_iter(request.inputs.iter().map(|input| { // We know from validation this is Some. - input.clone().into_name_and_node().unwrap() + input.clone().try_into_name_and_node().unwrap() })); let patterns = ReferencePattern::new(request.refscan_needles.clone()); // NOTE: impl Drop for FuseDaemon unmounts, so if the call is cancelled, umount. diff --git a/tvix/build/src/oci/spec.rs b/tvix/build/src/oci/spec.rs index ce70ad91e9a9..e80e442f0350 100644 --- a/tvix/build/src/oci/spec.rs +++ b/tvix/build/src/oci/spec.rs @@ -266,7 +266,7 @@ fn configure_mounts<'a>( for input in inputs { let (input_name, _input) = input .clone() - .into_name_and_node() + .try_into_name_and_node() .expect("invalid input name"); let input_name = std::str::from_utf8(input_name.as_ref()).expect("invalid input name"); diff --git a/tvix/build/src/proto/mod.rs b/tvix/build/src/proto/mod.rs index e6c94f5b56c9..0e106bb4cf63 100644 --- a/tvix/build/src/proto/mod.rs +++ b/tvix/build/src/proto/mod.rs @@ -128,7 +128,7 @@ impl TryFrom<BuildRequest> for crate::buildservice::BuildRequest { for (i, node) in value.inputs.iter().enumerate() { let (name, node) = node .clone() - .into_name_and_node() + .try_into_name_and_node() .map_err(|e| ValidateBuildRequestError::InvalidInputNode(i, e))?; if name.as_ref() <= last_name.as_ref() { 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") + ) +} diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs index 839d2ae85845..fa3326ff937c 100644 --- a/tvix/glue/src/tvix_store_io.rs +++ b/tvix/glue/src/tvix_store_io.rs @@ -292,9 +292,9 @@ impl TvixStoreIO { .zip(build_result.outputs_needles.iter()) .zip(drv.outputs.iter()) { - let (_, output_node) = output + let output_node = output .clone() - .into_name_bytes_and_node() + .try_into_anonymous_node() .expect("invalid node"); let output_needles: Vec<_> = output_needles @@ -365,7 +365,7 @@ impl TvixStoreIO { build_result .outputs .into_iter() - .map(|e| e.into_name_and_node().expect("invalid node")) + .map(|e| e.try_into_name_and_node().expect("invalid node")) .find(|(output_name, _output_node)| { output_name.as_ref() == s.as_bytes() }) diff --git a/tvix/nar-bridge/src/nar.rs b/tvix/nar-bridge/src/nar.rs index d88719b02be9..abc0d854d7c7 100644 --- a/tvix/nar-bridge/src/nar.rs +++ b/tvix/nar-bridge/src/nar.rs @@ -56,16 +56,11 @@ pub async fn get_head( StatusCode::NOT_FOUND })?; - let (root_name, root_node) = root_node.into_name_bytes_and_node().map_err(|e| { + let root_node = root_node.try_into_anonymous_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); - } - Ok(( // headers [ diff --git a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs index d003d4bdb728..4325e79d5077 100644 --- a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs +++ b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs @@ -77,9 +77,9 @@ where &self, request: Request<castorepb::Node>, ) -> Result<Response<proto::CalculateNarResponse>> { - let (_, root_node) = request + let root_node = request .into_inner() - .into_name_bytes_and_node() + .try_into_anonymous_node() .map_err(|e| { warn!(err = %e, "invalid root node"); Status::invalid_argument("invalid root node") diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs index 807f03854ddc..d9edb81f655d 100644 --- a/tvix/store/src/proto/mod.rs +++ b/tvix/store/src/proto/mod.rs @@ -274,7 +274,7 @@ impl TryFrom<PathInfo> for crate::pathinfoservice::PathInfo { let (name, node) = value .node .ok_or_else(|| ValidatePathInfoError::NoNodePresent)? - .into_name_and_node() + .try_into_name_and_node() .map_err(ValidatePathInfoError::InvalidRootNode)?; Ok(Self { |