diff options
Diffstat (limited to 'tvix')
| -rw-r--r-- | tvix/nix-compat/src/derivation/output.rs | 2 | ||||
| -rw-r--r-- | tvix/nix-compat/src/derivation/tests/mod.rs | 5 | ||||
| -rw-r--r-- | tvix/nix-compat/src/derivation/validate.rs | 12 | ||||
| -rw-r--r-- | tvix/nix-compat/src/store_path/mod.rs | 118 | ||||
| -rw-r--r-- | tvix/nix-compat/src/store_path/utils.rs | 2 | ||||
| -rw-r--r-- | tvix/store/src/fuse/mod.rs | 3 | ||||
| -rw-r--r-- | tvix/store/src/proto/mod.rs | 3 | ||||
| -rw-r--r-- | tvix/store/src/proto/tests/pathinfo.rs | 7 |
8 files changed, 84 insertions, 68 deletions
diff --git a/tvix/nix-compat/src/derivation/output.rs b/tvix/nix-compat/src/derivation/output.rs index 4bfc7bf8014d..37161cbe98b5 100644 --- a/tvix/nix-compat/src/derivation/output.rs +++ b/tvix/nix-compat/src/derivation/output.rs @@ -27,7 +27,7 @@ impl Output { } } if validate_output_paths { - if let Err(e) = StorePath::from_absolute_path(&self.path) { + if let Err(e) = StorePath::from_absolute_path(self.path.as_bytes()) { return Err(OutputError::InvalidOutputPath(self.path.to_string(), e)); } } diff --git a/tvix/nix-compat/src/derivation/tests/mod.rs b/tvix/nix-compat/src/derivation/tests/mod.rs index 5daa16da03cb..f4070b2496f4 100644 --- a/tvix/nix-compat/src/derivation/tests/mod.rs +++ b/tvix/nix-compat/src/derivation/tests/mod.rs @@ -6,6 +6,7 @@ use std::collections::BTreeSet; use std::fs::File; use std::io::Read; use std::path::Path; +use std::str::FromStr; use test_case::test_case; use test_generator::test_resources; @@ -67,7 +68,7 @@ fn derivation_path(name: &str, expected_path: &str) { assert_eq!( derivation.calculate_derivation_path(name).unwrap(), - StorePath::from_string(expected_path).unwrap() + StorePath::from_str(expected_path).unwrap() ); } @@ -307,7 +308,7 @@ fn output_path_construction() { assert_eq!(foo_drv_expected, foo_drv); assert_eq!( - StorePath::from_string("4wvvbi4jwn0prsdxb7vs673qa5h9gr7x-foo.drv").expect("must succeed"), + StorePath::from_str("4wvvbi4jwn0prsdxb7vs673qa5h9gr7x-foo.drv").expect("must succeed"), foo_drv .calculate_derivation_path("foo") .expect("must succeed") diff --git a/tvix/nix-compat/src/derivation/validate.rs b/tvix/nix-compat/src/derivation/validate.rs index d8dc24a92ae1..6b81c4c80b86 100644 --- a/tvix/nix-compat/src/derivation/validate.rs +++ b/tvix/nix-compat/src/derivation/validate.rs @@ -1,5 +1,5 @@ use crate::derivation::{Derivation, DerivationError}; -use crate::store_path::StorePath; +use crate::store_path::{self, StorePath}; impl Derivation { /// validate ensures a Derivation struct is properly populated, @@ -26,10 +26,10 @@ impl Derivation { // meaning. // // Other output names that don't match the name restrictions from - // [StorePath] will fail the [StorePath::validate_name] check. + // [StorePath] will fail the [store_path::validate_name] check. if output_name.is_empty() || output_name == "drv" - || StorePath::validate_name(output_name).is_err() + || store_path::validate_name(output_name.as_bytes()).is_err() { return Err(DerivationError::InvalidOutputName(output_name.to_string())); } @@ -55,7 +55,7 @@ impl Derivation { // Validate all input_derivations for (input_derivation_path, output_names) in &self.input_derivations { // Validate input_derivation_path - if let Err(e) = StorePath::from_absolute_path(input_derivation_path) { + if let Err(e) = StorePath::from_absolute_path(input_derivation_path.as_bytes()) { return Err(DerivationError::InvalidInputDerivationPath( input_derivation_path.to_string(), e, @@ -86,7 +86,7 @@ impl Derivation { // [StorePath] will fail the [StorePath::validate_name] check. if output_name.is_empty() || output_name == "drv" - || StorePath::validate_name(output_name).is_err() + || store_path::validate_name(output_name.as_bytes()).is_err() { return Err(DerivationError::InvalidInputDerivationOutputName( input_derivation_path.to_string(), @@ -98,7 +98,7 @@ impl Derivation { // Validate all input_sources for input_source in self.input_sources.iter() { - if let Err(e) = StorePath::from_absolute_path(input_source) { + if let Err(e) = StorePath::from_absolute_path(input_source.as_bytes()) { return Err(DerivationError::InvalidInputSourcesPath( input_source.to_string(), e, diff --git a/tvix/nix-compat/src/store_path/mod.rs b/tvix/nix-compat/src/store_path/mod.rs index f7d42b2f7ef4..74dc5949076b 100644 --- a/tvix/nix-compat/src/store_path/mod.rs +++ b/tvix/nix-compat/src/store_path/mod.rs @@ -1,7 +1,10 @@ use crate::nixbase32::{self, Nixbase32DecodeError}; -use std::{fmt, path::PathBuf}; +use std::{fmt, path::PathBuf, str::FromStr}; use thiserror::Error; +#[cfg(target_family = "unix")] +use std::os::unix::ffi::OsStringExt; + mod utils; pub use utils::*; @@ -25,8 +28,8 @@ pub enum Error { InvalidHashEncoding(Nixbase32DecodeError), #[error("Invalid length")] InvalidLength(), - #[error("Invalid name: {0}")] - InvalidName(String), + #[error("Invalid name: {0:?}")] + InvalidName(Vec<u8>), #[error("Tried to parse an absolute path which was missing the store dir prefix.")] MissingStoreDir(), } @@ -48,10 +51,20 @@ pub struct StorePath { pub name: String, } +impl FromStr for StorePath { + type Err = Error; + + /// Construct a [StorePath] by passing the `$digest-$name` string + /// that comes after [STORE_DIR_WITH_SLASH]. + fn from_str(s: &str) -> Result<Self, Self::Err> { + Self::from_bytes(s.as_bytes()) + } +} + impl StorePath { /// Construct a [StorePath] by passing the `$digest-$name` string /// that comes after [STORE_DIR_WITH_SLASH]. - pub fn from_string(s: &str) -> Result<StorePath, Error> { + pub fn from_bytes(s: &[u8]) -> Result<StorePath, Error> { // the whole string needs to be at least: // // - 32 characters (encoded hash) @@ -61,19 +74,17 @@ impl StorePath { Err(Error::InvalidLength())? } - let digest = match nixbase32::decode(s[..ENCODED_DIGEST_SIZE].as_bytes()) { + let digest = match nixbase32::decode(&s[..ENCODED_DIGEST_SIZE]) { Ok(decoded) => decoded, Err(decoder_error) => return Err(Error::InvalidHashEncoding(decoder_error)), }; - if s.as_bytes()[ENCODED_DIGEST_SIZE] != b'-' { + if s[ENCODED_DIGEST_SIZE] != b'-' { return Err(Error::MissingDash()); } - StorePath::validate_name(&s[ENCODED_DIGEST_SIZE + 2..])?; - Ok(StorePath { - name: s[ENCODED_DIGEST_SIZE + 1..].to_string(), + name: validate_name(&s[ENCODED_DIGEST_SIZE + 1..])?, digest: digest.try_into().expect("size is known"), }) } @@ -81,15 +92,16 @@ impl StorePath { /// Construct a [StorePath] from an absolute store path string. /// This is equivalent to calling [StorePath::from_string], but stripping /// the [STORE_DIR_WITH_SLASH] prefix before. - pub fn from_absolute_path(s: &str) -> Result<StorePath, Error> { - match s.strip_prefix(STORE_DIR_WITH_SLASH) { - Some(s_stripped) => Self::from_string(s_stripped), + pub fn from_absolute_path(s: &[u8]) -> Result<StorePath, Error> { + match s.strip_prefix(STORE_DIR_WITH_SLASH.as_bytes()) { + Some(s_stripped) => Self::from_bytes(s_stripped), None => Err(Error::MissingStoreDir()), } } /// Decompose a string into a [StorePath] and a [PathBuf] containing the /// rest of the path, or an error. + #[cfg(target_family = "unix")] pub fn from_absolute_path_full(s: &str) -> Result<(StorePath, PathBuf), Error> { // strip [STORE_DIR_WITH_SLASH] from s match s.strip_prefix(STORE_DIR_WITH_SLASH) { @@ -102,17 +114,15 @@ impl StorePath { let mut it = p.components(); // The first component of the rest must be parse-able as a [StorePath] - if let Some(s) = it.next() { - // convert first component to string - if let Some(s) = s.as_os_str().to_str() { - let store_path = StorePath::from_string(s)?; - let rest_buf: PathBuf = it.collect(); - Ok((store_path, rest_buf)) - } else { - Err(Error::InvalidName("".to_string())) - } + if let Some(first_component) = it.next() { + // convert first component to StorePath + let first_component_bytes = first_component.as_os_str().to_owned().into_vec(); + let store_path = StorePath::from_bytes(&first_component_bytes)?; + // collect rest + let rest_buf: PathBuf = it.collect(); + Ok((store_path, rest_buf)) } else { - Err(Error::InvalidName("".to_string())) + Err(Error::InvalidLength()) // Well, or missing "/"? } } } @@ -124,26 +134,27 @@ impl StorePath { pub fn to_absolute_path(&self) -> String { format!("{}{}", STORE_DIR_WITH_SLASH, self) } +} - /// Checks a given &str to match the restrictions for store path names. - pub fn validate_name(s: &str) -> Result<(), Error> { - for c in s.chars() { - if c.is_ascii_alphanumeric() - || c == '-' - || c == '_' - || c == '.' - || c == '+' - || c == '?' - || c == '=' - { - continue; - } - - return Err(Error::InvalidName(s.to_string())); +/// Checks a given &[u8] to match the restrictions for store path names, and +/// returns the name as string if successful. +pub(crate) fn validate_name(s: &[u8]) -> Result<String, Error> { + for c in s { + if c.is_ascii_alphanumeric() + || *c == b'-' + || *c == b'_' + || *c == b'.' + || *c == b'+' + || *c == b'?' + || *c == b'=' + { + continue; } - Ok(()) + return Err(Error::InvalidName(s.to_vec())); } + + Ok(String::from_utf8(s.to_vec()).unwrap()) } impl fmt::Display for StorePath { @@ -174,8 +185,8 @@ mod tests { fn happy_path() { let example_nix_path_str = "00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432"; - let nixpath = - StorePath::from_string(example_nix_path_str).expect("Error parsing example string"); + let nixpath = StorePath::from_bytes(example_nix_path_str.as_bytes()) + .expect("Error parsing example string"); let expected_digest: [u8; DIGEST_SIZE] = [ 0x8a, 0x12, 0x32, 0x15, 0x22, 0xfd, 0x91, 0xef, 0xbd, 0x60, 0xeb, 0xb2, 0x48, 0x1a, @@ -190,27 +201,27 @@ mod tests { #[test] fn invalid_hash_length() { - StorePath::from_string("00bgd045z0d4icpbc2yy-net-tools-1.60_p20170221182432") + StorePath::from_bytes(b"00bgd045z0d4icpbc2yy-net-tools-1.60_p20170221182432") .expect_err("must fail"); } #[test] fn invalid_encoding_hash() { - StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48aku4la-net-tools-1.60_p20170221182432") + StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48aku4la-net-tools-1.60_p20170221182432") .expect_err("must fail"); } #[test] fn more_than_just_the_bare_nix_store_path() { - StorePath::from_string( - "00bgd045z0d4icpbc2yyz4gx48aku4la-net-tools-1.60_p20170221182432/bin/arp", + StorePath::from_bytes( + b"00bgd045z0d4icpbc2yyz4gx48aku4la-net-tools-1.60_p20170221182432/bin/arp", ) .expect_err("must fail"); } #[test] fn no_dash_between_hash_and_name() { - StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48ak44lanet-tools-1.60_p20170221182432") + StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48ak44lanet-tools-1.60_p20170221182432") .expect_err("must fail"); } @@ -218,10 +229,11 @@ mod tests { fn absolute_path() { let example_nix_path_str = "00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432"; - let nixpath_expected = StorePath::from_string(example_nix_path_str).expect("must parse"); + let nixpath_expected = + StorePath::from_bytes(example_nix_path_str.as_bytes()).expect("must parse"); let nixpath_actual = StorePath::from_absolute_path( - "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432", + "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432".as_bytes(), ) .expect("must parse"); @@ -237,25 +249,25 @@ mod tests { fn absolute_path_missing_prefix() { assert_eq!( Error::MissingStoreDir(), - StorePath::from_absolute_path("foobar-123").expect_err("must fail") + StorePath::from_absolute_path(b"foobar-123").expect_err("must fail") ); } #[test_case( "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432", - (StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::new()) + (StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::new()) ; "without prefix")] #[test_case( "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432/", - (StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::new()) + (StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::new()) ; "without prefix, but trailing slash")] #[test_case( "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432/bin/arp", - (StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::from("bin/arp")) + (StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::from("bin/arp")) ; "with prefix")] #[test_case( "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432/bin/arp/", - (StorePath::from_string("00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::from("bin/arp/")) + (StorePath::from_bytes(b"00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432").unwrap(), PathBuf::from("bin/arp/")) ; "with prefix and trailing slash")] fn from_absolute_path_full(s: &str, expected: (StorePath, PathBuf)) { let actual = StorePath::from_absolute_path_full(s).expect("must succeed"); @@ -265,7 +277,7 @@ mod tests { #[test] fn from_absolute_path_errors() { assert_eq!( - Error::InvalidName("".into()), + Error::InvalidLength(), StorePath::from_absolute_path_full("/nix/store/").expect_err("must fail") ); assert_eq!( diff --git a/tvix/nix-compat/src/store_path/utils.rs b/tvix/nix-compat/src/store_path/utils.rs index 2d0134cc6863..16a1bf3e1fe5 100644 --- a/tvix/nix-compat/src/store_path/utils.rs +++ b/tvix/nix-compat/src/store_path/utils.rs @@ -145,7 +145,7 @@ fn build_store_path_from_fingerprint_parts( hasher.finalize() }; let compressed = compress_hash::<20>(&digest); - StorePath::validate_name(name)?; + super::validate_name(name.as_bytes())?; Ok(StorePath { digest: compressed, name: name.to_string(), diff --git a/tvix/store/src/fuse/mod.rs b/tvix/store/src/fuse/mod.rs index a357890f59e3..8aa49c499774 100644 --- a/tvix/store/src/fuse/mod.rs +++ b/tvix/store/src/fuse/mod.rs @@ -19,6 +19,7 @@ use crate::{ use fuser::{FileAttr, ReplyAttr, Request}; use nix_compat::store_path::StorePath; use std::io::Read; +use std::str::FromStr; use std::sync::Arc; use std::{collections::HashMap, time::Duration}; use tracing::{debug, info_span, warn}; @@ -99,7 +100,7 @@ impl FUSE { ) -> Result<Option<(u64, Arc<InodeData>)>, Error> { // parse the name into a [StorePath]. let store_path = if let Some(name) = name.to_str() { - match StorePath::from_string(name) { + match StorePath::from_str(name) { Ok(store_path) => store_path, Err(e) => { debug!(e=?e, "unable to parse as store path"); diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs index 4db0b9731edc..7e69726632ce 100644 --- a/tvix/store/src/proto/mod.rs +++ b/tvix/store/src/proto/mod.rs @@ -1,5 +1,6 @@ #![allow(clippy::derive_partial_eq_without_eq)] // https://github.com/hyperium/tonic/issues/1056 +use std::str::FromStr; use std::{collections::HashSet, iter::Peekable}; use thiserror::Error; @@ -97,7 +98,7 @@ fn parse_node_name_root<E>( name: &str, err: fn(String, store_path::Error) -> E, ) -> Result<StorePath, E> { - match StorePath::from_string(name) { + match StorePath::from_str(name) { Ok(np) => Ok(np), Err(e) => Err(err(name.to_string(), e)), } diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs index 57104a5fda19..bccec539f9f3 100644 --- a/tvix/store/src/proto/tests/pathinfo.rs +++ b/tvix/store/src/proto/tests/pathinfo.rs @@ -1,6 +1,7 @@ use crate::proto::{self, Node, PathInfo, ValidatePathInfoError}; use lazy_static::lazy_static; use nix_compat::store_path::{self, StorePath}; +use std::str::FromStr; use test_case::test_case; lazy_static! { @@ -46,7 +47,7 @@ fn validate_no_node( digest: DUMMY_DIGEST.to_vec(), size: 0, }, - Ok(StorePath::from_string(DUMMY_NAME).expect("must succeed")); + Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed")); "ok" )] #[test_case( @@ -91,7 +92,7 @@ fn validate_directory( size: 0, executable: false, }, - Ok(StorePath::from_string(DUMMY_NAME).expect("must succeed")); + Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed")); "ok" )] #[test_case( @@ -131,7 +132,7 @@ fn validate_file(t_file_node: proto::FileNode, t_result: Result<StorePath, Valid name: DUMMY_NAME.to_string(), ..Default::default() }, - Ok(StorePath::from_string(DUMMY_NAME).expect("must succeed")); + Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed")); "ok" )] #[test_case( |