From fe3f8293ef278e21649e6b891f36c8f0d8a5d696 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 14 Jan 2024 00:01:45 +0100 Subject: [PATCH 1/6] uucore: add a new feature called fsxattr --- Cargo.lock | 1 + src/uucore/Cargo.toml | 2 + src/uucore/src/lib/features.rs | 2 + src/uucore/src/lib/features/fsxattr.rs | 116 +++++++++++++++++++++++++ src/uucore/src/lib/lib.rs | 8 +- 5 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 src/uucore/src/lib/features/fsxattr.rs diff --git a/Cargo.lock b/Cargo.lock index 978c7fc3f..0ef909d73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3209,6 +3209,7 @@ dependencies = [ "wild", "winapi-util", "windows-sys 0.48.0", + "xattr", "z85", ] diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 44f8bb2d1..557cdc4dd 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -54,6 +54,7 @@ sm3 = { workspace = true, optional = true } [target.'cfg(unix)'.dependencies] walkdir = { workspace = true, optional = true } nix = { workspace = true, features = ["fs", "uio", "zerocopy", "signal"] } +xattr = { workspace = true, optional = true } [dev-dependencies] clap = { workspace = true } @@ -77,6 +78,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "thiserror"] entries = ["libc"] fs = ["dunce", "libc", "winapi-util", "windows-sys"] fsext = ["libc", "time", "windows-sys"] +fsxattr = [ "xattr" ] lines = [] format = ["itertools"] mode = ["libc"] diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index e26de487b..423ff34ba 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -46,6 +46,8 @@ pub mod pipes; #[cfg(all(unix, feature = "process"))] pub mod process; +#[cfg(all(unix, not(target_os = "macos"), feature = "fsxattr"))] +pub mod fsxattr; #[cfg(all(unix, not(target_os = "fuchsia"), feature = "signals"))] pub mod signals; #[cfg(all( diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs new file mode 100644 index 000000000..7bda023f9 --- /dev/null +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -0,0 +1,116 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +//! Set of functions to manage xattr on files and dirs +use std::collections::HashMap; +use std::ffi::OsString; +use std::path::Path; + +/// Copies extended attributes (xattrs) from one file or directory to another. +/// +/// # Arguments +/// +/// * `source` - A reference to the source path. +/// * `dest` - A reference to the destination path. +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn copy_xattrs>(source: P, dest: P) -> std::io::Result<()> { + for attr_name in xattr::list(&source)? { + if let Some(value) = xattr::get(&source, &attr_name)? { + xattr::set(&dest, &attr_name, &value)?; + } + } + Ok(()) +} + +/// Retrieves the extended attributes (xattrs) of a given file or directory. +/// +/// # Arguments +/// +/// * `source` - A reference to the path of the file or directory. +/// +/// # Returns +/// +/// A result containing a HashMap of attributes names and values, or an error. +pub fn retrieve_xattrs>(source: P) -> std::io::Result>> { + let mut attrs = HashMap::new(); + for attr_name in xattr::list(&source)? { + if let Some(value) = xattr::get(&source, &attr_name)? { + attrs.insert(attr_name, value); + } + } + Ok(attrs) +} + +/// Applies extended attributes (xattrs) to a given file or directory. +/// +/// # Arguments +/// +/// * `dest` - A reference to the path of the file or directory. +/// * `xattrs` - A HashMap containing attribute names and their corresponding values. +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn apply_xattrs>( + dest: P, + xattrs: HashMap>, +) -> std::io::Result<()> { + for (attr, value) in xattrs { + xattr::set(&dest, &attr, &value)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs::File; + use tempfile::tempdir; + + #[test] + fn test_copy_xattrs() { + let temp_dir = tempdir().unwrap(); + let source_path = temp_dir.path().join("source.txt"); + let dest_path = temp_dir.path().join("dest.txt"); + + File::create(&source_path).unwrap(); + File::create(&dest_path).unwrap(); + + let test_attr = "user.test"; + let test_value = b"test value"; + xattr::set(&source_path, test_attr, test_value).unwrap(); + + copy_xattrs(&source_path, &dest_path).unwrap(); + + let copied_value = xattr::get(&dest_path, test_attr).unwrap().unwrap(); + assert_eq!(copied_value, test_value); + } + + #[test] + fn test_apply_and_retrieve_xattrs() { + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("test_file.txt"); + + File::create(&file_path).unwrap(); + + let mut test_xattrs = HashMap::new(); + let test_attr = "user.test_attr"; + let test_value = b"test value"; + test_xattrs.insert(OsString::from(test_attr), test_value.to_vec()); + apply_xattrs(&file_path, test_xattrs).unwrap(); + + let retrieved_xattrs = retrieve_xattrs(&file_path).unwrap(); + assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str())); + assert_eq!( + retrieved_xattrs + .get(OsString::from(test_attr).as_os_str()) + .unwrap(), + test_value + ); + } +} diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 2fc0ae301..6f8400589 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -43,8 +43,6 @@ pub use crate::features::encoding; pub use crate::features::format; #[cfg(feature = "fs")] pub use crate::features::fs; -#[cfg(feature = "fsext")] -pub use crate::features::fsext; #[cfg(feature = "lines")] pub use crate::features::lines; #[cfg(feature = "quoting-style")] @@ -89,6 +87,12 @@ pub use crate::features::utmpx; #[cfg(all(windows, feature = "wide"))] pub use crate::features::wide; +#[cfg(feature = "fsext")] +pub use crate::features::fsext; + +#[cfg(all(unix, not(target_os = "macos"), feature = "fsxattr"))] +pub use crate::features::fsxattr; + //## core functions use std::ffi::OsString; From 238fb776ad01146976212ce3e331dd3bbe8a45f4 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 14 Jan 2024 00:38:17 +0100 Subject: [PATCH 2/6] test: add a function to compare the xattr between two files. used by cp & mv (at least) --- tests/common/util.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/common/util.rs b/tests/common/util.rs index 5dac61f7e..9055c238e 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -756,6 +756,26 @@ pub fn get_root_path() -> &'static str { } } +/// Compares the extended attributes (xattrs) of two files or directories. +/// +/// # Returns +/// +/// `true` if both paths have the same set of extended attributes, `false` otherwise. +#[cfg(all(unix, not(target_os = "macos")))] +pub fn compare_xattrs>(path1: P, path2: P) -> bool { + let get_sorted_xattrs = |path: P| { + xattr::list(path) + .map(|attrs| { + let mut attrs = attrs.collect::>(); + attrs.sort(); + attrs + }) + .unwrap_or_else(|_| Vec::new()) + }; + + get_sorted_xattrs(path1) == get_sorted_xattrs(path2) +} + /// Object-oriented path struct that represents and operates on /// paths relative to the directory it was constructed for. #[derive(Clone)] @@ -3375,4 +3395,26 @@ mod tests { ); assert!(command.tmpd.is_some()); } + + #[cfg(all(unix, not(target_os = "macos")))] + #[test] + fn test_compare_xattrs() { + use tempfile::tempdir; + + let temp_dir = tempdir().unwrap(); + let file_path1 = temp_dir.path().join("test_file1.txt"); + let file_path2 = temp_dir.path().join("test_file2.txt"); + + File::create(&file_path1).unwrap(); + File::create(&file_path2).unwrap(); + + let test_attr = "user.test_attr"; + let test_value = b"test value"; + xattr::set(&file_path1, test_attr, test_value).unwrap(); + + assert!(!compare_xattrs(&file_path1, &file_path2)); + + xattr::set(&file_path2, test_attr, test_value).unwrap(); + assert!(compare_xattrs(&file_path1, &file_path2)); + } } From 2ec4e9f78490f5cd8d4ccff4ac87a442da5461b2 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 14 Jan 2024 00:39:00 +0100 Subject: [PATCH 3/6] mv: preserve the xattr Should make tests/mv/acl pass --- .../cspell.dictionaries/jargon.wordlist.txt | 2 + src/uu/mv/Cargo.toml | 1 + src/uu/mv/src/mv.rs | 15 +++++++ src/uucore/Cargo.toml | 2 +- tests/by-util/test_mv.rs | 41 +++++++++++++++++++ 5 files changed, 60 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index dca883dc8..20e26990f 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -42,6 +42,7 @@ fileio filesystem filesystems flamegraph +fsxattr fullblock getfacl gibi @@ -133,6 +134,7 @@ urand whitespace wordlist wordlists +xattrs # * abbreviations consts diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 83a10ef6b..83d68bc3d 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -21,6 +21,7 @@ indicatif = { workspace = true } uucore = { workspace = true, features = [ "backup-control", "fs", + "fsxattr", "update-control", ] } diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 223ac9119..9f24cf770 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -27,7 +27,10 @@ use uucore::fs::{ are_hardlinks_or_one_way_symlink_to_same_file, are_hardlinks_to_same_file, path_ends_with_terminator, }; +#[cfg(all(unix, not(target_os = "macos")))] +use uucore::fsxattr; use uucore::update_control; + // These are exposed for projects (e.g. nushell) that want to create an `Options` value, which // requires these enums pub use uucore::{backup_control::BackupMode, update_control::UpdateMode}; @@ -631,6 +634,10 @@ fn rename_with_fallback( None }; + #[cfg(all(unix, not(target_os = "macos")))] + let xattrs = + fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| std::collections::HashMap::new()); + let result = if let Some(ref pb) = progress_bar { move_dir_with_progress(from, to, &options, |process_info: TransitProcess| { pb.set_position(process_info.copied_bytes); @@ -641,6 +648,9 @@ fn rename_with_fallback( move_dir(from, to, &options) }; + #[cfg(all(unix, not(target_os = "macos")))] + fsxattr::apply_xattrs(to, xattrs).unwrap(); + if let Err(err) = result { return match err.kind { fs_extra::error::ErrorKind::PermissionDenied => Err(io::Error::new( @@ -651,6 +661,11 @@ fn rename_with_fallback( }; } } else { + #[cfg(all(unix, not(target_os = "macos")))] + fs::copy(from, to) + .and_then(|_| fsxattr::copy_xattrs(&from, &to)) + .and_then(|_| fs::remove_file(from))?; + #[cfg(any(target_os = "macos", not(unix)))] fs::copy(from, to).and_then(|_| fs::remove_file(from))?; } } diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 557cdc4dd..8500faeff 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -78,7 +78,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "thiserror"] entries = ["libc"] fs = ["dunce", "libc", "winapi-util", "windows-sys"] fsext = ["libc", "time", "windows-sys"] -fsxattr = [ "xattr" ] +fsxattr = ["xattr"] lines = [] format = ["itertools"] mode = ["libc"] diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 175b91e7d..dd05ffbcd 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1569,6 +1569,47 @@ fn test_mv_dir_into_path_slash() { assert!(at.dir_exists("f/b")); } +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +fn test_acl() { + use std::process::Command; + + use crate::common::util::compare_xattrs; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let path1 = "a"; + let path2 = "b"; + let file = "a/file"; + let file_target = "b/file"; + at.mkdir(path1); + at.mkdir(path2); + at.touch(file); + + let path = at.plus_as_string(file); + // calling the command directly. xattr requires some dev packages to be installed + // and it adds a complex dependency just for a test + match Command::new("setfacl") + .args(["-m", "group::rwx", &path1]) + .status() + .map(|status| status.code()) + { + Ok(Some(0)) => {} + Ok(_) => { + println!("test skipped: setfacl failed"); + return; + } + Err(e) => { + println!("test skipped: setfacl failed with {}", e); + return; + } + } + + scene.ucmd().arg(&path).arg(path2).succeeds(); + + assert!(compare_xattrs(&file, &file_target)); +} + // Todo: // $ at.touch a b From 66637a650321f920d8463155b7afdb1e9c8e113f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 14 Jan 2024 17:46:45 +0100 Subject: [PATCH 4/6] move the file_has_acl function into uucore --- Cargo.lock | 1 - src/uu/ls/Cargo.toml | 4 +-- src/uu/ls/src/ls.rs | 19 +++----------- src/uucore/src/lib/features/fsxattr.rs | 36 ++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ef909d73..cbff54c4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2608,7 +2608,6 @@ dependencies = [ "unicode-width", "uucore", "uutils_term_grid", - "xattr", ] [[package]] diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index 38312eefc..dc79c6f93 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -27,6 +27,7 @@ uucore = { workspace = true, features = [ "colors", "entries", "fs", + "fsxattr", "quoting-style", "version-cmp", ] } @@ -34,9 +35,6 @@ once_cell = { workspace = true } selinux = { workspace = true, optional = true } hostname = { workspace = true } -[target.'cfg(unix)'.dependencies] -xattr = { workspace = true } - [[bin]] name = "ls" path = "src/main.rs" diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 1c89cd353..ed100477f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm getxattr +// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm use clap::{ builder::{NonEmptyStringValueParser, ValueParser}, @@ -36,7 +36,8 @@ use std::{ }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use unicode_width::UnicodeWidthStr; - +#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] +use uucore::fsxattr::has_acl; #[cfg(any( target_os = "linux", target_os = "macos", @@ -2621,18 +2622,6 @@ fn display_grid( Ok(()) } -#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] -fn file_has_acl>(file: P) -> bool { - // don't use exacl here, it is doing more getxattr call then needed - match xattr::list(file) { - Ok(acl) => { - // if we have extra attributes, we have an acl - acl.count() > 0 - } - Err(_) => false, - } -} - /// This writes to the BufWriter out a single string of the output of `ls -l`. /// /// It writes the following keys, in order: @@ -2680,7 +2669,7 @@ fn display_item_long( // TODO: See how Mac should work here let is_acl_set = false; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] - let is_acl_set = file_has_acl(item.display_name.as_os_str()); + let is_acl_set = has_acl(item.display_name.as_os_str()); write!( output_display, "{}{}{} {}", diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 7bda023f9..36ec0ba8e 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -66,6 +66,26 @@ pub fn apply_xattrs>( Ok(()) } +/// Checks if a file has an Access Control List (ACL) based on its extended attributes. +/// +/// # Arguments +/// +/// * `file` - A reference to the path of the file. +/// +/// # Returns +/// +/// `true` if the file has extended attributes (indicating an ACL), `false` otherwise. +pub fn has_acl>(file: P) -> bool { + // don't use exacl here, it is doing more getxattr call then needed + match xattr::list(file) { + Ok(acl) => { + // if we have extra attributes, we have an acl + acl.count() > 0 + } + Err(_) => false, + } +} + #[cfg(test)] mod tests { use super::*; @@ -113,4 +133,20 @@ mod tests { test_value ); } + + #[test] + fn test_file_has_acl() { + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("test_file.txt"); + + File::create(&file_path).unwrap(); + + assert!(!has_acl(&file_path)); + + let test_attr = "user.test_acl"; + let test_value = b"test value"; + xattr::set(&file_path, test_attr, test_value).unwrap(); + + assert!(has_acl(&file_path)); + } } From 3872aca9c66a34ea99566e011c1828e6c0ff20be Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 14 Jan 2024 21:39:59 +0100 Subject: [PATCH 5/6] spell: ignore getxattr --- src/uucore/src/lib/features/fsxattr.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 36ec0ba8e..3cb00edc0 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -3,6 +3,8 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// spell-checker:ignore getxattr + //! Set of functions to manage xattr on files and dirs use std::collections::HashMap; use std::ffi::OsString; From 69c8753f80947dd56bfc9450be2aa36de89c27f8 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 15 Jan 2024 11:28:29 +0100 Subject: [PATCH 6/6] cp test: use compare_xattrs from tests/utils --- tests/by-util/test_cp.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1271909ec..c0d81d9a9 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -57,7 +57,7 @@ static TEST_MOUNT_OTHER_FILESYSTEM_FILE: &str = "mount/DO_NOT_copy_me.txt"; #[cfg(unix)] static TEST_NONEXISTENT_FILE: &str = "nonexistent_file.txt"; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] -use xattr; +use crate::common::util::compare_xattrs; /// Assert that mode, ownership, and permissions of two metadata objects match. #[cfg(all(not(windows), not(target_os = "freebsd")))] @@ -3739,21 +3739,6 @@ fn test_cp_no_such() { .stderr_is("cp: 'no-such/' is not a directory\n"); } -#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] -fn compare_xattrs>(path1: P, path2: P) -> bool { - let get_sorted_xattrs = |path: P| { - xattr::list(path) - .map(|attrs| { - let mut attrs = attrs.collect::>(); - attrs.sort(); - attrs - }) - .unwrap_or_else(|_| Vec::new()) - }; - - get_sorted_xattrs(path1) == get_sorted_xattrs(path2) -} - #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] #[test] fn test_acl_preserve() {