fix: throw error when asset is empty #3379
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ealmloff
reviewed
Dec 17, 2024
| path: input.clone(), | ||
| }) | ||
| let path = | ||
| std::path::absolute(manifest_dir.join(raw.trim_start_matches('/'))).map_err(|err| { |
There was a problem hiding this comment.
std::path::absolute does not check if the path exists (unlike canonicalize) so this error does not trigger when the path doesn't exist
ealmloff
reviewed
Dec 17, 2024
|
|
||
| // 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 }); |
There was a problem hiding this comment.
Instead, this case triggers which gives the wrong error message
|
Looks good overall once the error message is fixed. I don't think the first if statement to check if the path should change behavior because the other changes have more robust checks that thow errors for |
ealmloff
approved these changes
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on the docsite I found that if you passed
asset!("")on accident, then we'd end up copying the entire current directory over. This is not good. It also causes newdx serveto hang infinitely since we get stuck on clearing the assets dir.This fixes that by throwing an error if the path is empty and ensuring the final path is a child of the crate.