From 01c8ecb7781ffb70a9ff95bae98e710f029760ad Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 6 Mar 2023 17:54:31 +1100 Subject: [PATCH] Fix zip extraction and `bins::infer_bin_dir_template` (#869) - Fix zip extraction code: Ensure dir is rwx and file is readable for curr user - Add more integration test for `ExtractedFiles` - Fix `bins::infer_bin_dir_template` introduced in #856 Signed-off-by: Jiahao XU --- crates/binstalk-downloader/src/download.rs | 34 ++++++++++++++++++- .../src/download/extracted_files.rs | 18 ++++++---- .../src/download/zip_extraction.rs | 12 +++++-- crates/binstalk/src/bins.rs | 6 +--- 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/crates/binstalk-downloader/src/download.rs b/crates/binstalk-downloader/src/download.rs index a0412e07..bebd7bce 100644 --- a/crates/binstalk-downloader/src/download.rs +++ b/crates/binstalk-downloader/src/download.rs @@ -204,6 +204,7 @@ mod test { ) .unwrap(); + // cargo-binstall let cargo_binstall_url = "https://github.com/cargo-bins/cargo-binstall/releases/download/v0.20.1/cargo-binstall-aarch64-unknown-linux-musl.tgz"; let extracted_files = @@ -232,9 +233,10 @@ mod test { ]) ); + // cargo-watch let cargo_watch_url = "https://github.com/watchexec/cargo-watch/releases/download/v8.4.0/cargo-watch-v8.4.0-aarch64-unknown-linux-gnu.tar.xz"; - let extracted_files = Download::new(client, Url::parse(cargo_watch_url).unwrap()) + let extracted_files = Download::new(client.clone(), Url::parse(cargo_watch_url).unwrap()) .and_extract(PkgFmt::Txz, tempdir().unwrap()) .await .unwrap(); @@ -276,5 +278,35 @@ mod test { assert!(!extracted_files.has_file(&dir.join("asdfcqwe"))); assert!(extracted_files.has_file(&dir.join("completions/zsh"))); + + // sccache, tgz and zip + let sccache_config = [ + ("https://github.com/mozilla/sccache/releases/download/v0.3.3/sccache-v0.3.3-x86_64-pc-windows-msvc.tar.gz", PkgFmt::Tgz), + ("https://github.com/mozilla/sccache/releases/download/v0.3.3/sccache-v0.3.3-x86_64-pc-windows-msvc.zip", PkgFmt::Zip), + ]; + + for (sccache_url, fmt) in sccache_config { + let extracted_files = Download::new(client.clone(), Url::parse(sccache_url).unwrap()) + .and_extract(fmt, tempdir().unwrap()) + .await + .unwrap(); + + let dir = Path::new("sccache-v0.3.3-x86_64-pc-windows-msvc"); + + assert_eq!( + extracted_files.get_dir(Path::new(".")).unwrap(), + &HashSet::from([dir.as_os_str().into()]) + ); + + assert_eq!( + extracted_files.get_dir(dir).unwrap(), + &HashSet::from_iter( + ["README.md", "LICENSE", "sccache.exe"] + .iter() + .map(OsStr::new) + .map(Box::::from) + ), + ); + } } } diff --git a/crates/binstalk-downloader/src/download/extracted_files.rs b/crates/binstalk-downloader/src/download/extracted_files.rs index 2cc4c90a..87074e00 100644 --- a/crates/binstalk-downloader/src/download/extracted_files.rs +++ b/crates/binstalk-downloader/src/download/extracted_files.rs @@ -79,14 +79,18 @@ impl ExtractedFiles { } } - /// * `path` - must be canonical and must not be empty, but could be "." - /// for top-level. + /// * `path` - must be a relative path without `.`, `..`, `/`, `prefix:/` + /// and must not be empty, for these values it is guaranteed to + /// return `None`. + /// But could be set to "." for top-level. pub fn get_entry(&self, path: &Path) -> Option<&ExtractedFilesEntry> { self.0.get(path) } - /// * `path` - must be canonical and must not be empty, but could be "." - /// for top-level. + /// * `path` - must be a relative path without `.`, `..`, `/`, `prefix:/` + /// and must not be empty, for these values it is guaranteed to + /// return `None`. + /// But could be set to "." for top-level. pub fn get_dir(&self, path: &Path) -> Option<&HashSet>> { match self.get_entry(path)? { ExtractedFilesEntry::Dir(file_names) => Some(file_names), @@ -94,8 +98,10 @@ impl ExtractedFiles { } } - /// * `path` - must be canonical and must not be empty, but could be "." - /// for top-level. + /// * `path` - must be a relative path without `.`, `..`, `/`, `prefix:/` + /// and must not be empty, for these values it is guaranteed to + /// return `false`. + /// But could be set to "." for top-level. pub fn has_file(&self, path: &Path) -> bool { matches!(self.get_entry(path), Some(ExtractedFilesEntry::File)) } diff --git a/crates/binstalk-downloader/src/download/zip_extraction.rs b/crates/binstalk-downloader/src/download/zip_extraction.rs index cf48465c..fad0f481 100644 --- a/crates/binstalk-downloader/src/download/zip_extraction.rs +++ b/crates/binstalk-downloader/src/download/zip_extraction.rs @@ -54,17 +54,25 @@ where // Get permissions let mut perms = None; + let is_dir = raw_filename.ends_with('/'); + #[cfg(unix)] { use std::{fs::Permissions, os::unix::fs::PermissionsExt}; if let Some(mode) = zip_reader.entry().unix_permissions() { - let mode: u16 = mode; + // If it is a dir, then it needs to be at least rwx for the current + // user so that we can create new files, search for existing files + // and list its contents. + // + // If it is a file, then it needs to be at least readable for the + // current user. + let mode: u16 = mode | if is_dir { 0o700 } else { 0o400 }; perms = Some(Permissions::from_mode(mode as u32)); } } - if raw_filename.ends_with('/') { + if is_dir { extracted_files.add_dir(&filename); // This entry is a dir. diff --git a/crates/binstalk/src/bins.rs b/crates/binstalk/src/bins.rs index 6ab63626..2e216b55 100644 --- a/crates/binstalk/src/bins.rs +++ b/crates/binstalk/src/bins.rs @@ -56,11 +56,7 @@ pub fn infer_bin_dir_template(data: &Data, extracted_files: &ExtractedFiles) -> gen_possible_dirs .into_iter() .map(|gen_possible_dir| gen_possible_dir(name, target, version)) - .find(|dirname| { - extracted_files - .get_dir(&data.bin_path.join(dirname)) - .is_some() - }) + .find(|dirname| extracted_files.get_dir(Path::new(&dirname)).is_some()) .map(|mut dir| { dir.reserve_exact(1 + default_bin_dir_template.len()); dir += "/";