diff --git a/rmf_site_editor/src/site/drawing.rs b/rmf_site_editor/src/site/drawing.rs index c95ea80e..53dee3fb 100644 --- a/rmf_site_editor/src/site/drawing.rs +++ b/rmf_site_editor/src/site/drawing.rs @@ -103,7 +103,19 @@ pub fn add_drawing_visuals( )), _ => source.clone(), }; - let texture_handle: Handle = asset_server.load(&String::from(&asset_source)); + let asset_path = match String::try_from(&asset_source) { + Ok(asset_path) => asset_path, + Err(err) => { + error!( + "Invalid syntax while creating asset path for a drawing: {err}. \ + Check that your asset information was input correctly. \ + Current value:\n{:?}", + asset_source, + ); + continue; + } + }; + let texture_handle: Handle = asset_server.load(asset_path); commands.entity(e).insert(LoadingDrawing(texture_handle)); } } @@ -194,7 +206,7 @@ pub fn handle_loaded_drawing( .remove::(); } LoadState::Failed => { - error!("Failed loading drawing {:?}", String::from(source)); + error!("Failed loading drawing {:?}", source); commands.entity(entity).remove::(); } _ => {} diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index ab26b6c1..4cf4fddf 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -230,7 +230,19 @@ pub fn update_model_scenes( } _ => source.clone(), }; - let handle = asset_server.load_untyped(&String::from(&asset_source)); + let asset_path = match String::try_from(&asset_source) { + Ok(asset_path) => asset_path, + Err(err) => { + error!( + "Invalid syntax while creating asset path for a model: {err}. \ + Check that your asset information was input correctly. \ + Current value:\n{:?}", + asset_source, + ); + return; + } + }; + let handle = asset_server.load_untyped(asset_path); commands .insert(PreventDeletion::because( "Waiting for model to spawn".to_string(), @@ -305,8 +317,19 @@ pub fn update_model_tentative_formats( continue; } } - let path = String::from(source); - let model_ext = path + let asset_path = match String::try_from(source) { + Ok(asset_path) => asset_path, + Err(err) => { + error!( + "Invalid syntax while creating asset path to load a model: {err}. \ + Check that your asset information was input correctly. \ + Current value:\n{:?}", + source, + ); + continue; + } + }; + let model_ext = asset_path .rsplit_once('.') .map(|s| s.1.to_owned()) .unwrap_or_else(|| tentative_format.to_string("")); @@ -323,7 +346,7 @@ pub fn update_model_tentative_formats( _ => "Failed parsing file".to_owned(), } }; - warn!("Failed loading Model with source {}: {}", path, reason); + warn!("Failed loading Model with source {}: {}", asset_path, reason); cmd.remove::(); } } diff --git a/rmf_site_editor/src/site/texture.rs b/rmf_site_editor/src/site/texture.rs index d4326e79..cf4854d2 100644 --- a/rmf_site_editor/src/site/texture.rs +++ b/rmf_site_editor/src/site/texture.rs @@ -28,10 +28,23 @@ pub fn fetch_image_for_texture( asset_server: Res, ) { for (e, image, texture) in &mut changed_textures { + let asset_path = match String::try_from(&texture.source) { + Ok(asset_path) => asset_path, + Err(err) => { + error!( + "Invalid syntax while creating asset path: {err}. \ + Check that your asset information was input correctly. \ + Current value:\n{:?}", + texture.source, + ); + continue; + } + }; + if let Some(mut image) = image { - *image = asset_server.load(String::from(&texture.source)); + *image = asset_server.load(asset_path); } else { - let image: Handle = asset_server.load(String::from(&texture.source)); + let image: Handle = asset_server.load(asset_path); commands.entity(e).insert(image); } } diff --git a/rmf_site_editor/src/workcell/urdf_package_exporter/generate_package.rs b/rmf_site_editor/src/workcell/urdf_package_exporter/generate_package.rs index c62b9011..e9ea3fa1 100644 --- a/rmf_site_editor/src/workcell/urdf_package_exporter/generate_package.rs +++ b/rmf_site_editor/src/workcell/urdf_package_exporter/generate_package.rs @@ -82,7 +82,7 @@ fn convert_and_copy_meshes( fn get_path_to_asset_file(asset_source: &AssetSource) -> Result> { match asset_source { AssetSource::Package(_) => Ok(urdf_rs::utils::expand_package_path( - &(String::from(asset_source)), + &(String::try_from(asset_source)?), None, ) .to_string() diff --git a/rmf_site_format/src/asset_source.rs b/rmf_site_format/src/asset_source.rs index e9a6e77d..0913e210 100644 --- a/rmf_site_format/src/asset_source.rs +++ b/rmf_site_format/src/asset_source.rs @@ -17,7 +17,10 @@ use crate::*; #[cfg(feature = "bevy")] -use bevy::prelude::{Component, Reflect, ReflectComponent}; +use bevy::{ + prelude::{Component, Reflect, ReflectComponent}, + asset::{AssetPath, ParseAssetPathError}, +}; use pathdiff::diff_paths; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; @@ -75,6 +78,17 @@ impl AssetSource { Ok(()) } + + /// Convert the asset source into an asset path without attempting to validate + /// whether the asset path has valid syntax. + pub unsafe fn as_unvalidated_asset_path(&self) -> String { + match self { + AssetSource::Remote(uri) => String::from("rmf-server://") + uri, + AssetSource::Local(filename) => String::from("file://") + filename, + AssetSource::Search(name) => String::from("search://") + name, + AssetSource::Package(path) => String::from("package://") + path, + } + } } impl Default for AssetSource { @@ -83,14 +97,19 @@ impl Default for AssetSource { } } -impl From<&AssetSource> for String { - fn from(asset_source: &AssetSource) -> String { - match asset_source { - AssetSource::Remote(uri) => String::from("rmf-server://") + uri, - AssetSource::Local(filename) => String::from("file://") + filename, - AssetSource::Search(name) => String::from("search://") + name, - AssetSource::Package(path) => String::from("package://") + path, - } +#[cfg(feature = "bevy")] +impl TryFrom<&AssetSource> for String { + type Error = ParseAssetPathError; + fn try_from(asset_source: &AssetSource) -> Result { + // SAFETY: After we get this string, we immediately validate it before + // returning it. + let result = unsafe { asset_source.as_unvalidated_asset_path() }; + + // Verify that the string can be parsed as an asset path before we + // return it. + AssetPath::try_parse(&result) + .map(|_| ()) // drop the borrowing of result + .map(|_| result) } } diff --git a/rmf_site_format/src/legacy/building_map.rs b/rmf_site_format/src/legacy/building_map.rs index fef34f47..b8c1f903 100644 --- a/rmf_site_format/src/legacy/building_map.rs +++ b/rmf_site_format/src/legacy/building_map.rs @@ -630,7 +630,11 @@ impl BuildingMap { let textures = textures .into_iter() .map(|(id, texture)| { - let name: String = (&texture.source).into(); + // SAFETY: We're picking the string apart to automatically generate + // a name for the texture. We don't need to validate the syntax + // because what we produce here will only exist to be viewed by + // humans. + let name: String = unsafe { (&texture.source).as_unvalidated_asset_path() }; let name = Path::new(&name) .file_stem() .map(|s| s.to_str().map(|s| s.to_owned())) diff --git a/rmf_site_format/src/workcell.rs b/rmf_site_format/src/workcell.rs index 3bb54b73..842dc5e1 100644 --- a/rmf_site_format/src/workcell.rs +++ b/rmf_site_format/src/workcell.rs @@ -494,7 +494,10 @@ impl From for urdf_rs::Geometry { fn from(geometry: Geometry) -> Self { match geometry { Geometry::Mesh { source, scale } => urdf_rs::Geometry::Mesh { - filename: (&source).into(), + // SAFETY: We don't need to validate the syntax of the asset + // path because that will be done later when we attempt to load + // this as an asset. + filename: unsafe { (&source).as_unvalidated_asset_path() }, scale: scale.map(|v| urdf_rs::Vec3([v.x as f64, v.y as f64, v.z as f64])), }, Geometry::Primitive(PrimitiveShape::Box { size: [x, y, z] }) => {