diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 5ff1e4fc8f278e..4f6e4589fa53b7 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -155,27 +155,28 @@ impl<'a> AssetPath<'a> { /// Resolves a possibly-relative path string relative to a base path, producing an `AssetPath`. /// - /// If the path argument begins with `#`, then it is considered an asset label, in which case + /// If the `path` argument begins with `#`, then it is considered an asset label, in which case /// the result is the base path with the label portion replaced. /// - /// If the path argument starts with "./" or "../", then the result the concatenation of - /// the base path and the path argument, which is then canonicalized. The rules are the same as - /// for URIs, which means that the path argument is resolved relative to the *directory* of the - /// base path, so a base path of `"x/y/z#foo"` combined with a relative path of `"./a#bar"` - /// yields `"x/y/a#bar"`. + /// If the path argument begins with '/', then it is considered a 'full' path, in which + /// case the result is simply the `path` argument converted to an `AssetPath`. Note that a 'full' + /// asset path is still relative to the asset root directory, and not necessarily an absolute + /// filesystem path. /// - /// If neither of the above are true, then the path argument is considered a 'full' path, - /// and the result is simply a canonicalized version of the argument string, converted - /// into an `AssetPath`. Note that a 'full' asset path is still relative to the asset root - /// directory, and not necessarily an absolute filesystem path. + /// Otherwise, the `path` argument is considered a relative path. The result is concatenated + /// using the following algorithm (as specified in IETF RFC 1808): /// - /// Note that "./" and "../" elements are only canonicalized in the path argument, any such - /// elements in the base path are left as-is. + /// * Any characters after the final '/' in the base path are removed. + /// * The base path and the `path` argument are concatenated. + /// * Path elements consisting of "/." or "/.." are removed. /// - /// # Panics + /// In other words, the relative path is resolved relative to the *directory* of the base + /// path, not the file. So a base path of `"x/y/z#foo"` combined with a relative path of + /// `"./a#bar"` yields `"x/y/a#bar"`. /// - /// This method will panic if the path argument has more 'parent' elements ("../") than - /// are available in the base path. + /// Note that "./" and "../" elements are only canonicalized in the path argument, any such + /// elements in the base path are left as-is. Also, if there are insufficient segments in the + /// base path to match the ".." segments, then any left-over ".." segments are left as-is. pub fn resolve<'b>(&'a self, path: &'b str) -> AssetPath<'a> { if let Some(label) = path.strip_prefix('#') { // It's a label only @@ -186,27 +187,21 @@ impl<'a> AssetPath<'a> { None => (path, None), }; let mut fpath = PathBuf::from(self.path()); - if !fpath.pop() { - panic!("Can't compute relative path - not enough path elements"); - } + // No error if base is empty (per RFC 1808). + fpath.pop(); let rpath = PathBuf::from(rpath); - let mut first = true; for elt in rpath.iter() { if elt == "." { // Skip } else if elt == ".." { if !fpath.pop() { - panic!("Can't compute relative path - not enough path elements"); + // Preserve ".." if insufficient matches (per RFC 1808). + fpath.push(elt); } } else { - if first { - // If the first path element is not '.' or '..' then start fresh. - fpath.clear(); - } fpath.push(elt); } - first = false; } match rlabel { @@ -474,10 +469,21 @@ mod tests { fn test_resolve_full() { // A "full" path should ignore the base path. let base = AssetPath::from("alice/bob#carol"); - assert_eq!(base.resolve("joe/next"), AssetPath::from("joe/next")); + assert_eq!(base.resolve("/joe/next"), AssetPath::from("/joe/next")); + assert_eq!( + base.resolve("/joe/next#dave"), + AssetPath::from("/joe/next#dave") + ); + } + + #[test] + fn test_resolve_implicit_relative() { + // A "full" path should ignore the base path. + let base = AssetPath::from("alice/bob#carol"); + assert_eq!(base.resolve("joe/next"), AssetPath::from("alice/joe/next")); assert_eq!( base.resolve("joe/next#dave"), - AssetPath::from("joe/next#dave") + AssetPath::from("alice/joe/next#dave") ); } @@ -506,8 +512,21 @@ mod tests { AssetPath::from("martin#dave") ); assert_eq!( - base.resolve("martin/stephan/..#dave"), - AssetPath::from("martin#dave") + base.resolve("/martin/stephan/..#dave"), + AssetPath::from("/martin#dave") + ); + } + + #[test] + fn test_resolve_absolute() { + let base = AssetPath::from("alice/bob#carol"); + assert_eq!( + base.resolve("/martin/stephan"), + AssetPath::from("/martin/stephan") + ); + assert_eq!( + base.resolve("/martin/stephan#dave"), + AssetPath::from("/martin/stephan/#dave") ); } @@ -519,9 +538,11 @@ mod tests { } #[test] - #[should_panic] fn test_resolve_insufficient_elements() { let base = AssetPath::from("alice/bob#carol"); - base.resolve("../../joe/next"); + assert_eq!( + base.resolve("../../joe/next"), + AssetPath::from("../joe/next") + ); } }