From 42dc18353d99453bc0f83492f9f5bc4796f4cc4c Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 18 Jul 2023 21:34:52 +0300 Subject: feat(tvix/nix-compat): have StorePath accept bytes The primary constructor for this is now from_bytes, from_string is simply calling .as_bytes() on the string, passing it along. The InvalidName error now contains a Vec, to encode the invalid name (which might not be a string anymore). from_absolute_path now accepts a &[u8] (even though we might want to make this a OSString of some sort). StorePath::validate_name has been degraded to a pub(crate) function. It's still used in src/derivation, even though it probably shouldn't at all - that cleanup is left for cl/8412 though. Change-Id: I6b4e62a6fa5c4bec13b535279e73444f0b83ad35 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8973 Autosubmit: flokli Tested-by: BuildkiteCI Reviewed-by: raitobezarius --- tvix/nix-compat/src/store_path/mod.rs | 118 ++++++++++++++++++-------------- tvix/nix-compat/src/store_path/utils.rs | 2 +- 2 files changed, 66 insertions(+), 54 deletions(-) (limited to 'tvix/nix-compat/src/store_path') 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), #[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::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 { + pub fn from_bytes(s: &[u8]) -> Result { // 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 { - match s.strip_prefix(STORE_DIR_WITH_SLASH) { - Some(s_stripped) => Self::from_string(s_stripped), + pub fn from_absolute_path(s: &[u8]) -> Result { + 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 { + 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(), -- cgit 1.4.1