forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

AssetPath source parse fix (#11543)

Browse Source 
# Objective


Fixes #11533 


When `AssetPath`s are created from a string type, they are parsed into
an `AssetSource`, a `Path`, and a `Label`.
The current method of parsing has some unnecessary quirks:

- The presence of a `:` character is assumed to be the start of an asset
source indicator.
- This is not necessarily true. There are valid uses of a `:` character
in an asset path, for example an http source's port such as
`localhost:80`.
- If there are multiple instances of `://`, the last one is assumed to
be the asset source deliminator.
- This has some unexpected behavior. Even in a fully formed path, such
as `http://localhost:80`, the `:` between `localhost` and `80` is
assumed to be the start of an asset source, causing an error since it
does not form the full sequence `://`.


## Solution
Changes the `AssetPath`'s `parse_internal` method to be more permissive.
- Only the exact sequence `://` is taken to be the asset source
deliminator, and only the first one if there are multiple.
- As a consequence, it is no longer possible to detect a malformed asset
source deliminator, and so the corresponding error was removed.
This commit is contained in:
thepackett 2024-01-26 15:23:06 -06:00 committed by GitHub
parent 7ae36a99c8
commit 182c21dc58
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 85 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
crates/bevy_asset/src/path.rs
View File

@ -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);
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;
}
_ => {}
}
let (index, char) = chars
.next()
.ok_or(ParseAssetPathError::InvalidSourceSyntax)?;
if char != '/' {
return Err(ParseAssetPathError::InvalidSourceSyntax);
}
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]