From 9d285e953dedb43d731ff9240680108057eb4247 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Sun, 10 Jul 2022 17:49:25 +0300 Subject: [PATCH] Realpath symlinks handling, solves issue #3669 (#3703) --- src/uu/cat/src/cat.rs | 4 +- src/uu/cp/src/cp.rs | 102 ++++-------- src/uucore/src/lib/features/fs.rs | 254 ++++++++++++++---------------- tests/by-util/test_cp.rs | 69 +++++++- tests/by-util/test_realpath.rs | 18 ++- tests/common/util.rs | 14 +- 6 files changed, 243 insertions(+), 218 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index cd10b5251..5e9f76c97 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -351,7 +351,7 @@ fn cat_path( if let Some(out_info) = out_info { if out_info.file_size() != 0 - && FileInformation::from_file(&file).as_ref() == Some(out_info) + && FileInformation::from_file(&file).ok().as_ref() == Some(out_info) { return Err(CatError::OutputIsInput); } @@ -367,7 +367,7 @@ fn cat_path( } fn cat_files(files: &[String], options: &OutputOptions) -> UResult<()> { - let out_info = FileInformation::from_file(&std::io::stdout()); + let out_info = FileInformation::from_file(&std::io::stdout()).ok(); let mut state = OutputState { line_number: 1, diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 9daf35efd..37cbf8f5d 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -19,10 +19,6 @@ extern crate uucore; use uucore::display::Quotable; use uucore::format_usage; use uucore::fs::FileInformation; -#[cfg(windows)] -use winapi::um::fileapi::CreateFileW; -#[cfg(windows)] -use winapi::um::fileapi::GetFileInformationByHandle; use std::borrow::Cow; @@ -35,22 +31,17 @@ use std::collections::HashSet; use std::env; #[cfg(not(windows))] use std::ffi::CString; -#[cfg(windows)] -use std::ffi::OsStr; use std::fs; use std::fs::File; use std::fs::OpenOptions; use std::io; use std::io::{stdin, stdout, Write}; -use std::mem; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; #[cfg(unix)] use std::os::unix::fs::{FileTypeExt, PermissionsExt}; #[cfg(any(target_os = "linux", target_os = "android"))] use std::os::unix::io::AsRawFd; -#[cfg(windows)] -use std::os::windows::ffi::OsStrExt; use std::path::{Path, PathBuf, StripPrefixError}; use std::str::FromStr; use std::string::ToString; @@ -793,60 +784,30 @@ fn preserve_hardlinks( #[cfg(not(target_os = "redox"))] { if !source.is_dir() { - unsafe { - let inode: u64; - let nlinks: u64; - #[cfg(unix)] - { - let src_path = CString::new(source.as_os_str().to_str().unwrap()).unwrap(); - let mut stat = mem::zeroed(); - if libc::lstat(src_path.as_ptr(), &mut stat) < 0 { - return Err(format!( - "cannot stat {}: {}", - source.quote(), - std::io::Error::last_os_error() - ) - .into()); - } - inode = stat.st_ino as u64; - nlinks = stat.st_nlink as u64; - } - #[cfg(windows)] - { - let src_path: Vec = OsStr::new(source).encode_wide().collect(); - #[allow(deprecated)] - let stat = mem::uninitialized(); - let handle = CreateFileW( - src_path.as_ptr(), - winapi::um::winnt::GENERIC_READ, - winapi::um::winnt::FILE_SHARE_READ, - std::ptr::null_mut(), - 0, - 0, - std::ptr::null_mut(), - ); - if GetFileInformationByHandle(handle, stat) != 0 { - return Err(format!( - "cannot get file information {:?}: {}", - source, - std::io::Error::last_os_error() - ) - .into()); - } - inode = ((*stat).nFileIndexHigh as u64) << 32 | (*stat).nFileIndexLow as u64; - nlinks = (*stat).nNumberOfLinks as u64; + let info = match FileInformation::from_path(source, false) { + Ok(info) => info, + Err(e) => { + return Err(format!("cannot stat {}: {}", source.quote(), e,).into()); } + }; - for hard_link in hard_links.iter() { - if hard_link.1 == inode { - std::fs::hard_link(hard_link.0.clone(), dest).unwrap(); - *found_hard_link = true; - } - } - if !(*found_hard_link) && nlinks > 1 { - hard_links.push((dest.to_str().unwrap().to_string(), inode)); + #[cfg(unix)] + let inode = info.inode(); + + #[cfg(windows)] + let inode = info.file_index(); + + let nlinks = info.number_of_links(); + + for hard_link in hard_links.iter() { + if hard_link.1 == inode { + std::fs::hard_link(hard_link.0.clone(), dest).unwrap(); + *found_hard_link = true; } } + if !(*found_hard_link) && nlinks > 1 { + hard_links.push((dest.to_str().unwrap().to_string(), inode)); + } } } Ok(()) @@ -1227,7 +1188,7 @@ fn symlink_file( { std::os::windows::fs::symlink_file(source, dest).context(context)?; } - if let Some(file_info) = FileInformation::from_path(dest, false) { + if let Ok(file_info) = FileInformation::from_path(dest, false) { symlinked_files.insert(file_info); } Ok(()) @@ -1245,7 +1206,8 @@ fn backup_dest(dest: &Path, backup_path: &Path) -> CopyResult { } fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { - if paths_refer_to_same_file(source, dest)? { + let dereference_to_compare = options.dereference || !is_symlink(source); + if paths_refer_to_same_file(source, dest, dereference_to_compare) { return Err(format!("{}: same file", context_for(source, dest)).into()); } @@ -1253,7 +1215,7 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe let backup_path = backup_control::get_backup_path(options.backup, dest, &options.backup_suffix); if let Some(backup_path) = backup_path { - if paths_refer_to_same_file(source, &backup_path)? { + if paths_refer_to_same_file(source, &backup_path, true) { return Err(format!( "backing up {} might destroy source; {} not copied", dest.quote(), @@ -1313,7 +1275,8 @@ fn copy_file( dest.display() ))); } - if options.dereference && !dest.exists() { + let copy_contents = options.dereference || !is_symlink(source); + if copy_contents && !dest.exists() { return Err(Error::Error(format!( "not writing through dangling symlink '{}'", dest.display() @@ -1542,7 +1505,7 @@ fn copy_link( } else { // we always need to remove the file to be able to create a symlink, // even if it is writeable. - if dest.is_file() { + if is_symlink(dest) || dest.is_file() { fs::remove_file(dest)?; } dest.into() @@ -1685,12 +1648,15 @@ pub fn localize_to_target(root: &Path, source: &Path, target: &Path) -> CopyResu Ok(target.join(&local_to_root)) } -pub fn paths_refer_to_same_file(p1: &Path, p2: &Path) -> io::Result { +pub fn paths_refer_to_same_file(p1: &Path, p2: &Path, dereference: bool) -> bool { // We have to take symlinks and relative paths into account. - let pathbuf1 = canonicalize(p1, MissingHandling::Normal, ResolveMode::Logical)?; - let pathbuf2 = canonicalize(p2, MissingHandling::Normal, ResolveMode::Logical)?; + let res1 = FileInformation::from_path(p1, dereference); + let res2 = FileInformation::from_path(p2, dereference); - Ok(pathbuf1 == pathbuf2) + match (res1, res2) { + (Ok(info1), Ok(info2)) => info1 == info2, + _ => false, + } } pub fn path_has_prefix(p1: &Path, p2: &Path) -> io::Result { diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index fe8726575..d1b48a127 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -17,12 +17,14 @@ use libc::{ S_IXUSR, }; use std::borrow::Cow; +#[cfg(unix)] +use std::collections::HashSet; +use std::collections::VecDeque; use std::env; +use std::ffi::{OsStr, OsString}; use std::fs; use std::hash::Hash; -use std::io::Error as IOError; -use std::io::Result as IOResult; -use std::io::{Error, ErrorKind}; +use std::io::{Error, ErrorKind, Result as IOResult}; #[cfg(unix)] use std::os::unix::{fs::MetadataExt, io::AsRawFd}; use std::path::{Component, Path, PathBuf}; @@ -46,29 +48,23 @@ pub struct FileInformation( 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 - } + pub fn from_file(file: &impl AsRawFd) -> IOResult { + let stat = nix::sys::stat::fstat(file.as_raw_fd())?; + Ok(Self(stat)) } /// 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 - } + pub fn from_file(file: &impl AsHandleRef) -> IOResult { + let info = winapi_util::file::information(file.as_handle_ref())?; + Ok(Self(info)) } /// 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 { + pub fn from_path(path: impl AsRef, dereference: bool) -> IOResult { #[cfg(unix)] { let stat = if dereference { @@ -76,11 +72,7 @@ impl FileInformation { } else { nix::sys::stat::lstat(path.as_ref()) }; - if let Ok(stat) = stat { - Some(Self(stat)) - } else { - None - } + Ok(Self(stat?)) } #[cfg(target_os = "windows")] { @@ -90,11 +82,8 @@ impl FileInformation { 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)) + let file = open_options.read(true).open(path.as_ref())?; + Self::from_file(&file) } } @@ -109,6 +98,23 @@ impl FileInformation { self.0.file_size() } } + + #[cfg(windows)] + pub fn file_index(&self) -> u64 { + self.0.file_index() + } + + pub fn number_of_links(&self) -> u64 { + #[cfg(unix)] + return self.0.st_nlink as u64; + #[cfg(windows)] + return self.0.number_of_links() as u64; + } + + #[cfg(unix)] + pub fn inode(&self) -> u64 { + self.0.st_ino as u64 + } } #[cfg(unix)] @@ -233,47 +239,45 @@ pub fn is_symlink>(path: P) -> bool { fs::symlink_metadata(path).map_or(false, |m| m.file_type().is_symlink()) } -fn resolve>(original: P) -> Result { - const MAX_LINKS_FOLLOWED: u32 = 255; - let mut followed = 0; - let mut result = original.as_ref().to_path_buf(); - let mut symlink_is_absolute = false; - let mut first_resolution = None; +fn resolve_symlink>(path: P) -> IOResult> { + let result = if fs::symlink_metadata(&path)?.file_type().is_symlink() { + Some(fs::read_link(&path)?) + } else { + None + }; + Ok(result) +} - loop { - if followed == MAX_LINKS_FOLLOWED { - return Err(( - symlink_is_absolute, - // When we hit MAX_LINKS_FOLLOWED we should return the first resolution (that's what GNU does - for whatever reason) - first_resolution.unwrap(), - Error::new(ErrorKind::InvalidInput, "maximum links followed"), - )); - } +enum OwningComponent { + Prefix(OsString), + RootDir, + CurDir, + ParentDir, + Normal(OsString), +} - match fs::symlink_metadata(&result) { - Ok(meta) => { - if !meta.file_type().is_symlink() { - break; - } - } - Err(e) => return Err((symlink_is_absolute, result, e)), - } - - followed += 1; - match fs::read_link(&result) { - Ok(path) => { - result.pop(); - symlink_is_absolute = path.is_absolute(); - result.push(path); - } - Err(e) => return Err((symlink_is_absolute, result, e)), - } - - if first_resolution.is_none() { - first_resolution = Some(result.clone()); +impl OwningComponent { + fn as_os_str(&self) -> &OsStr { + match self { + Self::Prefix(s) => s.as_os_str(), + Self::RootDir => Component::RootDir.as_os_str(), + Self::CurDir => Component::CurDir.as_os_str(), + Self::ParentDir => Component::ParentDir.as_os_str(), + Self::Normal(s) => s.as_os_str(), + } + } +} + +impl<'a> From> for OwningComponent { + fn from(comp: Component<'a>) -> Self { + match comp { + Component::Prefix(_) => Self::Prefix(comp.as_os_str().to_os_string()), + Component::RootDir => Self::RootDir, + Component::CurDir => Self::CurDir, + Component::ParentDir => Self::ParentDir, + Component::Normal(s) => Self::Normal(s.to_os_string()), } } - Ok(result) } /// Return the canonical, absolute form of a path. @@ -307,7 +311,7 @@ pub fn canonicalize>( miss_mode: MissingHandling, res_mode: ResolveMode, ) -> IOResult { - // Create an absolute path + const SYMLINKS_TO_LOOK_FOR_LOOPS: i32 = 256; let original = original.as_ref(); let original = if original.is_absolute() { original.to_path_buf() @@ -315,86 +319,72 @@ pub fn canonicalize>( let current_dir = env::current_dir()?; dunce::canonicalize(current_dir)?.join(original) }; - + let path = if res_mode == ResolveMode::Logical { + normalize_path(&original) + } else { + original + }; + let mut parts: VecDeque = path.components().map(|part| part.into()).collect(); let mut result = PathBuf::new(); - let mut parts = vec![]; - - // Split path by directory separator; add prefix (Windows-only) and root - // directory to final path buffer; add remaining parts to temporary - // vector for canonicalization. - for part in original.components() { + let mut followed_symlinks = 0; + #[cfg(unix)] + let mut visited_files = HashSet::new(); + while let Some(part) = parts.pop_front() { match part { - Component::Prefix(_) | Component::RootDir => { - result.push(part.as_os_str()); - } - Component::CurDir => (), - Component::ParentDir => { - if res_mode == ResolveMode::Logical { - parts.pop(); - } else { - parts.push(part.as_os_str()); - } - } - Component::Normal(_) => { - parts.push(part.as_os_str()); - } - } - } - - // Resolve the symlinks where possible - if !parts.is_empty() { - for part in parts[..parts.len() - 1].iter() { - result.push(part); - - //resolve as we go to handle long relative paths on windows - if res_mode == ResolveMode::Physical { - result = normalize_path(&result); - } - - if res_mode == ResolveMode::None { + OwningComponent::Prefix(s) => { + result.push(s); continue; } + OwningComponent::RootDir | OwningComponent::Normal(..) => { + result.push(part.as_os_str()); + } + OwningComponent::CurDir => {} + OwningComponent::ParentDir => { + result.pop(); + } + } + if res_mode == ResolveMode::None { + continue; + } + match resolve_symlink(&result) { + Ok(Some(link_path)) => { + for link_part in link_path.components().rev() { + parts.push_front(link_part.into()); + } + if followed_symlinks < SYMLINKS_TO_LOOK_FOR_LOOPS { + followed_symlinks += 1; + } else { + #[cfg(unix)] + let has_loop = { + let file_info = + FileInformation::from_path(&result.parent().unwrap(), false).unwrap(); + let mut path_to_follow = PathBuf::new(); + for part in &parts { + path_to_follow.push(part.as_os_str()); + } + !visited_files.insert((file_info, path_to_follow)) + }; - match resolve(&result) { - Err((_, path, e)) => { - if miss_mode == MissingHandling::Missing { - result = path; - } else { - return Err(e); + #[cfg(not(unix))] + let has_loop = true; + + if has_loop { + return Err(Error::new( + ErrorKind::InvalidInput, + "Too many levels of symbolic links", + )); // TODO use ErrorKind::FilesystemLoop when stable } } - Ok(path) => { - result = path; - } + result.pop(); } - } - - result.push(parts.last().unwrap()); - - if res_mode == ResolveMode::None { - return Ok(result); - } - - match resolve(&result) { - Err((is_absolute, path, err)) => { - // If the resolved symlink is an absolute path and non-existent, - // `realpath` throws no such file error. + Err(e) => { if miss_mode == MissingHandling::Existing - || (err.kind() == ErrorKind::NotFound - && is_absolute - && miss_mode == MissingHandling::Normal) + || (miss_mode == MissingHandling::Normal && !parts.is_empty()) { - return Err(err); - } else { - result = path; + return Err(e); } } - Ok(path) => { - result = path; - } - } - if res_mode == ResolveMode::Physical { - result = normalize_path(&result); + _ => {} } } Ok(result) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index c246d0ef9..5394e9eec 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1492,13 +1492,12 @@ fn test_copy_through_just_created_symlink() { 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.relative_symlink_file("../t", "a/1"); at.touch("b/1"); + at.write("b/1", "hello"); if create_t { at.touch("t"); + at.write("t", "world"); } ucmd.arg("--no-dereference") .arg("a/1") @@ -1510,6 +1509,9 @@ fn test_copy_through_just_created_symlink() { } else { "cp: will not copy 'b/1' through just-created symlink 'c\\1'" }); + if create_t { + assert_eq!(at.read("a/1"), "world"); + } } } @@ -1536,6 +1538,16 @@ fn test_copy_through_dangling_symlink_no_dereference() { .no_stdout(); } +#[test] +fn test_copy_through_dangling_symlink_no_dereference_2() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + at.symlink_file("nonexistent", "target"); + ucmd.args(&["-P", "file", "target"]) + .fails() + .stderr_only("cp: not writing through dangling symlink 'target'"); +} + #[test] #[cfg(unix)] fn test_cp_archive_on_nonexistent_file() { @@ -1658,3 +1670,52 @@ fn test_cp_overriding_arguments() { s.fixtures.remove("file2"); } } + +#[test] +fn test_copy_no_dereference_1() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkdir("b"); + at.touch("a/foo"); + at.write("a/foo", "bar"); + at.relative_symlink_file("../a/foo", "b/foo"); + ucmd.args(&["-P", "a/foo", "b"]).fails(); +} + +#[test] +fn test_abuse_existing() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.relative_symlink_file("../t", "a/1"); + at.touch("b/1"); + at.write("b/1", "hello"); + at.relative_symlink_file("../t", "c/1"); + at.touch("t"); + at.write("t", "i"); + ucmd.args(&["-dR", "a/1", "b/1", "c"]) + .fails() + .stderr_contains(format!( + "will not copy 'b/1' through just-created symlink 'c{}1'", + if cfg!(windows) { "\\" } else { "/" } + )); + assert_eq!(at.read("t"), "i"); +} + +#[test] +fn test_copy_same_symlink_no_dereference() { + let (at, mut ucmd) = at_and_ucmd!(); + at.relative_symlink_file("t", "a"); + at.relative_symlink_file("t", "b"); + at.touch("t"); + ucmd.args(&["-d", "a", "b"]).succeeds(); +} + +#[test] +fn test_copy_same_symlink_no_dereference_dangling() { + let (at, mut ucmd) = at_and_ucmd!(); + at.relative_symlink_file("t", "a"); + at.relative_symlink_file("t", "b"); + ucmd.args(&["-d", "a", "b"]).succeeds(); +} diff --git a/tests/by-util/test_realpath.rs b/tests/by-util/test_realpath.rs index 6e413ce9e..3318c214d 100644 --- a/tests/by-util/test_realpath.rs +++ b/tests/by-util/test_realpath.rs @@ -1,6 +1,6 @@ use crate::common::util::*; -use std::path::Path; +use std::path::{Path, MAIN_SEPARATOR}; static GIBBERISH: &str = "supercalifragilisticexpialidocious"; @@ -155,8 +155,8 @@ fn test_realpath_dangling() { let (at, mut ucmd) = at_and_ucmd!(); at.symlink_file("nonexistent-file", "link"); ucmd.arg("link") - .fails() - .stderr_contains("realpath: link: No such file or directory"); + .succeeds() + .stdout_contains(at.plus_as_string("nonexistent-file\n")); } #[test] @@ -166,8 +166,8 @@ fn test_realpath_loop() { at.symlink_file("3", "2"); at.symlink_file("1", "3"); ucmd.arg("1") - .succeeds() - .stdout_only(at.plus_as_string("2\n")); + .fails() + .stderr_contains("Too many levels of symbolic links"); } #[test] @@ -241,7 +241,6 @@ fn test_realpath_when_symlink_is_absolute_and_enoent() { } #[test] -#[ignore = "issue #3669"] fn test_realpath_when_symlink_part_is_missing() { let (at, mut ucmd) = at_and_ucmd!(); @@ -254,10 +253,13 @@ fn test_realpath_when_symlink_part_is_missing() { at.relative_symlink_file("../dir2/baz", "dir1/foo3"); at.symlink_file("dir3/bar", "dir1/foo4"); + let expect1 = format!("dir2{}bar", MAIN_SEPARATOR); + let expect2 = format!("dir2{}baz", MAIN_SEPARATOR); + ucmd.args(&["dir1/foo1", "dir1/foo2", "dir1/foo3", "dir1/foo4"]) .run() - .stdout_contains(at.plus_as_string("dir2/bar") + "\n") - .stdout_contains(at.plus_as_string("dir2/baz") + "\n") + .stdout_contains(expect1 + "\n") + .stdout_contains(expect2 + "\n") .stderr_contains("realpath: dir1/foo2: No such file or directory\n") .stderr_contains("realpath: dir1/foo4: No such file or directory\n"); } diff --git a/tests/common/util.rs b/tests/common/util.rs index 580dd1584..7fee86b3d 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -22,6 +22,8 @@ use std::io::{Read, Result, Write}; use std::os::unix::fs::{symlink as symlink_dir, symlink as symlink_file}; #[cfg(windows)] use std::os::windows::fs::{symlink_dir, symlink_file}; +#[cfg(windows)] +use std::path::MAIN_SEPARATOR; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::rc::Rc; @@ -702,11 +704,13 @@ impl AtPath { } pub fn relative_symlink_file(&self, original: &str, link: &str) { + #[cfg(windows)] + let original = original.replace('/', &MAIN_SEPARATOR.to_string()); log_info( "symlink", - &format!("{},{}", original, &self.plus_as_string(link)), + &format!("{},{}", &original, &self.plus_as_string(link)), ); - symlink_file(original, &self.plus(link)).unwrap(); + symlink_file(&original, &self.plus(link)).unwrap(); } pub fn symlink_dir(&self, original: &str, link: &str) { @@ -722,11 +726,13 @@ impl AtPath { } pub fn relative_symlink_dir(&self, original: &str, link: &str) { + #[cfg(windows)] + let original = original.replace('/', &MAIN_SEPARATOR.to_string()); log_info( "symlink", - &format!("{},{}", original, &self.plus_as_string(link)), + &format!("{},{}", &original, &self.plus_as_string(link)), ); - symlink_dir(original, &self.plus(link)).unwrap(); + symlink_dir(&original, &self.plus(link)).unwrap(); } pub fn is_symlink(&self, path: &str) -> bool {