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 <g.goller@proxmox.com>
This commit is contained in:
Gabriel Goller 2023-08-22 16:05:17 +02:00 committed by Wolfgang Bumiller
parent 1b8ff68dc2
commit b65908fac2
5 changed files with 54 additions and 32 deletions

View File

@ -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" }

View File

@ -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));

View File

@ -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<FileStat> = 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(());

View File

@ -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()) {

View File

@ -678,9 +678,9 @@ impl<R: Read + Seek> CatalogReader<R> {
}
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)?;