Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throw error when asset is empty #3379

Merged
merged 7 commits into from
Dec 17, 2024
Merged

fix: throw error when asset is empty #3379

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b6b9694
fix: throw error when asset is empty
jkelleyrtp Dec 17, 2024
38f1754
Use std::path::absolute to fix windows unc problems
jkelleyrtp Dec 17, 2024
d4bf1d0
fix: don't use canonicalize at all on windows
jkelleyrtp Dec 17, 2024
39deb42
remove unnecessary check
jkelleyrtp Dec 17, 2024
b29a401
fix comment
jkelleyrtp Dec 17, 2024
6cb2cd7
fix error not actually being thrown
jkelleyrtp Dec 17, 2024
b223317
remove test cases from example
jkelleyrtp Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions packages/manganis/manganis-macro/src/asset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ use syn::{
};

fn resolve_path(raw: &str) -> Result<PathBuf, AssetParseError> {
if raw.is_empty() {
return Err(AssetParseError::InvalidPath {
path: PathBuf::new(),
});
}

// Get the location of the root of the crate which is where all assets are relative to
//
// IE
Expand All @@ -19,19 +25,28 @@ fn resolve_path(raw: &str) -> Result<PathBuf, AssetParseError> {
// /users/dioxus/dev/app/assets/blah.css
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.map(std::path::absolute)
.unwrap()
.unwrap();

// 1. the input file should be a pathbuf
let input = PathBuf::from(raw);

// 2. absolute path to the asset
manifest_dir
.join(raw.trim_start_matches('/'))
.canonicalize()
.map_err(|err| AssetParseError::AssetDoesntExist {
err,
path: input.clone(),
})
let path =
std::path::absolute(manifest_dir.join(raw.trim_start_matches('/'))).map_err(|err| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::path::absolute does not check if the path exists (unlike canonicalize) so this error does not trigger when the path doesn't exist

AssetParseError::AssetDoesntExist {
err,
path: input.clone(),
}
})?;

// 3. Ensure the path is not the current dir or exist outside the current dir
if path == manifest_dir || !path.starts_with(manifest_dir) || !path.exists() {
return Err(AssetParseError::InvalidPath { path });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, this case triggers which gives the wrong error message

}

Ok(path)
}

fn hash_file_contents(file_path: &Path) -> Result<u64, AssetParseError> {
Expand Down Expand Up @@ -82,6 +97,9 @@ pub(crate) enum AssetParseError {
err: std::io::Error,
path: std::path::PathBuf,
},
InvalidPath {
path: std::path::PathBuf,
},
FailedToReadAsset(std::io::Error),
}

Expand All @@ -91,6 +109,13 @@ impl std::fmt::Display for AssetParseError {
AssetParseError::AssetDoesntExist { err, path } => {
write!(f, "Asset at {} doesn't exist: {}", path.display(), err)
}
AssetParseError::InvalidPath { path } => {
write!(
f,
"Asset path {} is invalid. Make sure the asset exists within this crate.",
path.display()
)
}
AssetParseError::FailedToReadAsset(err) => write!(f, "Failed to read asset: {}", err),
}
}
Expand Down
Loading