Update zip dependency and fix deprecation warnings #3617
Conversation
Update zip crate to latest version, and fix deprecation warnings using functionally equivalent APIs
| @@ -57,7 +72,7 @@ pub fn extract_files(from_archive: File, dest: &Path, files: Vec<PathBuf>) -> io | |||
| let mut archive = zip_rs::ZipArchive::new(from_archive).expect("archive file exists"); | |||
| for x in files { | |||
| if let Ok(file) = archive.by_name(x.to_str().expect("valid path")) { | |||
| let path = dest.join(file.sanitized_name()); | |||
| let path = dest.join(file.mangled_name()); | |||
There was a problem hiding this comment.
Is enclosed_name a better option here, based on the doc comments?
There was a problem hiding this comment.
I looked at that too, but we actually want the side-effects of mangled_name.
There was a problem hiding this comment.
I'm 👍 on replacing sanitized_name with mangled_name based on the deprecated sanitized_name simply delegating to mangled_name.
But it might be worth revisiting with a separate PR to consider replacing with enclosed_name.
From the docs referenced by @trevyn -
/// This will read well-formed ZIP files correctly, and is resistant
/// to path-based exploits. It is recommended over
/// [`ZipFile::mangled_name`].
| @@ -37,7 +52,7 @@ pub fn create_zip(dst_file: &File, src_dir: &Path, files: Vec<PathBuf>) -> io::R | |||
| let file_path = src_dir.join(x); | |||
| if let Ok(file) = File::open(file_path.clone()) { | |||
| info!("compress: {:?} -> {:?}", file_path, x); | |||
| writer.get_mut().start_file_from_path(x, options)?; | |||
| writer.get_mut().start_file(path_to_string(x), options)?; | |||
There was a problem hiding this comment.
Would just x.to_str() work here? We generate the path, so it shouldn't need to be sanitized with the path_to_string function, right?
There was a problem hiding this comment.
path_to_string performs extra checks that x.to_str() does not, like normalizing the path. I was also going for functional equivalence to what the previous function does. If you look at zip-rs source, this is exactly what the start_file_from_path function does internally.
|
@GeneFerneau check the comments when you find the time, let's get this one in if it is easy to resolve those 👍 |
Addressed the comments. Let me know if that answers your questions. |
Update zip crate to latest version, and fix deprecation warnings using functionally equivalent APIs