mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(tvix/nix-compat): have StorePath accept bytes

Browse source 
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<u8>, 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 <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This commit is contained in:
Florian Klink 2023-07-18 21:34:52 +03:00 committed by clbot
parent 5364fcb127
commit 42dc18353d
8 changed files with 84 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
tvix/nix-compat/src/derivation/output.rs
View file

@ -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));
}
}

5
tvix/nix-compat/src/derivation/tests/mod.rs
View file

@ -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")

12
tvix/nix-compat/src/derivation/validate.rs
View file

@ -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,

118
tvix/nix-compat/src/store_path/mod.rs
View file

@ -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!(

2
tvix/nix-compat/src/store_path/utils.rs
View file

@ -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(),

3
tvix/store/src/fuse/mod.rs
View file

@ -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");

3
tvix/store/src/proto/mod.rs
View file

@ -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)),
}

7
tvix/store/src/proto/tests/pathinfo.rs
View file

@ -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(