Skip to content

Commit 5148ba1

Browse files
committed
set_selinux_security_context: also display the error from the crate
+ fix comments from review
1 parent b3a2b74 commit 5148ba1

File tree

3 files changed

+14
-20
lines changed

3 files changed

+14
-20
lines changed

src/uu/cp/src/copydir.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,6 @@ pub(crate) fn copy_directory(
436436
&entry.source_absolute,
437437
&entry.local_to_target,
438438
&options.attributes,
439-
options,
440439
)?;
441440
}
442441
}
@@ -467,7 +466,6 @@ pub(crate) fn copy_directory(
467466
&entry.source_absolute,
468467
&entry.local_to_target,
469468
&options.attributes,
470-
options,
471469
)?;
472470
}
473471
}
@@ -478,7 +476,7 @@ pub(crate) fn copy_directory(
478476
let dest = target.join(root.file_name().unwrap());
479477
for (x, y) in aligned_ancestors(root, dest.as_path()) {
480478
if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
481-
copy_attributes(&src, y, &options.attributes, options)?;
479+
copy_attributes(&src, y, &options.attributes)?;
482480
}
483481
}
484482
}

src/uu/cp/src/cp.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,7 +1492,7 @@ fn copy_source(
14921492
if options.parents {
14931493
for (x, y) in aligned_ancestors(source, dest.as_path()) {
14941494
if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
1495-
copy_attributes(&src, y, &options.attributes, options)?;
1495+
copy_attributes(&src, y, &options.attributes)?;
14961496
}
14971497
}
14981498
}
@@ -1640,12 +1640,10 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> {
16401640
}
16411641

16421642
/// Copy the specified attributes from one path to another.
1643-
#[allow(unused_variables)]
16441643
pub(crate) fn copy_attributes(
16451644
source: &Path,
16461645
dest: &Path,
16471646
attributes: &Attributes,
1648-
options: &Options,
16491647
) -> CopyResult<()> {
16501648
let context = &*format!("{} -> {}", source.quote(), dest.quote());
16511649
let source_metadata = fs::symlink_metadata(source).context(context)?;
@@ -2442,23 +2440,26 @@ fn copy_file(
24422440
if options.dereference(source_in_command_line) {
24432441
if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) {
24442442
if src.exists() {
2445-
copy_attributes(&src, dest, &options.attributes, options)?;
2443+
copy_attributes(&src, dest, &options.attributes)?;
24462444
}
24472445
}
24482446
} else if source_is_stream && source.exists() {
24492447
// Some stream files may not exist after we have copied it,
24502448
// like anonymous pipes. Thus, we can't really copy its
24512449
// attributes. However, this is already handled in the stream
24522450
// copy function (see `copy_stream` under platform/linux.rs).
2453-
copy_attributes(source, dest, &options.attributes, options)?;
24542451
} else {
2455-
copy_attributes(source, dest, &options.attributes, options)?;
2452+
copy_attributes(source, dest, &options.attributes)?;
24562453
}
24572454

24582455
#[cfg(feature = "selinux")]
24592456
if options.set_selinux_context && uucore::selinux::is_selinux_enabled() {
24602457
// Set the given selinux permissions on the copied file.
2461-
uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())?;
2458+
if let Err(e) =
2459+
uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())
2460+
{
2461+
return Err(Error::Error(format!("SELinux error: {}", e)));
2462+
}
24622463
}
24632464

24642465
copied_files.insert(

tests/by-util/test_cp.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6292,9 +6292,7 @@ fn test_cp_selinux() {
62926292

62936293
assert!(
62946294
selinux_perm.contains("unconfined_u"),
6295-
"Expected '{}' not found in getfattr output:\n{}",
6296-
"foo",
6297-
selinux_perm
6295+
"Expected 'foo' not found in getfattr output:\n{selinux_perm}"
62986296
);
62996297
at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST));
63006298
}
@@ -6342,9 +6340,7 @@ fn test_cp_preserve_selinux() {
63426340
let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST));
63436341
assert!(
63446342
selinux_perm_dest.contains("unconfined_u"),
6345-
"Expected '{}' not found in getfattr output:\n{}",
6346-
"foo",
6347-
selinux_perm_dest
6343+
"Expected 'foo' not found in getfattr output:\n{selinux_perm_dest}"
63486344
);
63496345
assert_eq!(
63506346
get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)),
@@ -6373,8 +6369,8 @@ fn test_cp_preserve_selinux_admin_context() {
63736369
at.touch(TEST_HELLO_WORLD_SOURCE);
63746370

63756371
// Get the default SELinux context for the destination file path
6376-
// on Debian/Ubuntu, this program is provided by the selinux-utils package
6377-
// on Fedora/RHEL, this program is provided by the libselinux-devel package
6372+
// On Debian/Ubuntu, this program is provided by the selinux-utils package
6373+
// On Fedora/RHEL, this program is provided by the libselinux-devel package
63786374
let output = std::process::Command::new("matchpathcon")
63796375
.arg(at.plus_as_string(TEST_HELLO_WORLD_DEST))
63806376
.output()
@@ -6432,7 +6428,6 @@ fn test_cp_selinux_context_priority() {
64326428
let ts = TestScenario::new(util_name!());
64336429
let at = &ts.fixtures;
64346430

6435-
// Create two different files
64366431
at.write(TEST_HELLO_WORLD_SOURCE, "source content");
64376432

64386433
// First, set a known context on source file (only if system supports it)
@@ -6642,7 +6637,7 @@ fn test_cp_preserve_context_root() {
66426637
}
66436638

66446639
// Copy the file with preserved context
6645-
// Only works at root
6640+
// Only works as root
66466641
if let Ok(result) = run_ucmd_as_root(&scene, &["--preserve=context", source_file, dest_file]) {
66476642
let src_ctx = get_getfattr_output(&at.plus_as_string(source_file));
66486643
let dest_ctx = get_getfattr_output(&at.plus_as_string(dest_file));

0 commit comments

Comments
 (0)