From b65908fac2723c7aaf37d824d96b2ad4e8fa6e25 Mon Sep 17 00:00:00 2001 From: Gabriel Goller Date: Tue, 22 Aug 2023 16:05:17 +0200 Subject: [PATCH] fix #4380: check if file is excluded before running `stat()` Passed a closure with the `stat()` function call to `matches()`. This will traverse through all patterns and try to match using the path only, if a `file_mode` is needed, it will run the closure. This means that if we exclude a file with the `MatchType::ANY_FILE_TYPE`, we will skip it without running `stat()` on it. As we updated the `matches()` function, we also updated all the invocations of it. Added `pathpatterns` crate to local overrides in cargo.toml. Signed-off-by: Gabriel Goller --- Cargo.toml | 5 ++-- pbs-client/src/catalog_shell.rs | 8 ++--- pbs-client/src/pxar/create.rs | 53 ++++++++++++++++++++++----------- pbs-client/src/pxar/extract.rs | 14 +++++---- pbs-datastore/src/catalog.rs | 6 ++-- 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5cbae1b8..560794a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -264,8 +264,9 @@ proxmox-rrd.workspace = true #proxmox-sortable-macro = { path = "../proxmox/proxmox-sortable-macro" } #proxmox-human-byte = { path = "../proxmox/proxmox-human-byte" } -#proxmox-apt = { path = "../proxmox/proxmox-apt" } -#proxmox-openid = { path = "../proxmox/proxmox-openid" } +#proxmox-apt = { path = "../proxmox-apt" } +#proxmox-openid = { path = "../proxmox-openid-rs" } +#pathpatterns = {path = "../pathpatterns" } #pxar = { path = "../pxar" } diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs index b8aaf8cb..f53b3cc5 100644 --- a/pbs-client/src/catalog_shell.rs +++ b/pbs-client/src/catalog_shell.rs @@ -1138,14 +1138,14 @@ impl<'a> ExtractorState<'a> { pub async fn handle_entry(&mut self, entry: catalog::DirEntry) -> Result<(), Error> { let match_result = self.match_list.matches(&self.path, entry.get_file_mode()); let did_match = match match_result { - Some(MatchType::Include) => true, - Some(MatchType::Exclude) => false, - None => self.matches, + Ok(Some(MatchType::Include)) => true, + Ok(Some(MatchType::Exclude)) => false, + _ => self.matches, }; match (did_match, &entry.attr) { (_, DirEntryAttribute::Directory { .. }) => { - self.handle_new_directory(entry, match_result).await?; + self.handle_new_directory(entry, match_result?).await?; } (true, DirEntryAttribute::File { .. }) => { self.dir_stack.push(PathStackEntry::new(entry)); diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs index 2577cf98..2a539957 100644 --- a/pbs-client/src/pxar/create.rs +++ b/pbs-client/src/pxar/create.rs @@ -16,12 +16,12 @@ use nix::fcntl::OFlag; use nix::sys::stat::{FileStat, Mode}; use pathpatterns::{MatchEntry, MatchFlag, MatchList, MatchType, PatternFlag}; +use proxmox_sys::error::SysError; use pxar::encoder::{LinkOffset, SeqWrite}; use pxar::Metadata; use proxmox_io::vec; use proxmox_lang::c_str; -use proxmox_sys::error::SysError; use proxmox_sys::fs::{self, acl, xattr}; use pbs_datastore::catalog::BackupCatalogWriter; @@ -420,7 +420,7 @@ impl Archiver { for file in dir.iter() { let file = file?; - let file_name = file.file_name().to_owned(); + let file_name = file.file_name(); let file_name_bytes = file_name.to_bytes(); if file_name_bytes == b"." || file_name_bytes == b".." { continue; @@ -434,25 +434,42 @@ impl Archiver { assert_single_path_component(os_file_name)?; let full_path = self.path.join(os_file_name); - let stat = match nix::sys::stat::fstatat( - dir_fd, - file_name.as_c_str(), - nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW, - ) { - Ok(stat) => stat, - Err(ref err) if err.not_found() => continue, - Err(err) => return Err(err).context(format!("stat failed on {:?}", full_path)), + let match_path = PathBuf::from("/").join(full_path.clone()); + + let mut stat_results: Option = None; + + let get_file_mode = || { + nix::sys::stat::fstatat( + dir_fd, + file_name.to_owned().as_c_str(), + nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW, + ) }; - let match_path = PathBuf::from("/").join(full_path.clone()); - if self + match self .patterns - .matches(match_path.as_os_str().as_bytes(), Some(stat.st_mode)) - == Some(MatchType::Exclude) - { - continue; + .matches(match_path.as_os_str().as_bytes(), || { + Ok::<_, Errno>(match &stat_results { + Some(result) => result.st_mode, + None => stat_results.insert(get_file_mode()?).st_mode, + }) + }) { + Ok(Some(MatchType::Exclude)) => continue, + Ok(_) => (), + Err(err) => { + if err.not_found() { + continue; + } else { + return Err(err).with_context(|| format!("stat failed on {:?}", full_path)); + } + } } + let stat = stat_results + .map(Ok) + .unwrap_or_else(get_file_mode) + .with_context(|| format!("stat failed on {:?}", full_path))?; + self.entry_counter += 1; if self.entry_counter > self.entry_limit { bail!( @@ -462,7 +479,7 @@ impl Archiver { } file_list.push(FileListEntry { - name: file_name, + name: file_name.to_owned(), path: full_path, stat, }); @@ -533,7 +550,7 @@ impl Archiver { let match_path = PathBuf::from("/").join(self.path.clone()); if self .patterns - .matches(match_path.as_os_str().as_bytes(), Some(stat.st_mode)) + .matches(match_path.as_os_str().as_bytes(), stat.st_mode)? == Some(MatchType::Exclude) { return Ok(()); diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs index 4eb6fb90..1e2e901a 100644 --- a/pbs-client/src/pxar/extract.rs +++ b/pbs-client/src/pxar/extract.rs @@ -251,15 +251,19 @@ where self.extractor.set_path(entry.path().as_os_str().to_owned()); - let match_result = self.match_list.matches( - entry.path().as_os_str().as_bytes(), - Some(metadata.file_type() as u32), - ); + // We can `unwrap()` safely here because we get a `Result<_, std::convert::Infallible>` + let match_result = self + .match_list + .matches( + entry.path().as_os_str().as_bytes(), + metadata.file_type() as u32, + ) + .unwrap(); let did_match = match match_result { Some(MatchType::Include) => true, Some(MatchType::Exclude) => false, - None => self.state.current_match, + _ => self.state.current_match, }; let extract_res = match (did_match, entry.kind()) { diff --git a/pbs-datastore/src/catalog.rs b/pbs-datastore/src/catalog.rs index 11c14b64..86e20c92 100644 --- a/pbs-datastore/src/catalog.rs +++ b/pbs-datastore/src/catalog.rs @@ -678,9 +678,9 @@ impl CatalogReader { } file_path.extend(&e.name); match match_list.matches(&file_path, e.get_file_mode()) { - Some(MatchType::Exclude) => continue, - Some(MatchType::Include) => callback(file_path)?, - None => (), + Ok(Some(MatchType::Exclude)) => continue, + Ok(Some(MatchType::Include)) => callback(file_path)?, + _ => (), } if is_dir { self.find(&e, file_path, match_list, callback)?;