From cc3086fb463b16baec04e7d000f1ebb2efcc6707 Mon Sep 17 00:00:00 2001 From: ChanTsune <41658782+ChanTsune@users.noreply.github.com> Date: Tue, 6 Jan 2026 20:57:14 +0900 Subject: [PATCH 1/2] :recycle: Consolidate OwnerOptions and same_owner into PermissionStrategy Refactor permission/ownership handling to follow the TimestampStrategy pattern, creating a single source of truth for all permission-related configuration. Changes: - Add OwnerSource enum with NoRestore and FromSource variants - Change PermissionStrategy from Never/Always to Never/Preserve{owner} - Add PermissionStrategyResolver with context-specific resolution: - resolve_for_creation(): always FromSource (ownership stored) - resolve_for_extraction(): respects same_owner flag - Remove OwnerOptions struct and owner_options from CreateOptions - Update all command modules to use the new resolver pattern This consolidation eliminates the scattered ownership concerns (OwnerOptions struct, same_owner boolean, PermissionStrategy enum) into a unified architecture where ownership handling is embedded within PermissionStrategy. --- cli/src/command/append.rs | 29 +++--- cli/src/command/core.rs | 144 ++++++++++++++++------------ cli/src/command/core/permission.rs | 25 +++++ cli/src/command/create.rs | 35 +++---- cli/src/command/extract.rs | 145 ++++++++++++++++------------- cli/src/command/stdio.rs | 124 +++++++++++------------- cli/src/command/update.rs | 29 +++--- 7 files changed, 285 insertions(+), 246 deletions(-) create mode 100644 cli/src/command/core/permission.rs diff --git a/cli/src/command/append.rs b/cli/src/command/append.rs index d932dae79..69c4304ff 100644 --- a/cli/src/command/append.rs +++ b/cli/src/command/append.rs @@ -7,9 +7,9 @@ use crate::{ Command, ask_password, check_password, core::{ AclStrategy, CollectOptions, CollectedItem, CreateOptions, FflagsStrategy, KeepOptions, - MacMetadataStrategy, OwnerOptions, PathFilter, PathTransformers, PathnameEditor, - PermissionStrategy, TimeFilterResolver, TimestampStrategyResolver, XattrStrategy, - collect_items_from_paths, create_entry, entry_option, + MacMetadataStrategy, PathFilter, PathTransformers, PathnameEditor, + PermissionStrategyResolver, TimeFilterResolver, TimestampStrategyResolver, + XattrStrategy, collect_items_from_paths, create_entry, entry_option, re::{bsd::SubstitutionRule, gnu::TransformRule}, read_paths, read_paths_stdin, }, @@ -385,22 +385,22 @@ fn append_to_archive(args: AppendCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname: args.uname, + gname: args.gname, + uid: args.uid, + gid: args.gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::Never, mac_metadata_strategy: MacMetadataStrategy::Never, }; - let owner_options = OwnerOptions::new( - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let time_filters = TimeFilterResolver { newer_ctime_than: args.newer_ctime_than.as_deref(), older_ctime_than: args.older_ctime_than.as_deref(), @@ -415,7 +415,6 @@ fn append_to_archive(args: AppendCommand) -> anyhow::Result<()> { let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), diff --git a/cli/src/command/core.rs b/cli/src/command/core.rs index 19d998e29..7b98147b0 100644 --- a/cli/src/command/core.rs +++ b/cli/src/command/core.rs @@ -2,11 +2,13 @@ pub(crate) mod path; mod path_filter; pub(crate) mod path_lock; mod path_transformer; +pub(crate) mod permission; pub(crate) mod re; pub(crate) mod time_filter; pub(crate) mod timestamp; pub(crate) use self::path::PathnameEditor; +pub(crate) use self::permission::{OwnerSource, PermissionStrategy}; pub(crate) use self::timestamp::{TimeSource, TimestampStrategy}; use crate::{ cli::{CipherAlgorithmArgs, CompressionAlgorithmArgs, HashAlgorithmArgs}, @@ -125,24 +127,6 @@ impl XattrStrategy { } } -#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub(crate) enum PermissionStrategy { - Never, - Always, -} - -impl PermissionStrategy { - pub(crate) const fn from_flags(keep_permission: bool, no_keep_permission: bool) -> Self { - if no_keep_permission { - Self::Never - } else if keep_permission { - Self::Always - } else { - Self::Never - } - } -} - #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] pub(crate) enum AclStrategy { Never, @@ -211,7 +195,7 @@ impl MacMetadataStrategy { } } -#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] pub(crate) struct KeepOptions { pub(crate) timestamp_strategy: TimestampStrategy, pub(crate) permission_strategy: PermissionStrategy, @@ -270,36 +254,59 @@ impl TimestampStrategyResolver { } } -#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub(crate) struct OwnerOptions { +/// Resolves CLI permission options into a `PermissionStrategy`. +/// +/// This struct encapsulates the CLI-specific logic for determining permission behavior: +/// - `no_keep_permission` forces `Never` +/// - `keep_permission` enables `Preserve` with ownership handling +/// - `same_owner` controls whether to restore ownership (extraction only) +/// - Owner override fields (uid, gid, uname, gname) are used in both creation and extraction +pub(crate) struct PermissionStrategyResolver { + pub(crate) keep_permission: bool, + pub(crate) no_keep_permission: bool, + pub(crate) same_owner: bool, pub(crate) uname: Option, pub(crate) gname: Option, pub(crate) uid: Option, pub(crate) gid: Option, + pub(crate) numeric_owner: bool, } -impl OwnerOptions { - #[inline] - pub(crate) fn new( - uname: Option, - gname: Option, - uid: Option, - gid: Option, - numeric_owner: bool, - ) -> Self { - Self { - uname: if numeric_owner { - Some(String::new()) - } else { - uname - }, - gname: if numeric_owner { - Some(String::new()) - } else { - gname - }, - uid, - gid, +impl PermissionStrategyResolver { + /// Resolves CLI options to PermissionStrategy. + /// + /// The `same_owner` field controls ownership handling: + /// - `true`: Restore/store ownership (FromSource) + /// - `false`: Skip ownership restoration (NoRestore, extraction only) + /// + /// For creation contexts, pass `same_owner: true` since ownership + /// is always stored when `--keep-permission` is enabled. + pub(crate) fn resolve(self) -> PermissionStrategy { + if self.no_keep_permission { + PermissionStrategy::Never + } else if self.keep_permission { + PermissionStrategy::Preserve { + owner: if self.same_owner { + OwnerSource::FromSource { + uname: if self.numeric_owner { + Some(String::new()) + } else { + self.uname + }, + gname: if self.numeric_owner { + Some(String::new()) + } else { + self.gname + }, + uid: self.uid, + gid: self.gid, + } + } else { + OwnerSource::NoRestore + }, + } + } else { + PermissionStrategy::Never } } } @@ -308,7 +315,6 @@ impl OwnerOptions { pub(crate) struct CreateOptions { pub(crate) option: WriteOptions, pub(crate) keep_options: KeepOptions, - pub(crate) owner_options: OwnerOptions, pub(crate) pathname_editor: PathnameEditor, } @@ -638,7 +644,6 @@ pub(crate) fn create_entry( CreateOptions { option, keep_options, - owner_options, pathname_editor, }: &CreateOptions, ) -> io::Result> { @@ -656,22 +661,22 @@ pub(crate) fn create_entry( return Ok(None); }; let entry = EntryBuilder::new_hard_link(entry_name, reference)?; - apply_metadata(entry, path, keep_options, owner_options, metadata)?.build() + apply_metadata(entry, path, keep_options, metadata)?.build() } StoreAs::Symlink => { let source = fs::read_link(path)?; let reference = pathname_editor.edit_symlink(&source); let entry = EntryBuilder::new_symlink(entry_name, reference)?; - apply_metadata(entry, path, keep_options, owner_options, metadata)?.build() + apply_metadata(entry, path, keep_options, metadata)?.build() } StoreAs::File => { let mut entry = EntryBuilder::new_file(entry_name, option)?; write_from_path(&mut entry, path)?; - apply_metadata(entry, path, keep_options, owner_options, metadata)?.build() + apply_metadata(entry, path, keep_options, metadata)?.build() } StoreAs::Dir => { let entry = EntryBuilder::new_dir(entry_name); - apply_metadata(entry, path, keep_options, owner_options, metadata)?.build() + apply_metadata(entry, path, keep_options, metadata)?.build() } } .map(Some) @@ -704,7 +709,6 @@ pub(crate) fn apply_metadata( mut entry: EntryBuilder, path: &Path, keep_options: &KeepOptions, - owner_options: &OwnerOptions, meta: &fs::Metadata, ) -> io::Result { if let TimestampStrategy::Preserve { @@ -724,16 +728,26 @@ pub(crate) fn apply_metadata( } } #[cfg(unix)] - if let PermissionStrategy::Always = keep_options.permission_strategy { + if let PermissionStrategy::Preserve { ref owner } = keep_options.permission_strategy { use crate::utils::fs::{Group, User}; use std::os::unix::fs::{MetadataExt, PermissionsExt}; let mode = meta.permissions().mode() as u16; - let uid = owner_options.uid.unwrap_or(meta.uid()); - let gid = owner_options.gid.unwrap_or(meta.gid()); + // Extract owner overrides from OwnerSource + let (o_uid, o_gid, o_uname, o_gname) = match owner { + OwnerSource::NoRestore => unreachable!("NoRestore is not used during creation"), + OwnerSource::FromSource { + uid, + gid, + uname, + gname, + } => (*uid, *gid, uname.as_deref(), gname.as_deref()), + }; + let uid = o_uid.unwrap_or(meta.uid()); + let gid = o_gid.unwrap_or(meta.gid()); entry.permission(pna::Permission::new( uid.into(), - match owner_options.uname.as_deref() { + match o_uname { None => User::from_uid(uid.into())? .name() .unwrap_or_default() @@ -741,7 +755,7 @@ pub(crate) fn apply_metadata( Some(uname) => uname.into(), }, gid.into(), - match owner_options.gname.as_deref() { + match o_gname { None => Group::from_gid(gid.into())? .name() .unwrap_or_default() @@ -752,7 +766,7 @@ pub(crate) fn apply_metadata( )); } #[cfg(windows)] - if let PermissionStrategy::Always = keep_options.permission_strategy { + if let PermissionStrategy::Preserve { ref owner } = keep_options.permission_strategy { use crate::utils::os::windows::{fs::stat, security::SecurityDescriptor}; let sd = SecurityDescriptor::try_from(path)?; @@ -760,11 +774,21 @@ pub(crate) fn apply_metadata( let mode = stat.st_mode; let user = sd.owner_sid()?; let group = sd.group_sid()?; + // Extract owner overrides from OwnerSource + let (o_uid, o_gid, o_uname, o_gname) = match owner { + OwnerSource::NoRestore => unreachable!("NoRestore is not used during creation"), + OwnerSource::FromSource { + uid, + gid, + uname, + gname, + } => (*uid, *gid, uname.clone(), gname.clone()), + }; entry.permission(pna::Permission::new( - owner_options.uid.map_or(u64::MAX, Into::into), - owner_options.uname.clone().unwrap_or(user.name), - owner_options.gid.map_or(u64::MAX, Into::into), - owner_options.gname.clone().unwrap_or(group.name), + o_uid.map_or(u64::MAX, Into::into), + o_uname.unwrap_or(user.name), + o_gid.map_or(u64::MAX, Into::into), + o_gname.unwrap_or(group.name), mode, )); } diff --git a/cli/src/command/core/permission.rs b/cli/src/command/core/permission.rs new file mode 100644 index 000000000..6c61f7d1e --- /dev/null +++ b/cli/src/command/core/permission.rs @@ -0,0 +1,25 @@ +/// How to handle ownership during permission operations. +#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub(crate) enum OwnerSource { + /// Don't restore ownership (extraction only - same as !same_owner) + /// During extraction: chmod() is called but lchown() is NOT + NoRestore, + /// Use ownership from source with optional overrides. + /// During creation: Override values stored in archive (None = use filesystem) + /// During extraction: Override values restored from archive (None = use archive) + FromSource { + uname: Option, + gname: Option, + uid: Option, + gid: Option, + }, +} + +#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub(crate) enum PermissionStrategy { + Never, + /// Preserve permissions with configurable ownership handling + Preserve { + owner: OwnerSource, + }, +} diff --git a/cli/src/command/create.rs b/cli/src/command/create.rs index 70de014ec..c19b2cb81 100644 --- a/cli/src/command/create.rs +++ b/cli/src/command/create.rs @@ -7,9 +7,10 @@ use crate::{ Command, ask_password, check_password, core::{ AclStrategy, CollectOptions, CollectedItem, CreateOptions, FflagsStrategy, KeepOptions, - MIN_SPLIT_PART_BYTES, MacMetadataStrategy, OwnerOptions, PathFilter, PathTransformers, - PathnameEditor, PermissionStrategy, TimeFilterResolver, TimestampStrategyResolver, - XattrStrategy, collect_items_from_paths, create_entry, entry_option, + MIN_SPLIT_PART_BYTES, MacMetadataStrategy, PathFilter, PathTransformers, + PathnameEditor, PermissionStrategyResolver, TimeFilterResolver, + TimestampStrategyResolver, XattrStrategy, collect_items_from_paths, create_entry, + entry_option, re::{bsd::SubstitutionRule, gnu::TransformRule}, read_paths, read_paths_stdin, write_split_archive, }, @@ -471,22 +472,22 @@ fn create_archive(args: CreateCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname: args.uname, + gname: args.gname, + uid: args.uid, + gid: args.gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::Never, mac_metadata_strategy: MacMetadataStrategy::Never, }; - let owner_options = OwnerOptions::new( - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let pathname_editor = PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), @@ -497,7 +498,6 @@ fn create_archive(args: CreateCommand) -> anyhow::Result<()> { let creation_context = CreationContext { write_option, keep_options, - owner_options, solid: args.solid, pathname_editor, }; @@ -526,7 +526,6 @@ fn create_archive(args: CreateCommand) -> anyhow::Result<()> { pub(crate) struct CreationContext { pub(crate) write_option: WriteOptions, pub(crate) keep_options: KeepOptions, - pub(crate) owner_options: OwnerOptions, pub(crate) solid: bool, pub(crate) pathname_editor: PathnameEditor, } @@ -536,7 +535,6 @@ pub(crate) fn create_archive_file( CreationContext { write_option, keep_options, - owner_options, solid, pathname_editor, }: CreationContext, @@ -555,7 +553,6 @@ where let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor, }; rayon::scope_fifo(|s| { @@ -599,7 +596,6 @@ fn create_archive_with_split( CreationContext { write_option, keep_options, - owner_options, solid, pathname_editor, }: CreationContext, @@ -616,7 +612,6 @@ fn create_archive_with_split( let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor, }; rayon::scope_fifo(|s| -> anyhow::Result<()> { diff --git a/cli/src/command/extract.rs b/cli/src/command/extract.rs index 1f331fa8f..46dd2fef3 100644 --- a/cli/src/command/extract.rs +++ b/cli/src/command/extract.rs @@ -8,10 +8,10 @@ use crate::{ command::{ Command, ask_password, core::{ - AclStrategy, FflagsStrategy, KeepOptions, MacMetadataStrategy, OwnerOptions, - PathFilter, PathTransformers, PathnameEditor, PermissionStrategy, TimeFilterResolver, - TimeFilters, TimestampStrategy, TimestampStrategyResolver, XattrStrategy, apply_chroot, - collect_split_archives, + AclStrategy, FflagsStrategy, KeepOptions, MacMetadataStrategy, OwnerSource, PathFilter, + PathTransformers, PathnameEditor, PermissionStrategy, PermissionStrategyResolver, + TimeFilterResolver, TimeFilters, TimestampStrategy, TimestampStrategyResolver, + XattrStrategy, apply_chroot, collect_split_archives, path_lock::PathLocks, re::{bsd::SubstitutionRule, gnu::TransformRule}, read_paths, run_process_archive, @@ -414,22 +414,22 @@ fn extract_archive(args: ExtractCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: !args.no_same_owner, + uname: args.uname, + gname: args.gname, + uid: args.uid, + gid: args.gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::Never, mac_metadata_strategy: MacMetadataStrategy::Never, }; - let owner_options = OwnerOptions::new( - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let output_options = OutputOption { overwrite_strategy, allow_unsafe_links: args.allow_unsafe_links, @@ -437,8 +437,6 @@ fn extract_archive(args: ExtractCommand) -> anyhow::Result<()> { to_stdout: false, filter, keep_options, - owner_options, - same_owner: !args.no_same_owner, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), @@ -521,8 +519,6 @@ pub(crate) struct OutputOption<'a> { pub(crate) to_stdout: bool, pub(crate) filter: PathFilter<'a>, pub(crate) keep_options: KeepOptions, - pub(crate) owner_options: OwnerOptions, - pub(crate) same_owner: bool, pub(crate) pathname_editor: PathnameEditor, pub(crate) path_locks: Arc, pub(crate) unlink_first: bool, @@ -681,8 +677,6 @@ pub(crate) fn extract_entry<'a, T>( to_stdout, filter, keep_options, - owner_options, - same_owner, pathname_editor, path_locks, unlink_first, @@ -835,7 +829,7 @@ where fs::hard_link(original, &path)?; } } - restore_metadata(&item, &path, keep_options, owner_options, same_owner)?; + restore_metadata(&item, &path, keep_options)?; drop(path_guard); log::debug!("end: {}", path.display()); Ok(()) @@ -874,22 +868,20 @@ fn restore_timestamps( Ok(()) } -/// Restores file metadata (permissions, extended attributes, ACLs, and macOS metadata) for an extracted entry according to the provided keep and owner options. +/// Restores file metadata (permissions, extended attributes, ACLs, and macOS metadata) for an extracted entry according to the provided keep options. /// -/// Permissions are applied when `keep_options.permission_strategy` is `Always`. Extended attributes are applied on Unix when `keep_options.xattr_strategy` is `Always` (logs a warning if the filesystem or platform does not support xattrs). ACLs are restored when the `acl` feature is enabled and `keep_options.acl_strategy` requests them; if the `acl` feature is not compiled in but ACLs were requested, a warning is emitted. macOS metadata (AppleDouble) is restored when `keep_options.mac_metadata_strategy` is `Always` on macOS. +/// Permissions are applied when `keep_options.permission_strategy` is `Preserve`. Extended attributes are applied on Unix when `keep_options.xattr_strategy` is `Always` (logs a warning if the filesystem or platform does not support xattrs). ACLs are restored when the `acl` feature is enabled and `keep_options.acl_strategy` requests them; if the `acl` feature is not compiled in but ACLs were requested, a warning is emitted. macOS metadata (AppleDouble) is restored when `keep_options.mac_metadata_strategy` is `Always` on macOS. fn restore_metadata( item: &NormalEntry, path: &Path, keep_options: &KeepOptions, - owner_options: &OwnerOptions, - same_owner: &bool, ) -> io::Result<()> where T: AsRef<[u8]>, { - if let PermissionStrategy::Always = keep_options.permission_strategy { + if let PermissionStrategy::Preserve { ref owner } = keep_options.permission_strategy { if let Some(p) = item.metadata().permission() { - restore_permissions(*same_owner, path, p, owner_options)?; + restore_permissions(path, p, owner)?; } } // On macOS, when mac_metadata_strategy is Always and the entry has mac_metadata, @@ -1020,35 +1012,79 @@ fn restore_acls(path: &Path, acls: Acls, acl_strategy: AclStrategy) -> io::Resul Ok(()) } +/// Resolves the user and group to use for ownership restoration. +/// +/// Priority: +/// 1. Override uid/gid if specified +/// 2. Override uname/gname if specified, searching by name +/// 3. Archive's uname/gname with fallback to archive's uid/gid +fn resolve_owner( + permission: &Permission, + uname_override: Option<&str>, + gname_override: Option<&str>, + uid_override: Option, + gid_override: Option, +) -> (Option, Option) { + let user = if let Some(uid) = uid_override { + User::from_uid(uid.into()).ok() + } else { + let name = uname_override.unwrap_or(permission.uname()); + search_owner(name, permission.uid()).ok() + }; + let group = if let Some(gid) = gid_override { + Group::from_gid(gid.into()).ok() + } else { + let name = gname_override.unwrap_or(permission.gname()); + search_group(name, permission.gid()).ok() + }; + (user, group) +} + #[inline] -fn restore_permissions( - same_owner: bool, - path: &Path, - p: &Permission, - options: &OwnerOptions, -) -> io::Result<()> { - let permissions = permissions(p, options); +fn restore_permissions(path: &Path, p: &Permission, owner: &OwnerSource) -> io::Result<()> { #[cfg(unix)] - if let Some((p, u, g)) = permissions { - if same_owner { - match lchown(path, u, g) { + { + // Restore ownership only when owner is FromSource (i.e., same_owner is true) + if let OwnerSource::FromSource { + uname, + gname, + uid, + gid, + } = owner + { + let (user, group) = resolve_owner(p, uname.as_deref(), gname.as_deref(), *uid, *gid); + match lchown(path, user, group) { Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { log::warn!("failed to restore owner of {}: {}", path.display(), e) } r => r?, } } + // Always restore mode permissions utils::os::unix::fs::chmod(path, p.permissions())?; - }; + } #[cfg(windows)] - if let Some((p, u, g)) = permissions { - if same_owner { - lchown(path, u, g)?; + { + if let OwnerSource::FromSource { + uname, + gname, + uid, + gid, + } = owner + { + let (user, group) = resolve_owner(p, uname.as_deref(), gname.as_deref(), *uid, *gid); + match lchown(path, user, group) { + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + log::warn!("failed to restore owner of {}: {}", path.display(), e) + } + r => r?, + } } utils::os::windows::fs::chmod(path, p.permissions())?; } #[cfg(not(any(unix, windows)))] - if let Some(_) = permissions { + { + let _ = (path, p, owner); log::warn!("Currently permission is not supported on this platform."); } Ok(()) @@ -1124,29 +1160,6 @@ fn ensure_directory_components(path: &Path, unlink_first: bool) -> io::Result<() Ok(()) } -fn permissions<'p>( - permission: &'p Permission, - owner_options: &'_ OwnerOptions, -) -> Option<(&'p Permission, Option, Option)> { - let user = if let Some(uid) = owner_options.uid { - User::from_uid(uid.into()) - } else { - search_owner( - owner_options.uname.as_deref().unwrap_or(permission.uname()), - permission.uid(), - ) - }; - let group = if let Some(gid) = owner_options.gid { - Group::from_gid(gid.into()) - } else { - search_group( - owner_options.gname.as_deref().unwrap_or(permission.gname()), - permission.gid(), - ) - }; - Some((permission, user.ok(), group.ok())) -} - fn search_owner(name: &str, id: u64) -> io::Result { let user = User::from_name(name); if user.is_ok() { diff --git a/cli/src/command/stdio.rs b/cli/src/command/stdio.rs index bdc9a9fa3..aaa93a0cb 100644 --- a/cli/src/command/stdio.rs +++ b/cli/src/command/stdio.rs @@ -9,8 +9,8 @@ use crate::{ ask_password, check_password, core::{ AclStrategy, CollectOptions, CreateOptions, FflagsStrategy, KeepOptions, - MacMetadataStrategy, OwnerOptions, PathFilter, PathTransformers, PathnameEditor, - PermissionStrategy, TimeFilterResolver, TimestampStrategyResolver, + MacMetadataStrategy, PathFilter, PathTransformers, PathnameEditor, + PermissionStrategyResolver, TimeFilterResolver, TimestampStrategyResolver, TransformStrategyUnSolid, XattrStrategy, apply_chroot, collect_items_from_paths, collect_split_archives, entry_option, path_lock::PathLocks, @@ -648,6 +648,8 @@ fn run_create_archive(args: StdioCommand) -> anyhow::Result<()> { let password = password.as_deref(); let cli_option = entry_option(args.compression, args.cipher, args.hash, password); + let (uname, uid) = resolve_name_id(args.owner, args.uname, args.uid); + let (gname, gid) = resolve_name_id(args.group, args.gname, args.gid); let keep_options = KeepOptions { timestamp_strategy: TimestampStrategyResolver { keep_timestamp: args.keep_timestamp, @@ -661,10 +663,17 @@ fn run_create_archive(args: StdioCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname, + gname, + uid, + gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::from_flags(args.keep_fflags, args.no_keep_fflags), @@ -673,19 +682,9 @@ fn run_create_archive(args: StdioCommand) -> anyhow::Result<()> { args.no_mac_metadata, ), }; - let owner_options = resolve_owner_options( - args.owner, - args.group, - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let creation_context = CreationContext { write_option: cli_option, keep_options, - owner_options, solid: args.solid, pathname_editor: PathnameEditor::new( args.strip_components, @@ -740,6 +739,8 @@ fn run_extract_archive(args: StdioCommand) -> anyhow::Result<()> { older_mtime: args.older_mtime.map(|it| it.to_system_time()), } .resolve()?; + let (uname, uid) = resolve_name_id(args.owner, args.uname, args.uid); + let (gname, gid) = resolve_name_id(args.group, args.gname, args.gid); let out_option = OutputOption { overwrite_strategy, allow_unsafe_links: args.allow_unsafe_links, @@ -759,10 +760,17 @@ fn run_extract_archive(args: StdioCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: !args.no_same_owner, + uname, + gname, + uid, + gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::from_flags(args.keep_fflags, args.no_keep_fflags), @@ -771,16 +779,6 @@ fn run_extract_archive(args: StdioCommand) -> anyhow::Result<()> { args.no_mac_metadata, ), }, - owner_options: resolve_owner_options( - args.owner, - args.group, - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ), - same_owner: !args.no_same_owner, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), @@ -910,6 +908,8 @@ fn run_append(args: StdioCommand) -> anyhow::Result<()> { check_password(&password, &args.cipher); let password = password.as_deref(); let option = entry_option(args.compression, args.cipher, args.hash, password); + let (uname, uid) = resolve_name_id(args.owner, args.uname, args.uid); + let (gname, gid) = resolve_name_id(args.group, args.gname, args.gid); let keep_options = KeepOptions { timestamp_strategy: TimestampStrategyResolver { keep_timestamp: args.keep_timestamp, @@ -923,10 +923,17 @@ fn run_append(args: StdioCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname, + gname, + uid, + gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::from_flags(args.keep_fflags, args.no_keep_fflags), @@ -935,19 +942,9 @@ fn run_append(args: StdioCommand) -> anyhow::Result<()> { args.no_mac_metadata, ), }; - let owner_options = resolve_owner_options( - args.owner, - args.group, - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), @@ -1020,20 +1017,6 @@ fn run_append(args: StdioCommand) -> anyhow::Result<()> { } } -fn resolve_owner_options( - owner: Option, - group: Option, - uname: Option, - gname: Option, - uid: Option, - gid: Option, - numeric_owner: bool, -) -> OwnerOptions { - let (uname, uid) = resolve_name_id(owner, uname, uid); - let (gname, gid) = resolve_name_id(group, gname, gid); - OwnerOptions::new(uname, gname, uid, gid, numeric_owner) -} - fn resolve_name_id( spec: Option, name: Option, @@ -1051,6 +1034,8 @@ fn run_update(args: StdioCommand) -> anyhow::Result<()> { check_password(&password, &args.cipher); let password = password.as_deref(); let option = entry_option(args.compression, args.cipher, args.hash, password); + let (uname, uid) = resolve_name_id(args.owner, args.uname, args.uid); + let (gname, gid) = resolve_name_id(args.group, args.gname, args.gid); let keep_options = KeepOptions { timestamp_strategy: TimestampStrategyResolver { keep_timestamp: args.keep_timestamp, @@ -1064,10 +1049,17 @@ fn run_update(args: StdioCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname, + gname, + uid, + gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::from_flags(args.keep_fflags, args.no_keep_fflags), @@ -1076,17 +1068,9 @@ fn run_update(args: StdioCommand) -> anyhow::Result<()> { args.no_mac_metadata, ), }; - let owner_options = OwnerOptions::new( - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), diff --git a/cli/src/command/update.rs b/cli/src/command/update.rs index 8af498211..4911af54f 100644 --- a/cli/src/command/update.rs +++ b/cli/src/command/update.rs @@ -11,9 +11,9 @@ use crate::{ Command, ask_password, check_password, core::{ AclStrategy, CollectOptions, CollectedItem, CreateOptions, FflagsStrategy, KeepOptions, - MacMetadataStrategy, OwnerOptions, PathFilter, PathTransformers, PathnameEditor, - PermissionStrategy, TimeFilterResolver, TimestampStrategyResolver, TransformStrategy, - TransformStrategyKeepSolid, TransformStrategyUnSolid, XattrStrategy, + MacMetadataStrategy, PathFilter, PathTransformers, PathnameEditor, + PermissionStrategyResolver, TimeFilterResolver, TimestampStrategyResolver, + TransformStrategy, TransformStrategyKeepSolid, TransformStrategyUnSolid, XattrStrategy, collect_items_from_paths, collect_split_archives, create_entry, entry_option, re::{bsd::SubstitutionRule, gnu::TransformRule}, read_paths, read_paths_stdin, @@ -394,22 +394,22 @@ fn update_archive(args: UpdateCommand) -> anyhow::Result<()> { clamp_atime: args.clamp_atime, } .resolve(), - permission_strategy: PermissionStrategy::from_flags( - args.keep_permission, - args.no_keep_permission, - ), + permission_strategy: PermissionStrategyResolver { + keep_permission: args.keep_permission, + no_keep_permission: args.no_keep_permission, + same_owner: true, // Must be `true` for creation + uname: args.uname, + gname: args.gname, + uid: args.uid, + gid: args.gid, + numeric_owner: args.numeric_owner, + } + .resolve(), xattr_strategy: XattrStrategy::from_flags(args.keep_xattr, args.no_keep_xattr), acl_strategy: AclStrategy::from_flags(args.keep_acl, args.no_keep_acl), fflags_strategy: FflagsStrategy::Never, mac_metadata_strategy: MacMetadataStrategy::Never, }; - let owner_options = OwnerOptions::new( - args.uname, - args.gname, - args.uid, - args.gid, - args.numeric_owner, - ); let time_filters = TimeFilterResolver { newer_ctime_than: args.newer_ctime_than.as_deref(), older_ctime_than: args.older_ctime_than.as_deref(), @@ -424,7 +424,6 @@ fn update_archive(args: UpdateCommand) -> anyhow::Result<()> { let create_options = CreateOptions { option, keep_options, - owner_options, pathname_editor: PathnameEditor::new( args.strip_components, PathTransformers::new(args.substitutions, args.transforms), From 849cb43a6dfb6c86620cc0b03817c6c9a687b333 Mon Sep 17 00:00:00 2001 From: ChanTsune <41658782+ChanTsune@users.noreply.github.com> Date: Tue, 6 Jan 2026 13:35:00 +0000 Subject: [PATCH 2/2] :recycle: Add unified utils::fs::chmod wrapper and simplify restore_permissions - Add chmod wrapper in utils/fs.rs following lchown pattern - Consolidate duplicate Unix/Windows chmod calls in extract.rs --- cli/src/command/extract.rs | 23 ++--------------------- cli/src/utils/fs.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/cli/src/command/extract.rs b/cli/src/command/extract.rs index 46dd2fef3..04382836a 100644 --- a/cli/src/command/extract.rs +++ b/cli/src/command/extract.rs @@ -1042,7 +1042,7 @@ fn resolve_owner( #[inline] fn restore_permissions(path: &Path, p: &Permission, owner: &OwnerSource) -> io::Result<()> { - #[cfg(unix)] + #[cfg(any(unix, windows))] { // Restore ownership only when owner is FromSource (i.e., same_owner is true) if let OwnerSource::FromSource { @@ -1061,26 +1061,7 @@ fn restore_permissions(path: &Path, p: &Permission, owner: &OwnerSource) -> io:: } } // Always restore mode permissions - utils::os::unix::fs::chmod(path, p.permissions())?; - } - #[cfg(windows)] - { - if let OwnerSource::FromSource { - uname, - gname, - uid, - gid, - } = owner - { - let (user, group) = resolve_owner(p, uname.as_deref(), gname.as_deref(), *uid, *gid); - match lchown(path, user, group) { - Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { - log::warn!("failed to restore owner of {}: {}", path.display(), e) - } - r => r?, - } - } - utils::os::windows::fs::chmod(path, p.permissions())?; + utils::fs::chmod(path, p.permissions())?; } #[cfg(not(any(unix, windows)))] { diff --git a/cli/src/utils/fs.rs b/cli/src/utils/fs.rs index a9489c8fa..47f452a5e 100644 --- a/cli/src/utils/fs.rs +++ b/cli/src/utils/fs.rs @@ -66,6 +66,19 @@ pub(crate) fn lchown>( inner(path.as_ref(), owner, group) } +#[cfg(any(windows, unix))] +pub(crate) fn chmod>(path: P, mode: u16) -> io::Result<()> { + #[cfg(windows)] + fn inner(path: &Path, mode: u16) -> io::Result<()> { + windows::fs::chmod(path, mode) + } + #[cfg(unix)] + fn inner(path: &Path, mode: u16) -> io::Result<()> { + crate::utils::os::unix::fs::chmod(path, mode) + } + inner(path.as_ref(), mode) +} + pub(crate) fn get_flags>(path: P) -> io::Result> { #[cfg(any( target_os = "macos",