AssetPath source parse fix

crates/bevy_asset/src/path.rs

@@ -78,12 +78,19 @@ impl<'a> Display for AssetPath<'a> {
     }
 }
 
+/// An error that occurs when parsing a string type to create an [`AssetPath`] fails, such as during [`AssetPath::parse`] or [`AssetPath::from<'static str>`].
 #[derive(Error, Debug, PartialEq, Eq)]
 pub enum ParseAssetPathError {
-    #[error("Asset source must be followed by '://'")]
+    /// Error that occurs when the [`AssetPath::source`] section of a path string contains the [`AssetPath::label`] delimiter `#`. E.g. `bad#source://file.test`.
+    #[error("Asset source must not contain a `#` character")]
     InvalidSourceSyntax,
+    /// Error that occurs when the [`AssetPath::label`] section of a path string contains the [`AssetPath::source`] delimiter `://`. E.g. `source://file.test#bad://label`.
+    #[error("Asset label must not contain a `://` substring")]
+    InvalidLabelSyntax,
+    /// Error that occurs when a path string has an [`AssetPath::source`] delimiter `://` with no characters preceeding it. E.g. `://file.test`.
     #[error("Asset source must be at least one character. Either specify the source before the '://' or remove the `://`")]
     MissingSource,
+    /// Error that occurs when a path string has an [`AssetPath::label`] delimiter `#` with no characters succeeding it. E.g. `file.test#`
     #[error("Asset label must be at least one character. Either specify the label after the '#' or remove the '#'")]
     MissingLabel,
 }
@@ -126,40 +133,70 @@ impl<'a> AssetPath<'a> {
         })
     }
 
+    // Attempts to Parse a &str into an `AssetPath`'s `AssetPath::source`, `AssetPath::path`, and `AssetPath::label` components.
     fn parse_internal(
         asset_path: &str,
     ) -> Result<(Option<&str>, &Path, Option<&str>), ParseAssetPathError> {
-        let mut chars = asset_path.char_indices();
+        let chars = asset_path.char_indices();
         let mut source_range = None;
         let mut path_range = 0..asset_path.len();
         let mut label_range = None;
-        while let Some((index, char)) = chars.next() {
+
+        // Loop through the characters of the passed in &str to accomplish the following:
+        // 1. Seach for the first instance of the `://` substring. If the `://` substring is found,
+        //  store the range of indices representing everything before the `://` substring as the `source_range`.
+        // 2. Seach for the last instance of the `#` character. If the `#` character is found,
+        //  store the range of indices representing everything after the `#` character as the `label_range`
+        // 3. Set the `path_range` to be everything in between the `source_range` and `label_range`,
+        //  excluding the `://` substring and `#` character.
+        // 4. Verify that there are no `#` characters in the `AssetPath::source` and no `://` substrings in the `AssetPath::label`
+        let mut source_delimiter_chars_matched = 0;
+        let mut last_found_source_index = 0;
+        for (index, char) in chars {
             match char {
                 ':' => {
-                    let (_, char) = chars
-                        .next()
-                        .ok_or(ParseAssetPathError::InvalidSourceSyntax)?;
-                    if char != '/' {
-                        return Err(ParseAssetPathError::InvalidSourceSyntax);
-                    }
-                    let (index, char) = chars
-                        .next()
-                        .ok_or(ParseAssetPathError::InvalidSourceSyntax)?;
-                    if char != '/' {
-                        return Err(ParseAssetPathError::InvalidSourceSyntax);
+                    source_delimiter_chars_matched = 1;
+                }
+                '/' => {
+                    match source_delimiter_chars_matched {
+                        1 => {
+                            source_delimiter_chars_matched = 2;
+                        }
+                        2 => {
+                            // If we haven't found our first `AssetPath::source` yet, check to make sure it is valid and then store it.
+                            if source_range.is_none() {
+                                // If the `AssetPath::source` containes a `#` character, it is invalid.
+                                if label_range.is_some() {
+                                    return Err(ParseAssetPathError::InvalidSourceSyntax);
+                                }
+                                source_range = Some(0..index - 2);
+                                path_range.start = index + 1;
+                            }
+                            last_found_source_index = index - 2;
+                            source_delimiter_chars_matched = 0;
+                        }
+                        _ => {}
                     }
-                    source_range = Some(0..index - 2);
-                    path_range.start = index + 1;
                 }
                 '#' => {
                     path_range.end = index;
                     label_range = Some(index + 1..asset_path.len());
-                    break;
+                    source_delimiter_chars_matched = 0;
+                }
+                _ => {
+                    source_delimiter_chars_matched = 0;
                 }
-                _ => {}
             }
         }
-
+        // If we found an `AssetPath::label`
+        if let Some(range) = label_range.clone() {
+            // If the `AssetPath::label` contained a `://` substring, it is invalid.
+            if range.start <= last_found_source_index {
+                return Err(ParseAssetPathError::InvalidLabelSyntax);
+            }
+        }
+        // Try to parse the range of indices that represents the `AssetPath::source` portion of the `AssetPath` to make sure it is not empty.
+        // This would be the case if the input &str was something like `://some/file.test`
         let source = match source_range {
             Some(source_range) => {
                 if source_range.is_empty() {
@@ -169,6 +206,8 @@ impl<'a> AssetPath<'a> {
             }
             None => None,
         };
+        // Try to parse the range of indices that represents the `AssetPath::label` portion of the `AssetPath` to make sure it is not empty.
+        // This would be the case if the input &str was something like `some/file.test#`.
         let label = match label_range {
             Some(label_range) => {
                 if label_range.is_empty() {
@@ -700,6 +739,33 @@ mod tests {
             Ok((Some("http"), Path::new("a/b.test"), Some("Foo")))
         );
 
+        let result = AssetPath::parse_internal("localhost:80/b.test");
+        assert_eq!(result, Ok((None, Path::new("localhost:80/b.test"), None)));
+
+        let result = AssetPath::parse_internal("http://localhost:80/b.test");
+        assert_eq!(
+            result,
+            Ok((Some("http"), Path::new("localhost:80/b.test"), None))
+        );
+
+        let result = AssetPath::parse_internal("http://localhost:80/b.test#Foo");
+        assert_eq!(
+            result,
+            Ok((Some("http"), Path::new("localhost:80/b.test"), Some("Foo")))
+        );
+
+        let result = AssetPath::parse_internal("#insource://a/b.test");
+        assert_eq!(result, Err(crate::ParseAssetPathError::InvalidSourceSyntax));
+
+        let result = AssetPath::parse_internal("source://a/b.test#://inlabel");
+        assert_eq!(result, Err(crate::ParseAssetPathError::InvalidLabelSyntax));
+
+        let result = AssetPath::parse_internal("#insource://a/b.test#://inlabel");
+        assert!(
+            result == Err(crate::ParseAssetPathError::InvalidSourceSyntax)
+                || result == Err(crate::ParseAssetPathError::InvalidLabelSyntax)
+        );
+
         let result = AssetPath::parse_internal("http://");
         assert_eq!(result, Ok((Some("http"), Path::new(""), None)));
 
@@ -708,9 +774,6 @@ mod tests {
 
         let result = AssetPath::parse_internal("a/b.test#");
         assert_eq!(result, Err(crate::ParseAssetPathError::MissingLabel));
-
-        let result = AssetPath::parse_internal("http:/");
-        assert_eq!(result, Err(crate::ParseAssetPathError::InvalidSourceSyntax));
     }
 
     #[test]