Re-wrote AssetPath::resolve to conform more closely to IETF RFC 1808.

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 "<name>/.." 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")
+        );
     }
 }