mirror of
https://github.com/uutils/coreutils
synced 2025-01-05 17:59:00 +00:00
Merge pull request #6922 from uutils/revert-6595-install_copy_permission
Revert "install: create destination file with safer modes before copy"
This commit is contained in:
commit
d6af68a3a1
2 changed files with 14 additions and 80 deletions
|
@ -13,12 +13,9 @@ use filetime::{set_file_times, FileTime};
|
|||
use std::error::Error;
|
||||
use std::fmt::{Debug, Display};
|
||||
use std::fs;
|
||||
#[cfg(not(unix))]
|
||||
use std::fs::File;
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::prelude::OsStrExt;
|
||||
use std::path::{Path, PathBuf, MAIN_SEPARATOR};
|
||||
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<()> {
|
||||
// fs::copy fails if destination is a invalid symlink.
|
||||
// so lets just remove all existing files at destination before copy.
|
||||
let remove_destination = || {
|
||||
if let Err(e) = fs::remove_file(to) {
|
||||
if e.kind() != std::io::ErrorKind::NotFound {
|
||||
show_error!(
|
||||
"Failed to remove existing file {}. Error: {:?}",
|
||||
to.display(),
|
||||
e
|
||||
);
|
||||
}
|
||||
if let Err(e) = fs::remove_file(to) {
|
||||
if e.kind() != std::io::ErrorKind::NotFound {
|
||||
show_error!(
|
||||
"Failed to remove existing file {}. Error: {:?}",
|
||||
to.display(),
|
||||
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
|
||||
drop(creation);
|
||||
|
||||
/* workaround a limitation of fs::copy: skip copy if source is /dev/null
|
||||
* https://github.com/rust-lang/rust/issues/79390
|
||||
*/
|
||||
if from.as_os_str() != "/dev/null" {
|
||||
if let Err(err) = fs::copy(from, to) {
|
||||
remove_destination();
|
||||
if from.as_os_str() == "/dev/null" {
|
||||
/* workaround a limitation of fs::copy
|
||||
* https://github.com/rust-lang/rust/issues/79390
|
||||
*/
|
||||
if let Err(err) = File::create(to) {
|
||||
return Err(
|
||||
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(())
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
||||
#[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());
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue