Skip to content

Commit f794d8a

Browse files
authored
Don't panic in AssetPath deserialization (#23935)
## Objective `AssetPath` deserialization panics if the path is malformed. This seems a bit rude, and prevents the user from getting deserialization errors that would help track down the problem. ## Solution Instead of calling `AssetPath::parse`, call `AssetPath::try_parse` and handle any errors. Now, calling `ron::de::from_str` on `"a/b.test#"` returns a useful error: ```rust Err(SpannedError { code: Message( "Asset label must be at least one character. Either specify the label after the '#' or remove the '#'", ), span: Span { start: Position { line: 1, col: 2, }, end: Position { line: 1, col: 12 }, }, }) ``` The PR also removes the `visit_string` method of the `AssetPath` deserializer. `visit_string` is an optimization that avoids copies when the deserializer can take ownership of the string (see [serde docs](https://docs.rs/serde/latest/serde/de/trait.Visitor.html#method.visit_string)). But `AssetPath` didn't take ownership of the string, so there's no reason to implement it. ## Testing ```sh cargo test -p bevy_asset cargo run --example world_serialization ```
1 parent 4dc8fb4 commit f794d8a

1 file changed

Lines changed: 10 additions & 8 deletions

File tree

crates/bevy_asset/src/path.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -680,14 +680,10 @@ impl<'de> Visitor<'de> for AssetPathVisitor {
680680
where
681681
E: serde::de::Error,
682682
{
683-
Ok(AssetPath::parse(v).into_owned())
684-
}
685-
686-
fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
687-
where
688-
E: serde::de::Error,
689-
{
690-
Ok(AssetPath::from(v))
683+
match AssetPath::try_parse(v) {
684+
Ok(path) => Ok(path.into_owned()),
685+
Err(err) => Err(E::custom(err)),
686+
}
691687
}
692688
}
693689

@@ -772,6 +768,12 @@ mod tests {
772768
assert_eq!(result, Err(crate::ParseAssetPathError::MissingLabel));
773769
}
774770

771+
#[test]
772+
fn test_serialize() {
773+
assert!(ron::de::from_str::<AssetPath>("\"a/b.test\"").is_ok());
774+
assert!(ron::de::from_str::<AssetPath>("\"a/b.test#\"").is_err());
775+
}
776+
775777
#[test]
776778
fn test_parent() {
777779
// Parent consumes path segments, returns None when insufficient

0 commit comments

Comments
 (0)