diff --git a/Cargo.lock b/Cargo.lock index 74337864f53..f95b890b2a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2385,6 +2385,7 @@ dependencies = [ "bstr", "gix-testtools", "gix-trace 0.1.12", + "gix-validate 0.9.4", "home", "known-folders", "once_cell", diff --git a/gix-fs/src/stack.rs b/gix-fs/src/stack.rs index 2c4d1c0e3a1..2486543d441 100644 --- a/gix-fs/src/stack.rs +++ b/gix-fs/src/stack.rs @@ -50,30 +50,43 @@ fn component_to_os_str<'a>( impl ToNormalPathComponents for &BStr { fn to_normal_path_components(&self) -> impl Iterator> { - self.split(|b| *b == b'/').filter_map(bytes_component_to_os_str) + self.split(|b| *b == b'/') + .filter_map(|c| bytes_component_to_os_str(c, self)) } } impl ToNormalPathComponents for &str { fn to_normal_path_components(&self) -> impl Iterator> { - self.split('/').filter_map(|c| bytes_component_to_os_str(c.as_bytes())) + self.split('/') + .filter_map(|c| bytes_component_to_os_str(c.as_bytes(), (*self).into())) } } impl ToNormalPathComponents for &BString { fn to_normal_path_components(&self) -> impl Iterator> { - self.split(|b| *b == b'/').filter_map(bytes_component_to_os_str) + self.split(|b| *b == b'/') + .filter_map(|c| bytes_component_to_os_str(c, self.as_bstr())) } } -fn bytes_component_to_os_str(component: &[u8]) -> Option> { +fn bytes_component_to_os_str<'a>( + component: &'a [u8], + path: &BStr, +) -> Option> { if component.is_empty() { return None; } - gix_path::try_from_byte_slice(component.as_bstr()) + let component = match gix_path::try_from_byte_slice(component.as_bstr()) .map_err(|_| to_normal_path_components::Error::IllegalUtf8) - .map(Path::as_os_str) - .into() + { + Ok(c) => c, + Err(err) => return Some(Err(err)), + }; + let component = component.components().next()?; + Some(component_to_os_str( + component, + gix_path::try_from_byte_slice(path.as_ref()).ok()?, + )) } /// Access diff --git a/gix-fs/tests/fs/stack.rs b/gix-fs/tests/fs/stack.rs index f3ba1d70751..0c5519b19fa 100644 --- a/gix-fs/tests/fs/stack.rs +++ b/gix-fs/tests/fs/stack.rs @@ -246,6 +246,19 @@ fn absolute_paths_are_invalid() -> crate::Result { r#"Input path "/" contains relative or absolute components"#, "a leading slash is always considered absolute" ); + s.make_relative_path_current("/", &mut r)?; + assert_eq!( + s.current(), + s.root(), + "as string this is a no-op as it's just split by /" + ); + + let err = s.make_relative_path_current("../breakout", &mut r).unwrap_err(); + assert_eq!( + err.to_string(), + r#"Input path "../breakout" contains relative or absolute components"#, + "otherwise breakout attempts are detected" + ); s.make_relative_path_current(p("a/"), &mut r)?; assert_eq!( s.current(), diff --git a/gix-negotiate/tests/baseline/mod.rs b/gix-negotiate/tests/baseline/mod.rs index 76c18b74cea..4c4df125516 100644 --- a/gix-negotiate/tests/baseline/mod.rs +++ b/gix-negotiate/tests/baseline/mod.rs @@ -71,7 +71,7 @@ fn run() -> crate::Result { // } for tip in lookup_names(&["HEAD"]).into_iter().chain( refs.iter()? - .prefixed(b"refs/heads")? + .prefixed(b"refs/heads".try_into().unwrap())? .filter_map(Result::ok) .map(|r| r.target.into_id()), ) { diff --git a/gix-path/Cargo.toml b/gix-path/Cargo.toml index ee4be88d33f..805e9720cf4 100644 --- a/gix-path/Cargo.toml +++ b/gix-path/Cargo.toml @@ -16,6 +16,7 @@ doctest = false [dependencies] gix-trace = { version = "^0.1.12", path = "../gix-trace" } +gix-validate = { version = "^0.9.4", path = "../gix-validate" } bstr = { version = "1.12.0", default-features = false, features = ["std"] } thiserror = "2.0.0" once_cell = "1.21.3" diff --git a/gix-path/src/lib.rs b/gix-path/src/lib.rs index a83b8bde7a8..5157e78fec1 100644 --- a/gix-path/src/lib.rs +++ b/gix-path/src/lib.rs @@ -47,7 +47,7 @@ //! ever get into a code-path which does panic though. //! #![deny(missing_docs, rust_2018_idioms)] -#![cfg_attr(not(test), forbid(unsafe_code))] +#![cfg_attr(not(test), deny(unsafe_code))] /// A dummy type to represent path specs and help finding all spots that take path specs once it is implemented. mod convert; @@ -62,3 +62,7 @@ pub use realpath::function::{realpath, realpath_opts}; /// Information about the environment in terms of locations of resources. pub mod env; + +/// +pub mod relative_path; +pub use relative_path::types::RelativePath; diff --git a/gix-path/src/relative_path.rs b/gix-path/src/relative_path.rs new file mode 100644 index 00000000000..2ee5ec222e3 --- /dev/null +++ b/gix-path/src/relative_path.rs @@ -0,0 +1,113 @@ +use bstr::BStr; +use bstr::BString; +use bstr::ByteSlice; +use gix_validate::path::component::Options; +use std::path::Path; + +use crate::os_str_into_bstr; +use crate::try_from_bstr; +use crate::try_from_byte_slice; + +pub(super) mod types { + use bstr::{BStr, ByteSlice}; + /// A wrapper for `BStr`. It is used to enforce the following constraints: + /// + /// - The path separator always is `/`, independent of the platform. + /// - Only normal components are allowed. + /// - It is always represented as a bunch of bytes. + #[derive()] + pub struct RelativePath { + inner: BStr, + } + + impl AsRef<[u8]> for RelativePath { + #[inline] + fn as_ref(&self) -> &[u8] { + self.inner.as_bytes() + } + } +} +use types::RelativePath; + +impl RelativePath { + fn new_unchecked(value: &BStr) -> Result<&RelativePath, Error> { + // SAFETY: `RelativePath` is transparent and equivalent to a `&BStr` if provided as reference. + #[allow(unsafe_code)] + unsafe { + std::mem::transmute(value) + } + } +} + +/// The error used in [`RelativePath`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("A RelativePath is not allowed to be absolute")] + IsAbsolute, + #[error(transparent)] + ContainsInvalidComponent(#[from] gix_validate::path::component::Error), + #[error(transparent)] + IllegalUtf8(#[from] crate::Utf8Error), +} + +fn relative_path_from_value_and_path<'a>(path_bstr: &'a BStr, path: &Path) -> Result<&'a RelativePath, Error> { + if path.is_absolute() { + return Err(Error::IsAbsolute); + } + + let options = Options::default(); + + for component in path.components() { + let component = os_str_into_bstr(component.as_os_str())?; + gix_validate::path::component(component, None, options)?; + } + + RelativePath::new_unchecked(BStr::new(path_bstr.as_bytes())) +} + +impl<'a> TryFrom<&'a str> for &'a RelativePath { + type Error = Error; + + fn try_from(value: &'a str) -> Result { + relative_path_from_value_and_path(value.into(), Path::new(value)) + } +} + +impl<'a> TryFrom<&'a BStr> for &'a RelativePath { + type Error = Error; + + fn try_from(value: &'a BStr) -> Result { + let path = try_from_bstr(value)?; + relative_path_from_value_and_path(value, &path) + } +} + +impl<'a> TryFrom<&'a [u8]> for &'a RelativePath { + type Error = Error; + + #[inline] + fn try_from(value: &'a [u8]) -> Result { + let path = try_from_byte_slice(value)?; + relative_path_from_value_and_path(value.as_bstr(), path) + } +} + +impl<'a, const N: usize> TryFrom<&'a [u8; N]> for &'a RelativePath { + type Error = Error; + + #[inline] + fn try_from(value: &'a [u8; N]) -> Result { + let path = try_from_byte_slice(value.as_bstr())?; + relative_path_from_value_and_path(value.as_bstr(), path) + } +} + +impl<'a> TryFrom<&'a BString> for &'a RelativePath { + type Error = Error; + + fn try_from(value: &'a BString) -> Result { + let path = try_from_bstr(value.as_bstr())?; + relative_path_from_value_and_path(value.as_bstr(), &path) + } +} diff --git a/gix-path/tests/path/main.rs b/gix-path/tests/path/main.rs index dae33bb7746..4b2f26262b4 100644 --- a/gix-path/tests/path/main.rs +++ b/gix-path/tests/path/main.rs @@ -2,6 +2,7 @@ pub type Result = std::result::Result>; mod convert; mod realpath; +mod relative_path; mod home_dir { #[test] fn returns_existing_directory() { diff --git a/gix-path/tests/path/relative_path.rs b/gix-path/tests/path/relative_path.rs new file mode 100644 index 00000000000..c8d880b3d71 --- /dev/null +++ b/gix-path/tests/path/relative_path.rs @@ -0,0 +1,160 @@ +use bstr::{BStr, BString}; +use gix_path::relative_path::Error; +use gix_path::RelativePath; + +#[cfg(not(windows))] +#[test] +fn absolute_paths_return_err() { + let path_str: &str = "/refs/heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8a: &[u8; 11] = b"/refs/heads"; + let path_u8: &[u8] = &b"/refs/heads"[..]; + let path_bstring: BString = "/refs/heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8a), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::IsAbsolute) + )); +} + +#[cfg(windows)] +#[test] +fn absolute_paths_with_backslashes_return_err() { + let path_str: &str = r"c:\refs\heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8: &[u8] = &b"c:\\refs\\heads"[..]; + let path_bstring: BString = r"c:\refs\heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::IsAbsolute) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::IsAbsolute) + )); +} + +#[test] +fn dots_in_paths_return_err() { + let path_str: &str = "./heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8: &[u8] = &b"./heads"[..]; + let path_bstring: BString = "./heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::ContainsInvalidComponent(_)) + )); +} + +#[test] +fn dots_in_paths_with_backslashes_return_err() { + let path_str: &str = r".\heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8: &[u8] = &b".\\heads"[..]; + let path_bstring: BString = r".\heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::ContainsInvalidComponent(_)) + )); +} + +#[test] +fn double_dots_in_paths_return_err() { + let path_str: &str = "../heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8: &[u8] = &b"../heads"[..]; + let path_bstring: BString = "../heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::ContainsInvalidComponent(_)) + )); +} + +#[test] +fn double_dots_in_paths_with_backslashes_return_err() { + let path_str: &str = r"..\heads"; + let path_bstr: &BStr = path_str.into(); + let path_u8: &[u8] = &b"..\\heads"[..]; + let path_bstring: BString = r"..\heads".into(); + + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_str), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_bstr), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(path_u8), + Err(Error::ContainsInvalidComponent(_)) + )); + assert!(matches!( + TryInto::<&RelativePath>::try_into(&path_bstring), + Err(Error::ContainsInvalidComponent(_)) + )); +} diff --git a/gix-ref/src/namespace.rs b/gix-ref/src/namespace.rs index 0570ff27434..912fe190ddc 100644 --- a/gix-ref/src/namespace.rs +++ b/gix-ref/src/namespace.rs @@ -1,6 +1,7 @@ use std::path::Path; use gix_object::bstr::{BStr, BString, ByteSlice, ByteVec}; +use gix_path::RelativePath; use crate::{FullName, FullNameRef, Namespace, PartialNameRef}; @@ -20,9 +21,8 @@ impl Namespace { /// Append the given `prefix` to this namespace so it becomes usable for prefixed iteration. /// /// The prefix is a relative path with slash-separated path components. - // TODO: use `RelativePath` type instead (see #1921), or a trait that helps convert into it. - pub fn into_namespaced_prefix<'a>(mut self, prefix: impl Into<&'a BStr>) -> BString { - self.0.push_str(prefix.into()); + pub fn into_namespaced_prefix(mut self, prefix: &RelativePath) -> BString { + self.0.push_str(prefix); gix_path::to_unix_separators_on_windows(self.0).into_owned() } pub(crate) fn into_namespaced_name(mut self, name: &FullNameRef) -> FullName { diff --git a/gix-ref/src/store/file/loose/iter.rs b/gix-ref/src/store/file/loose/iter.rs index 2982e4a22e3..31d91a4b094 100644 --- a/gix-ref/src/store/file/loose/iter.rs +++ b/gix-ref/src/store/file/loose/iter.rs @@ -2,8 +2,9 @@ use std::path::{Path, PathBuf}; use gix_features::fs::walkdir::DirEntryIter; use gix_object::bstr::ByteSlice; +use gix_path::RelativePath; -use crate::{file::iter::LooseThenPacked, store_impl::file, BStr, BString, FullName}; +use crate::{file::iter::LooseThenPacked, store_impl::file, BString, FullName}; /// An iterator over all valid loose reference paths as seen from a particular base directory. pub(in crate::store_impl::file) struct SortedLoosePaths { @@ -88,8 +89,7 @@ impl file::Store { /// starts with `foo`, like `refs/heads/foo` and `refs/heads/foobar`. /// /// Prefixes are relative paths with slash-separated components. - // TODO: use `RelativePath` type instead (see #1921), or a trait that helps convert into it. - pub fn loose_iter_prefixed<'a>(&self, prefix: impl Into<&'a BStr>) -> std::io::Result> { - self.iter_prefixed_packed(prefix.into(), None) + pub fn loose_iter_prefixed(&self, prefix: &RelativePath) -> std::io::Result> { + self.iter_prefixed_packed(prefix, None) } } diff --git a/gix-ref/src/store/file/overlay_iter.rs b/gix-ref/src/store/file/overlay_iter.rs index e69c9cfed15..bd5ca365201 100644 --- a/gix-ref/src/store/file/overlay_iter.rs +++ b/gix-ref/src/store/file/overlay_iter.rs @@ -12,6 +12,9 @@ use crate::{ BStr, FullName, Namespace, Reference, }; +use gix_object::bstr::ByteSlice; +use gix_path::RelativePath; + /// An iterator stepping through sorted input of loose references and packed references, preferring loose refs over otherwise /// equivalent packed references. /// @@ -203,10 +206,9 @@ impl Platform<'_> { /// starts with `foo`, like `refs/heads/foo` and `refs/heads/foobar`. /// /// Prefixes are relative paths with slash-separated components. - // TODO: use `RelativePath` type instead (see #1921), or a trait that helps convert into it. - pub fn prefixed<'a>(&self, prefix: impl Into<&'a BStr>) -> std::io::Result> { + pub fn prefixed(&self, prefix: &RelativePath) -> std::io::Result> { self.store - .iter_prefixed_packed(prefix.into(), self.packed.as_ref().map(|b| &***b)) + .iter_prefixed_packed(prefix, self.packed.as_ref().map(|b| &***b)) } } @@ -291,28 +293,10 @@ impl<'a> IterInfo<'a> { .peekable() } - fn from_prefix( - base: &'a Path, - prefix: impl Into>, - precompose_unicode: bool, - ) -> std::io::Result { - let prefix = prefix.into(); - let prefix_path = gix_path::from_bstr(prefix.as_ref()); - if prefix_path.is_absolute() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "prefix must be a relative path, like 'refs/heads/'", - )); - } - use std::path::Component::*; - if prefix_path.components().any(|c| matches!(c, CurDir | ParentDir)) { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "Refusing to handle prefixes with relative path components", - )); - } + fn from_prefix(base: &'a Path, prefix: &'a RelativePath, precompose_unicode: bool) -> std::io::Result { + let prefix_path = gix_path::from_bstr(prefix.as_ref().as_bstr()); let iter_root = base.join(&prefix_path); - if prefix.ends_with(b"/") { + if prefix.as_ref().ends_with(b"/") { Ok(IterInfo::BaseAndIterRoot { base, iter_root, @@ -326,7 +310,7 @@ impl<'a> IterInfo<'a> { .to_owned(); Ok(IterInfo::ComputedIterationRoot { base, - prefix, + prefix: prefix.as_ref().as_bstr().into(), iter_root, precompose_unicode, }) @@ -377,13 +361,11 @@ impl file::Store { /// starts with `foo`, like `refs/heads/foo` and `refs/heads/foobar`. /// /// Prefixes are relative paths with slash-separated components. - // TODO: use `RelativePath` type instead (see #1921), or a trait that helps convert into it. - pub fn iter_prefixed_packed<'a, 's, 'p>( + pub fn iter_prefixed_packed<'s, 'p>( &'s self, - prefix: impl Into<&'a BStr>, + prefix: &RelativePath, packed: Option<&'p packed::Buffer>, ) -> std::io::Result> { - let prefix = prefix.into(); match self.namespace.as_ref() { None => { let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix, self.precompose_unicode)?; @@ -395,7 +377,8 @@ impl file::Store { } Some(namespace) => { let prefix = namespace.to_owned().into_namespaced_prefix(prefix); - let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix.clone(), self.precompose_unicode)?; + let prefix = prefix.as_bstr().try_into().map_err(std::io::Error::other)?; + let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix, self.precompose_unicode)?; let common_dir_info = self .common_dir() .map(|base| IterInfo::from_prefix(base, prefix, self.precompose_unicode)) diff --git a/gix-ref/tests/refs/file/store/iter.rs b/gix-ref/tests/refs/file/store/iter.rs index fb0a6ddb3b3..527fd03c1cb 100644 --- a/gix-ref/tests/refs/file/store/iter.rs +++ b/gix-ref/tests/refs/file/store/iter.rs @@ -26,7 +26,7 @@ mod with_namespace { let ns_two = gix_ref::namespace::expand("bar")?; let namespaced_refs = store .iter()? - .prefixed(ns_two.as_bstr())? + .prefixed(ns_two.as_bstr().try_into().unwrap())? .map(Result::unwrap) .map(|r: gix_ref::Reference| r.name) .collect::>(); @@ -45,7 +45,7 @@ mod with_namespace { ); assert_eq!( store - .loose_iter_prefixed(ns_two.as_bstr())? + .loose_iter_prefixed(ns_two.as_bstr().try_into().unwrap())? .map(Result::unwrap) .map(|r| r.name.into_inner()) .collect::>(), @@ -90,7 +90,7 @@ mod with_namespace { assert_eq!( store .iter()? - .prefixed(ns_one.as_bstr())? + .prefixed(ns_one.as_bstr().try_into().unwrap())? .map(Result::unwrap) .map(|r: gix_ref::Reference| ( r.name.as_bstr().to_owned(), @@ -316,26 +316,11 @@ fn loose_iter_with_broken_refs() -> crate::Result { Ok(()) } -#[test] -fn loose_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result { - let store = store()?; - #[cfg(not(windows))] - let abs_path = "/hello"; - #[cfg(windows)] - let abs_path = r"c:\hello"; - - match store.loose_iter_prefixed(abs_path) { - Ok(_) => unreachable!("absolute paths aren't allowed"), - Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads/'"), - } - Ok(()) -} - #[test] fn loose_iter_with_prefix() -> crate::Result { let prefix_with_slash = b"refs/heads/"; let actual = store()? - .loose_iter_prefixed(prefix_with_slash)? + .loose_iter_prefixed(prefix_with_slash.try_into().unwrap())? .collect::, _>>() .expect("no broken ref in this subset") .into_iter() @@ -363,7 +348,7 @@ fn loose_iter_with_prefix() -> crate::Result { fn loose_iter_with_partial_prefix_dir() -> crate::Result { let prefix_without_slash = b"refs/heads"; let actual = store()? - .loose_iter_prefixed(prefix_without_slash)? + .loose_iter_prefixed(prefix_without_slash.try_into().unwrap())? .collect::, _>>() .expect("no broken ref in this subset") .into_iter() @@ -390,7 +375,7 @@ fn loose_iter_with_partial_prefix_dir() -> crate::Result { #[test] fn loose_iter_with_partial_prefix() -> crate::Result { let actual = store()? - .loose_iter_prefixed(b"refs/heads/d".as_bstr())? + .loose_iter_prefixed(b"refs/heads/d".as_bstr().try_into().unwrap())? .collect::, _>>() .expect("no broken ref in this subset") .into_iter() @@ -526,21 +511,6 @@ fn overlay_iter_reproduce_1928() -> crate::Result { Ok(()) } -#[test] -fn overlay_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result { - let store = store_with_packed_refs()?; - #[cfg(not(windows))] - let abs_path = "/hello"; - #[cfg(windows)] - let abs_path = r"c:\hello"; - - match store.iter()?.prefixed(abs_path) { - Ok(_) => unreachable!("absolute paths aren't allowed"), - Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads/'"), - } - Ok(()) -} - #[test] fn overlay_prefixed_iter() -> crate::Result { use gix_ref::Target::*; @@ -548,7 +518,7 @@ fn overlay_prefixed_iter() -> crate::Result { let store = store_at("make_packed_ref_repository_for_overlay.sh")?; let ref_names = store .iter()? - .prefixed(b"refs/heads/")? + .prefixed(b"refs/heads/".try_into().unwrap())? .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); @@ -571,7 +541,7 @@ fn overlay_partial_prefix_iter() -> crate::Result { let store = store_at("make_packed_ref_repository_for_overlay.sh")?; let ref_names = store .iter()? - .prefixed(b"refs/heads/m")? // 'm' is partial + .prefixed(b"refs/heads/m".try_into().unwrap())? // 'm' is partial .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); @@ -589,7 +559,7 @@ fn overlay_partial_prefix_iter_reproduce_1934() -> crate::Result { let ref_names = store .iter()? - .prefixed(b"refs/d")? + .prefixed(b"refs/d".try_into().unwrap())? .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; assert_eq!( @@ -610,7 +580,7 @@ fn overlay_partial_prefix_iter_when_prefix_is_dir() -> crate::Result { let ref_names = store .iter()? - .prefixed(b"refs/prefix/feature")? + .prefixed(b"refs/prefix/feature".try_into().unwrap())? .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; assert_eq!( @@ -623,7 +593,7 @@ fn overlay_partial_prefix_iter_when_prefix_is_dir() -> crate::Result { let ref_names = store .iter()? - .prefixed(b"refs/prefix/feature/")? + .prefixed(b"refs/prefix/feature/".try_into().unwrap())? .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; assert_eq!( diff --git a/gix-ref/tests/refs/file/worktree.rs b/gix-ref/tests/refs/file/worktree.rs index 6bc4d73f515..de8cc54f777 100644 --- a/gix-ref/tests/refs/file/worktree.rs +++ b/gix-ref/tests/refs/file/worktree.rs @@ -290,7 +290,7 @@ mod writable { assert_eq!( store .iter()? - .prefixed(b"refs/stacks/")? + .prefixed(b"refs/stacks/".try_into().unwrap())? .map(Result::unwrap) .map(|r| (r.name.to_string(), r.target.to_string())) .collect::>(), @@ -571,7 +571,7 @@ mod writable { assert_eq!( store .iter()? - .prefixed(b"refs/stacks/")? + .prefixed(b"refs/stacks/".try_into().unwrap())? .map(Result::unwrap) .map(|r| (r.name.to_string(), r.target.to_string())) .collect::>(), diff --git a/gix-ref/tests/refs/namespace.rs b/gix-ref/tests/refs/namespace.rs index 58cf96e3092..3f0a6e180e0 100644 --- a/gix-ref/tests/refs/namespace.rs +++ b/gix-ref/tests/refs/namespace.rs @@ -3,13 +3,13 @@ fn into_namespaced_prefix() { assert_eq!( gix_ref::namespace::expand("foo") .unwrap() - .into_namespaced_prefix("prefix"), + .into_namespaced_prefix("prefix".try_into().unwrap()), "refs/namespaces/foo/prefix", ); assert_eq!( gix_ref::namespace::expand("foo") .unwrap() - .into_namespaced_prefix("prefix/"), + .into_namespaced_prefix("prefix/".try_into().unwrap()), "refs/namespaces/foo/prefix/", ); } diff --git a/gix/src/open/mod.rs b/gix/src/open/mod.rs index efb95e153cb..ac22c42ae8f 100644 --- a/gix/src/open/mod.rs +++ b/gix/src/open/mod.rs @@ -56,6 +56,8 @@ pub enum Error { UnsafeGitDir { path: PathBuf }, #[error(transparent)] EnvironmentAccessDenied(#[from] gix_sec::permission::Error), + #[error(transparent)] + PrefixNotRelative(#[from] gix_path::relative_path::Error), } mod options; diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index 7cadf579028..bb3e15e26ee 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -1,7 +1,6 @@ #![allow(clippy::result_large_err)] use super::{Error, Options}; use crate::{ - bstr, bstr::BString, config, config::{ @@ -12,6 +11,8 @@ use crate::{ ThreadSafeRepository, }; use gix_features::threading::OwnShared; +use gix_object::bstr::ByteSlice; +use gix_path::RelativePath; use std::ffi::OsStr; use std::{borrow::Cow, path::PathBuf}; @@ -345,24 +346,30 @@ impl ThreadSafeRepository { refs.write_reflog = config::cache::util::reflog_or_default(config.reflog, worktree_dir.is_some()); refs.namespace.clone_from(&config.refs_namespace); - let replacements = replacement_objects_refs_prefix(&config.resolved, lenient_config, filter_config_section)? - .and_then(|prefix| { - use bstr::ByteSlice; - let _span = gix_trace::detail!("find replacement objects"); - let platform = refs.iter().ok()?; - let iter = platform.prefixed(prefix.as_bstr()).ok()?; - let replacements = iter - .filter_map(Result::ok) - .filter_map(|r: gix_ref::Reference| { - let target = r.target.try_id()?.to_owned(); - let source = - gix_hash::ObjectId::from_hex(r.name.as_bstr().strip_prefix(prefix.as_slice())?).ok()?; - Some((source, target)) - }) - .collect::>(); - Some(replacements) - }) - .unwrap_or_default(); + let prefix = replacement_objects_refs_prefix(&config.resolved, lenient_config, filter_config_section)?; + let replacements = match prefix { + Some(prefix) => { + let prefix: &RelativePath = prefix.as_bstr().try_into()?; + + Some(prefix).and_then(|prefix| { + let _span = gix_trace::detail!("find replacement objects"); + let platform = refs.iter().ok()?; + let iter = platform.prefixed(prefix).ok()?; + let replacements = iter + .filter_map(Result::ok) + .filter_map(|r: gix_ref::Reference| { + let target = r.target.try_id()?.to_owned(); + let source = + gix_hash::ObjectId::from_hex(r.name.as_bstr().strip_prefix(prefix.as_ref())?).ok()?; + Some((source, target)) + }) + .collect::>(); + Some(replacements) + }) + } + None => None, + }; + let replacements = replacements.unwrap_or_default(); Ok(ThreadSafeRepository { objects: OwnShared::new(gix_odb::Store::at_opts( diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs index 6c975d95bc3..a18ecbeda91 100644 --- a/gix/src/reference/iter.rs +++ b/gix/src/reference/iter.rs @@ -1,8 +1,8 @@ //! #![allow(clippy::empty_docs)] -use std::borrow::Cow; -use gix_ref::{bstr::BStr, file::ReferenceExt}; +use gix_path::RelativePath; +use gix_ref::file::ReferenceExt; /// A platform to create iterators over references. #[must_use = "Iterators should be obtained from this iterator platform"] @@ -43,10 +43,11 @@ impl Platform<'_> { /// Return an iterator over all references that match the given `prefix`. /// /// These are of the form `refs/heads/` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`. - // TODO: Create a custom `Path` type that enforces the requirements of git naturally, this type is surprising possibly on windows - // and when not using a trailing '/' to signal directories. - pub fn prefixed<'a>(&self, prefix: impl Into>) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed(prefix.into().as_ref())?)) + pub fn prefixed<'a>( + &self, + prefix: impl TryInto<&'a RelativePath, Error = gix_path::relative_path::Error>, + ) -> Result, init::Error> { + Ok(Iter::new(self.repo, self.platform.prefixed(prefix.try_into()?)?)) } // TODO: tests @@ -54,7 +55,7 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/tags`. pub fn tags(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/tags/")?)) + Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/tags/".try_into()?)?)) } // TODO: tests @@ -62,7 +63,10 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/heads`. pub fn local_branches(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/heads/")?)) + Ok(Iter::new( + self.repo, + self.platform.prefixed(b"refs/heads/".try_into()?)?, + )) } // TODO: tests @@ -70,7 +74,10 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/remotes`. pub fn remote_branches(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/remotes/")?)) + Ok(Iter::new( + self.repo, + self.platform.prefixed(b"refs/remotes/".try_into()?)?, + )) } } @@ -123,6 +130,8 @@ pub mod init { pub enum Error { #[error(transparent)] Io(#[from] std::io::Error), + #[error(transparent)] + RelativePath(#[from] gix_path::relative_path::Error), } } diff --git a/gix/tests/gix/repository/reference.rs b/gix/tests/gix/repository/reference.rs index bd5079aabfc..b69ac5445c7 100644 --- a/gix/tests/gix/repository/reference.rs +++ b/gix/tests/gix/repository/reference.rs @@ -1,5 +1,5 @@ mod set_namespace { - use gix::{bstr::ByteSlice, refs::transaction::PreviousValue}; + use gix::refs::transaction::PreviousValue; use gix_testtools::tempfile; fn easy_repo_rw() -> crate::Result<(gix::Repository, tempfile::TempDir)> { @@ -48,7 +48,7 @@ mod set_namespace { assert_eq!( repo.references()? - .prefixed(b"refs/tags/".as_bstr())? + .prefixed("refs/tags/")? .filter_map(Result::ok) .map(|r| r.name().as_bstr().to_owned()) .collect::>(), @@ -81,8 +81,6 @@ mod set_namespace { } mod iter_references { - use gix::bstr::ByteSlice; - use crate::util::hex_to_id; fn repo() -> crate::Result { @@ -125,7 +123,7 @@ mod iter_references { let repo = repo()?; assert_eq!( repo.references()? - .prefixed(b"refs/heads/".as_bstr())? + .prefixed("refs/heads/")? .filter_map(Result::ok) .map(|r| ( r.name().as_bstr().to_string(), @@ -156,7 +154,7 @@ mod iter_references { let repo = repo()?; assert_eq!( repo.references()? - .prefixed(b"refs/heads/".as_bstr())? + .prefixed(b"refs/heads/")? .peeled()? .filter_map(Result::ok) .map(|r| (