diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index bd210d3218c26..9a2182d37aadc 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -926,6 +926,35 @@ mod tests { storage.0.extend(reader.read().cloned()); } + #[test] + fn load_relative_path() { + let dir = Dir::default(); + let d_path = "a/b/c/d.cool.ron"; + let d_ron = r#" +( + text: "hello", + dependencies: [], + embedded_dependencies: [], + sub_texts: [], +)"#; + dir.insert_asset_text(Path::new(d_path), d_ron); + + let (mut app, gate_opener) = test_app(dir); + gate_opener.open(d_path); + app.init_asset::() + .register_asset_loader(CoolTextLoader); + let asset_server = app.world().resource::().clone(); + let handle: Handle = asset_server.load("a/b/c/../c/d.cool.ron"); + let d_id = handle.id(); + app.update(); + + let handle2: Handle = asset_server.load("a/b/../b/c/d.cool.ron"); + let handle3: Handle = asset_server.load("a/b/c/./d.cool.ron"); + + assert_eq!(handle2.id(), d_id); + assert_eq!(handle3.id(), d_id); + } + #[test] fn load_dependencies() { let dir = Dir::default(); diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index de9dfa5ca583a..319720d1d47a0 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -129,7 +129,7 @@ impl<'a> AssetPath<'a> { Some(source) => AssetSourceId::Name(CowArc::Borrowed(source)), None => AssetSourceId::Default, }, - path: CowArc::Borrowed(path), + path: normalize_atomicow_path(CowArc::Borrowed(path)), label: label.map(CowArc::Borrowed), }) } @@ -227,7 +227,7 @@ impl<'a> AssetPath<'a> { #[inline] pub fn from_path_buf(path_buf: PathBuf) -> AssetPath<'a> { AssetPath { - path: CowArc::Owned(path_buf.into()), + path: normalize_atomicow_path(CowArc::Owned(path_buf.into())), source: AssetSourceId::Default, label: None, } @@ -237,7 +237,7 @@ impl<'a> AssetPath<'a> { #[inline] pub fn from_path(path: &'a Path) -> AssetPath<'a> { AssetPath { - path: CowArc::Borrowed(path), + path: normalize_atomicow_path(CowArc::Borrowed(path)), source: AssetSourceId::Default, label: None, } @@ -339,6 +339,18 @@ impl<'a> AssetPath<'a> { } } + /// Normalizes the path component of the `AssetPath` by collapsing all + /// occurrences of '.' and '..' dot-segments where possible as per [RFC + /// 1808](https://datatracker.ietf.org/doc/html/rfc1808). + /// If the path is already normalized, this will return `self` unchanged. + pub fn normalized(self) -> AssetPath<'a> { + AssetPath { + source: self.source, + path: normalize_atomicow_path(self.path), + label: self.label, + } + } + /// Clones this into an "owned" value. If internally a value is borrowed, it will be cloned into an "owned [`Arc`]". /// If internally a value is a static reference, the static reference will be used unchanged. /// If internally a value is an "owned [`Arc`]", the [`Arc`] will be cloned. @@ -448,14 +460,20 @@ impl<'a> AssetPath<'a> { PathBuf::new() }; result_path.push(rpath); - result_path = normalize_path(result_path.as_path()); + + // Boxing the result_path into a CowArc after normalization to + // avoid a potential unnecessary allocation. + let path: CowArc = maybe_normalize_path(&result_path).map_or_else( + || CowArc::Owned(result_path.into()), + |path| CowArc::Owned(path.into()), + ); Ok(AssetPath { source: match source { Some(source) => AssetSourceId::Name(CowArc::Owned(source.into())), None => self.source.clone_owned(), }, - path: CowArc::Owned(result_path.into()), + path, label: rlabel.map(|l| CowArc::Owned(l.into())), }) } @@ -544,7 +562,7 @@ impl From<&'static str> for AssetPath<'static> { let (source, path, label) = Self::parse_internal(asset_path).unwrap(); AssetPath { source: source.into(), - path: CowArc::Static(path), + path: normalize_atomicow_path(CowArc::Static(path)), label: label.map(CowArc::Static), } } @@ -569,7 +587,7 @@ impl From<&'static Path> for AssetPath<'static> { fn from(path: &'static Path) -> Self { Self { source: AssetSourceId::Default, - path: CowArc::Static(path), + path: normalize_atomicow_path(CowArc::Static(path)), label: None, } } @@ -578,9 +596,10 @@ impl From<&'static Path> for AssetPath<'static> { impl From for AssetPath<'static> { #[inline] fn from(path: PathBuf) -> Self { + let path: CowArc<'_, Path> = path.into(); Self { source: AssetSourceId::Default, - path: path.into(), + path: normalize_atomicow_path(path), label: None, } } @@ -642,29 +661,86 @@ impl<'de> Visitor<'de> for AssetPathVisitor { /// Normalizes the path by collapsing all occurrences of '.' and '..' dot-segments where possible /// as per [RFC 1808](https://datatracker.ietf.org/doc/html/rfc1808) -pub(crate) fn normalize_path(path: &Path) -> PathBuf { - let mut result_path = PathBuf::new(); - for elt in path.iter() { +#[cfg_attr( + not(feature = "file_watcher"), + expect(dead_code, reason = "used in file_watcher feature") +)] +pub(crate) fn normalize_path(path: &Path) -> alloc::borrow::Cow<'_, Path> { + match maybe_normalize_path(path) { + Some(pathbuf) => alloc::borrow::Cow::Owned(pathbuf), + None => alloc::borrow::Cow::Borrowed(path), + } +} + +/// Normalizes the path by collapsing all occurrences of '.' and '..' dot-segments where possible +/// as per [RFC 1808](https://datatracker.ietf.org/doc/html/rfc1808) +/// Returns `None` if no normalization was performed, otherwise returns a normalized `PathBuf`. +pub(crate) fn maybe_normalize_path<'a, P: AsRef + 'a>(as_path: P) -> Option { + let path = as_path.as_ref(); + let mut result_path: core::cell::OnceCell = core::cell::OnceCell::new(); + let init = |i: usize| -> PathBuf { path.iter().take(i).collect() }; + + for (i, elt) in path.iter().enumerate() { if elt == "." { - // Skip + result_path.get_or_init(|| init(i)); } else if elt == ".." { - if !result_path.pop() { + result_path.get_or_init(|| init(i)); + + if let Some(path) = result_path.get_mut() + && !path.pop() + { // Preserve ".." if insufficient matches (per RFC 1808). - result_path.push(elt); + path.push(elt); } - } else { - result_path.push(elt); + } else if let Some(path) = result_path.get_mut() { + path.push(elt); } } - result_path + + result_path.into_inner() +} + +pub(crate) fn normalize_atomicow_path<'a>(path: CowArc<'a, Path>) -> CowArc<'a, Path> { + match maybe_normalize_path(&path) { + Some(normalized) => CowArc::Owned(normalized.into()), + None => path, + } } #[cfg(test)] mod tests { - use crate::AssetPath; + use crate::{normalize_atomicow_path, AssetPath}; use alloc::string::ToString; + use atomicow::CowArc; use std::path::Path; + #[test] + fn normalize_cow_paths() { + let path: CowArc = Path::new("a/../a/b").into(); + + assert_eq!( + normalize_atomicow_path(path), + CowArc::::Owned(Path::new("a/b").into()) + ); + + let path: CowArc = Path::new("a/b").into(); + assert_eq!( + normalize_atomicow_path(path), + CowArc::Static(Path::new("a/b")) + ); + + let path = "a/b"; + let donor = 3; + fn steal_lifetime<'a, 'b: 'a, T, U: ?Sized>(_: &'a T, u: &'b U) -> &'a U { + u + } + let path = CowArc::::Borrowed(steal_lifetime(&donor, Path::new(path))); + assert_eq!( + normalize_atomicow_path(path), + CowArc::Borrowed(Path::new("a/b")) + ); + } + #[test] fn parse_asset_path() { let result = AssetPath::parse_internal("a/b.test");