1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 19:47:45 +00:00

Revert "install: create destination file with safer modes before copy"

This commit is contained in:
Sylvestre Ledru 2024-12-04 11:08:28 +01:00 committed by GitHub
parent d878f6c774
commit 0780e26914
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 14 additions and 80 deletions

View file

@ -13,12 +13,9 @@ use filetime::{set_file_times, FileTime};
use std::error::Error; use std::error::Error;
use std::fmt::{Debug, Display}; use std::fmt::{Debug, Display};
use std::fs; use std::fs;
#[cfg(not(unix))]
use std::fs::File; use std::fs::File;
use std::os::unix::fs::MetadataExt; use std::os::unix::fs::MetadataExt;
#[cfg(unix)] #[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
#[cfg(unix)]
use std::os::unix::prelude::OsStrExt; use std::os::unix::prelude::OsStrExt;
use std::path::{Path, PathBuf, MAIN_SEPARATOR}; use std::path::{Path, PathBuf, MAIN_SEPARATOR};
use std::process; use std::process;
@ -753,52 +750,27 @@ fn perform_backup(to: &Path, b: &Behavior) -> UResult<Option<PathBuf>> {
fn copy_file(from: &Path, to: &Path) -> UResult<()> { fn copy_file(from: &Path, to: &Path) -> UResult<()> {
// fs::copy fails if destination is a invalid symlink. // fs::copy fails if destination is a invalid symlink.
// so lets just remove all existing files at destination before copy. // so lets just remove all existing files at destination before copy.
let remove_destination = || { if let Err(e) = fs::remove_file(to) {
if let Err(e) = fs::remove_file(to) { if e.kind() != std::io::ErrorKind::NotFound {
if e.kind() != std::io::ErrorKind::NotFound { show_error!(
show_error!( "Failed to remove existing file {}. Error: {:?}",
"Failed to remove existing file {}. Error: {:?}", to.display(),
to.display(), e
e );
);
}
} }
};
remove_destination();
// create the destination file first. Using safer mode on unix to avoid
// potential unsafe mode between time-of-creation and time-of-chmod.
#[cfg(unix)]
let creation = fs::OpenOptions::new()
.write(true)
.create_new(true)
.mode(0o600)
.open(to);
#[cfg(not(unix))]
let creation = File::create(to);
if let Err(e) = creation {
show_error!(
"Failed to create destination file {}. Error: {:?}",
to.display(),
e
);
return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), e).into());
} }
// drop the file to close the fd of creation if from.as_os_str() == "/dev/null" {
drop(creation); /* workaround a limitation of fs::copy
* https://github.com/rust-lang/rust/issues/79390
/* workaround a limitation of fs::copy: skip copy if source is /dev/null */
* https://github.com/rust-lang/rust/issues/79390 if let Err(err) = File::create(to) {
*/
if from.as_os_str() != "/dev/null" {
if let Err(err) = fs::copy(from, to) {
remove_destination();
return Err( return Err(
InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(), InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
); );
} }
} else if let Err(err) = fs::copy(from, to) {
return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
} }
Ok(()) Ok(())
} }

View file

@ -1717,41 +1717,3 @@ fn test_install_root_combined() {
run_and_check(&["-Cv", "c", "d"], "d", 0, 0); run_and_check(&["-Cv", "c", "d"], "d", 0, 0);
run_and_check(&["-Cv", "c", "d"], "d", 0, 0); run_and_check(&["-Cv", "c", "d"], "d", 0, 0);
} }
#[cfg(all(unix, feature = "chmod"))]
#[test]
fn test_install_copy_failures() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
let file1 = "source_file";
let file2 = "target_file";
at.touch(file1);
scene.ccmd("chmod").arg("000").arg(file1).succeeds();
// if source file is not readable, it will raise a permission error.
// since we create the file with mode 0600 before `fs::copy`, if the
// copy failed, the file should be removed.
scene
.ucmd()
.arg(file1)
.arg(file2)
.arg("--mode=400")
.fails()
.stderr_contains("permission denied");
assert!(!at.file_exists(file2));
// if source file is good to copy, it should succeed and set the
// destination file permissions accordingly
scene.ccmd("chmod").arg("644").arg(file1).succeeds();
scene
.ucmd()
.arg(file1)
.arg(file2)
.arg("--mode=400")
.succeeds();
assert!(at.file_exists(file2));
assert_eq!(0o100_400_u32, at.metadata(file2).permissions().mode());
}