diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index 9b1d0a8da..69c72d17d 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -59,6 +59,7 @@ kibibytes libacl lcase lossily +lstat mebi mebibytes mergeable diff --git a/Cargo.lock b/Cargo.lock index ee64670a4..db5d22078 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3212,6 +3212,7 @@ dependencies = [ "walkdir", "wild", "winapi 0.3.9", + "winapi-util", "z85", ] diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index af84890db..f647906ba 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -20,6 +20,7 @@ use std::io::{self, Read, Write}; use thiserror::Error; use uucore::display::Quotable; use uucore::error::UResult; +use uucore::fs::FileInformation; #[cfg(unix)] use std::os::unix::io::AsRawFd; @@ -317,8 +318,7 @@ fn cat_path( path: &str, options: &OutputOptions, state: &mut OutputState, - #[cfg(unix)] out_info: &nix::sys::stat::FileStat, - #[cfg(windows)] out_info: &winapi_util::file::Information, + out_info: Option<&FileInformation>, ) -> CatResult<()> { if path == "-" { let stdin = io::stdin(); @@ -342,10 +342,15 @@ fn cat_path( } _ => { let file = File::open(path)?; - #[cfg(any(windows, unix))] - if same_file(out_info, &file) { - return Err(CatError::OutputIsInput); + + if let Some(out_info) = out_info { + if out_info.file_size() != 0 + && FileInformation::from_file(&file).as_ref() == Some(out_info) + { + return Err(CatError::OutputIsInput); + } } + let mut handle = InputHandle { reader: file, is_interactive: false, @@ -355,25 +360,8 @@ fn cat_path( } } -#[cfg(unix)] -fn same_file(a_info: &nix::sys::stat::FileStat, b: &File) -> bool { - let b_info = nix::sys::stat::fstat(b.as_raw_fd()).unwrap(); - b_info.st_size != 0 && b_info.st_dev == a_info.st_dev && b_info.st_ino == a_info.st_ino -} - -#[cfg(windows)] -fn same_file(a_info: &winapi_util::file::Information, b: &File) -> bool { - let b_info = winapi_util::file::information(b).unwrap(); - b_info.file_size() != 0 - && b_info.volume_serial_number() == a_info.volume_serial_number() - && b_info.file_index() == a_info.file_index() -} - fn cat_files(files: Vec, options: &OutputOptions) -> UResult<()> { - #[cfg(windows)] - let out_info = winapi_util::file::information(&std::io::stdout()).unwrap(); - #[cfg(unix)] - let out_info = nix::sys::stat::fstat(std::io::stdout().as_raw_fd()).unwrap(); + let out_info = FileInformation::from_file(&std::io::stdout()); let mut state = OutputState { line_number: 1, @@ -384,7 +372,7 @@ fn cat_files(files: Vec, options: &OutputOptions) -> UResult<()> { let mut error_messages: Vec = Vec::new(); for path in &files { - if let Err(err) = cat_path(path, options, &mut state, &out_info) { + if let Err(err) = cat_path(path, options, &mut state, out_info.as_ref()) { error_messages.push(format!("{}: {}", path.maybe_quote(), err)); } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e98ced717..4f4f10186 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -8,7 +8,7 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -// spell-checker:ignore (ToDO) ficlone linkgs lstat nlink nlinks pathbuf reflink strs xattrs +// spell-checker:ignore (ToDO) ficlone linkgs lstat nlink nlinks pathbuf reflink strs xattrs symlinked #[cfg(target_os = "linux")] #[macro_use] @@ -19,6 +19,7 @@ extern crate quick_error; extern crate uucore; use uucore::display::Quotable; +use uucore::fs::FileInformation; #[cfg(windows)] use winapi::um::fileapi::CreateFileW; #[cfg(windows)] @@ -838,8 +839,10 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu let mut non_fatal_errors = false; let mut seen_sources = HashSet::with_capacity(sources.len()); + let mut symlinked_files = HashSet::new(); for source in sources { if seen_sources.contains(source) { + // FIXME: compare sources by the actual file they point to, not their path. (e.g. dir/file == dir/../dir/file in most cases) show_warning!("source {} specified more than once", source.quote()); } else { let mut found_hard_link = false; @@ -848,7 +851,9 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu preserve_hardlinks(&mut hard_links, source, dest, &mut found_hard_link).unwrap(); } if !found_hard_link { - if let Err(error) = copy_source(source, target, &target_type, options) { + if let Err(error) = + copy_source(source, target, &target_type, options, &mut symlinked_files) + { match error { // When using --no-clobber, we don't want to show // an error message @@ -909,15 +914,16 @@ fn copy_source( target: &TargetSlice, target_type: &TargetType, options: &Options, + symlinked_files: &mut HashSet, ) -> CopyResult<()> { let source_path = Path::new(&source); if source_path.is_dir() { // Copy as directory - copy_directory(source, target, options) + copy_directory(source, target, options, symlinked_files) } else { // Copy as file let dest = construct_dest_path(source_path, target, target_type, options)?; - copy_file(source_path, dest.as_path(), options) + copy_file(source_path, dest.as_path(), options, symlinked_files) } } @@ -947,14 +953,19 @@ fn adjust_canonicalization(p: &Path) -> Cow { /// /// Any errors encountered copying files in the tree will be logged but /// will not cause a short-circuit. -fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyResult<()> { +fn copy_directory( + root: &Path, + target: &TargetSlice, + options: &Options, + symlinked_files: &mut HashSet, +) -> CopyResult<()> { if !options.recursive { return Err(format!("omitting directory {}", root.quote()).into()); } // if no-dereference is enabled and this is a symlink, copy it as a file if !options.dereference && fs::symlink_metadata(root).unwrap().file_type().is_symlink() { - return copy_file(root, target, options); + return copy_file(root, target, options, symlinked_files); } let current_dir = @@ -1011,7 +1022,7 @@ fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyR let local_to_target = target.join(&local_to_root_parent); if is_symlink && !options.dereference { - copy_link(&path, &local_to_target)?; + copy_link(&path, &local_to_target, symlinked_files)?; } else if path.is_dir() && !local_to_target.exists() { or_continue!(fs::create_dir_all(local_to_target)); } else if !path.is_dir() { @@ -1021,7 +1032,12 @@ fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyR let dest = local_to_target.as_path().to_path_buf(); preserve_hardlinks(&mut hard_links, &source, dest, &mut found_hard_link).unwrap(); if !found_hard_link { - match copy_file(path.as_path(), local_to_target.as_path(), options) { + match copy_file( + path.as_path(), + local_to_target.as_path(), + options, + symlinked_files, + ) { Ok(_) => Ok(()), Err(err) => { if fs::symlink_metadata(&source)?.file_type().is_symlink() { @@ -1036,7 +1052,12 @@ fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyR }?; } } else { - copy_file(path.as_path(), local_to_target.as_path(), options)?; + copy_file( + path.as_path(), + local_to_target.as_path(), + options, + symlinked_files, + )?; } } } @@ -1145,18 +1166,24 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu Ok(()) } -#[cfg(not(windows))] -#[allow(clippy::unnecessary_wraps)] // needed for windows version -fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> { - match std::os::unix::fs::symlink(source, dest).context(context) { - Ok(_) => Ok(()), - Err(_) => Ok(()), +fn symlink_file( + source: &Path, + dest: &Path, + context: &str, + symlinked_files: &mut HashSet, +) -> CopyResult<()> { + #[cfg(not(windows))] + { + std::os::unix::fs::symlink(source, dest).context(context)?; } -} - -#[cfg(windows)] -fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> { - Ok(std::os::windows::fs::symlink_file(source, dest).context(context)?) + #[cfg(windows)] + { + std::os::windows::fs::symlink_file(source, dest).context(context)?; + } + if let Some(file_info) = FileInformation::from_path(dest, false) { + symlinked_files.insert(file_info); + } + Ok(()) } fn context_for(src: &Path, dest: &Path) -> String { @@ -1183,6 +1210,7 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe } match options.overwrite { + // FIXME: print that the file was removed if --verbose is enabled OverwriteMode::Clobber(ClobberMode::Force) => { if fs::metadata(dest)?.permissions().readonly() { fs::remove_file(dest)?; @@ -1206,11 +1234,39 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe /// /// The original permissions of `source` will be copied to `dest` /// after a successful copy. -fn copy_file(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { +fn copy_file( + source: &Path, + dest: &Path, + options: &Options, + symlinked_files: &mut HashSet, +) -> CopyResult<()> { if dest.exists() { handle_existing_dest(source, dest, options)?; } + // Fail if dest is a dangling symlink or a symlink this program created previously + if fs::symlink_metadata(dest) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + if FileInformation::from_path(dest, false) + .map(|info| symlinked_files.contains(&info)) + .unwrap_or(false) + { + return Err(Error::Error(format!( + "will not copy '{}' through just-created symlink '{}'", + source.display(), + dest.display() + ))); + } + if !dest.exists() { + return Err(Error::Error(format!( + "not writing through dangling symlink '{}'", + dest.display() + ))); + } + } + if options.verbose { println!("{}", context_for(source, dest)); } @@ -1255,10 +1311,10 @@ fn copy_file(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { fs::hard_link(&source, &dest).context(context)?; } CopyMode::Copy => { - copy_helper(&source, &dest, options, context)?; + copy_helper(&source, &dest, options, context, symlinked_files)?; } CopyMode::SymLink => { - symlink_file(&source, &dest, context)?; + symlink_file(&source, &dest, context, symlinked_files)?; } CopyMode::Sparse => return Err(Error::NotImplemented(options::SPARSE.to_string())), CopyMode::Update => { @@ -1271,10 +1327,10 @@ fn copy_file(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { if src_time <= dest_time { return Ok(()); } else { - copy_helper(&source, &dest, options, context)?; + copy_helper(&source, &dest, options, context, symlinked_files)?; } } else { - copy_helper(&source, &dest, options, context)?; + copy_helper(&source, &dest, options, context, symlinked_files)?; } } CopyMode::AttrOnly => { @@ -1308,7 +1364,13 @@ fn copy_file(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { /// Copy the file from `source` to `dest` either using the normal `fs::copy` or a /// copy-on-write scheme if --reflink is specified and the filesystem supports it. -fn copy_helper(source: &Path, dest: &Path, options: &Options, context: &str) -> CopyResult<()> { +fn copy_helper( + source: &Path, + dest: &Path, + options: &Options, + context: &str, + symlinked_files: &mut HashSet, +) -> CopyResult<()> { if options.parents { let parent = dest.parent().unwrap_or(dest); fs::create_dir_all(parent)?; @@ -1320,7 +1382,7 @@ fn copy_helper(source: &Path, dest: &Path, options: &Options, context: &str) -> */ File::create(dest).context(dest.display().to_string())?; } else if is_symlink { - copy_link(source, dest)?; + copy_link(source, dest, symlinked_files)?; } else if options.reflink_mode != ReflinkMode::Never { #[cfg(not(any(target_os = "linux", target_os = "macos")))] return Err("--reflink is only supported on linux and macOS" @@ -1338,7 +1400,11 @@ fn copy_helper(source: &Path, dest: &Path, options: &Options, context: &str) -> Ok(()) } -fn copy_link(source: &Path, dest: &Path) -> CopyResult<()> { +fn copy_link( + source: &Path, + dest: &Path, + symlinked_files: &mut HashSet, +) -> CopyResult<()> { // Here, we will copy the symlink itself (actually, just recreate it) let link = fs::read_link(&source)?; let dest: Cow<'_, Path> = if dest.is_dir() { @@ -1358,7 +1424,7 @@ fn copy_link(source: &Path, dest: &Path) -> CopyResult<()> { } dest.into() }; - symlink_file(&link, &dest, &*context_for(&link, &dest)) + symlink_file(&link, &dest, &*context_for(&link, &dest), symlinked_files) } /// Copies `source` to `dest` using copy-on-write if possible. diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index f4b66e799..ece988aed 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -41,6 +41,7 @@ lazy_static = "1.3" [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3", features = ["errhandlingapi", "fileapi", "handleapi", "winerror"] } +winapi-util = { version= "0.1.5", optional=true } [target.'cfg(target_os = "redox")'.dependencies] termion = "1.5" @@ -50,7 +51,7 @@ default = [] # * non-default features encoding = ["data-encoding", "data-encoding-macro", "z85", "thiserror"] entries = ["libc"] -fs = ["libc"] +fs = ["libc", "nix", "winapi-util"] fsext = ["libc", "time"] mode = ["libc"] perms = ["libc", "walkdir"] diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 0b8079a5c..ef3dd6adf 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -17,12 +17,15 @@ use libc::{ use std::borrow::Cow; use std::env; use std::fs; +use std::hash::Hash; use std::io::Error as IOError; use std::io::Result as IOResult; use std::io::{Error, ErrorKind}; -#[cfg(any(unix, target_os = "redox"))] -use std::os::unix::fs::MetadataExt; +#[cfg(unix)] +use std::os::unix::{fs::MetadataExt, io::AsRawFd}; use std::path::{Component, Path, PathBuf}; +#[cfg(target_os = "windows")] +use winapi_util::AsHandleRef; #[cfg(unix)] #[macro_export] @@ -32,6 +35,114 @@ macro_rules! has { }; } +/// Information to uniquely identify a file +pub struct FileInformation( + #[cfg(unix)] nix::sys::stat::FileStat, + #[cfg(windows)] winapi_util::file::Information, +); + +impl FileInformation { + /// Get information from a currently open file + #[cfg(unix)] + pub fn from_file(file: &impl AsRawFd) -> Option { + if let Ok(x) = nix::sys::stat::fstat(file.as_raw_fd()) { + Some(Self(x)) + } else { + None + } + } + + /// Get information from a currently open file + #[cfg(target_os = "windows")] + pub fn from_file(file: &impl AsHandleRef) -> Option { + if let Ok(x) = winapi_util::file::information(file.as_handle_ref()) { + Some(Self(x)) + } else { + None + } + } + + /// Get information for a given path. + /// + /// If `path` points to a symlink and `dereference` is true, information about + /// the link's target will be returned. + pub fn from_path(path: impl AsRef, dereference: bool) -> Option { + #[cfg(unix)] + { + let stat = if dereference { + nix::sys::stat::stat(path.as_ref()) + } else { + nix::sys::stat::lstat(path.as_ref()) + }; + if let Ok(stat) = stat { + Some(Self(stat)) + } else { + None + } + } + #[cfg(target_os = "windows")] + { + use std::fs::OpenOptions; + use std::os::windows::prelude::*; + let mut open_options = OpenOptions::new(); + if !dereference { + open_options.custom_flags(winapi::um::winbase::FILE_FLAG_OPEN_REPARSE_POINT); + } + open_options + .read(true) + .open(path.as_ref()) + .ok() + .and_then(|file| Self::from_file(&file)) + } + } + + pub fn file_size(&self) -> u64 { + #[cfg(unix)] + { + use std::convert::TryInto; + + assert!(self.0.st_size >= 0, "File size is negative"); + self.0.st_size.try_into().unwrap() + } + #[cfg(target_os = "windows")] + { + self.0.file_size() + } + } +} + +#[cfg(unix)] +impl PartialEq for FileInformation { + fn eq(&self, other: &Self) -> bool { + self.0.st_dev == other.0.st_dev && self.0.st_ino == other.0.st_ino + } +} + +#[cfg(target_os = "windows")] +impl PartialEq for FileInformation { + fn eq(&self, other: &Self) -> bool { + self.0.volume_serial_number() == other.0.volume_serial_number() + && self.0.file_index() == other.0.file_index() + } +} + +impl Eq for FileInformation {} + +impl Hash for FileInformation { + fn hash(&self, state: &mut H) { + #[cfg(unix)] + { + self.0.st_dev.hash(state); + self.0.st_ino.hash(state); + } + #[cfg(target_os = "windows")] + { + self.0.volume_serial_number().hash(state); + self.0.file_index().hash(state); + } + } +} + pub fn resolve_relative_path(path: &Path) -> Cow { if path.components().all(|e| e != Component::ParentDir) { return path.into(); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 65d89b163..b2a6eede5 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1379,3 +1379,42 @@ fn test_canonicalize_symlink() { .no_stderr() .no_stdout(); } + +#[test] +fn test_copy_through_just_created_symlink() { + for &create_t in &[true, false] { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + #[cfg(unix)] + fs::symlink("../t", at.plus("a/1")).unwrap(); + #[cfg(target_os = "windows")] + symlink_file("../t", at.plus("a/1")).unwrap(); + at.touch("b/1"); + if create_t { + at.touch("t"); + } + ucmd.arg("--no-dereference") + .arg("a/1") + .arg("b/1") + .arg("c") + .fails() + .stderr_only(if cfg!(not(target_os = "windows")) { + "cp: will not copy 'b/1' through just-created symlink 'c/1'" + } else { + "cp: will not copy 'b/1' through just-created symlink 'c\\1'" + }); + } +} + +#[test] +fn test_copy_through_dangling_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + at.symlink_file("nonexistent", "target"); + ucmd.arg("file") + .arg("target") + .fails() + .stderr_only("cp: not writing through dangling symlink 'target'"); +}