diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 0702148957695..5bff8c9470621 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -3470,22 +3470,22 @@ pub fn set_permissions>(path: P, perm: Permissions) -> io::Result /// /// # Platform-specific behavior /// -/// This function currently corresponds to: -/// * `open` with `O_NOFOLLOW` flag enabled + `fchmod` on WASI -/// * `fchmodat` function with the flag `AT_SYMLINK_NOFOLLOW` enabled -/// on Unix platforms -/// * The flag `FILE_FLAG_OPEN_REPARSE_POINT` is enabled and then the -/// permissions of the file is set through `SetFileInformationByHandle` -/// on Windows. -/// * On all other platforms, the behavior remains the same with +/// This function currently corresponds to the following underlying operations: +/// * Linux, BSD-based platforms, Android, QNX, NTO: `fchmodat` with `AT_SYMLINK_NOFOLLOW`. +/// If that is not supported, we fall back to `open` with `O_NOFOLLOW` followed by /// [`fs::set_permissions`]. -/// -/// [`fs::set_permissions`]: crate::fs::set_permissions +/// * Other Unix-based platforms with symlinks: `open` with `O_NOFOLLOW` followed by +/// [`fs::set_permissions`]. +/// * Other Unix-based platforms without symlinks: `open` followed by [`fs::set_permissions`]. +/// * Windows: `CreateFileW` with `FILE_FLAG_OPEN_REPARSE_POINT` followed +/// by `SetFileInformationByHandle`. /// /// Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior /// +/// [`fs::set_permissions`]: crate::fs::set_permissions +/// /// # Errors /// /// This function will return an error in the following situations, but is not @@ -3494,10 +3494,8 @@ pub fn set_permissions>(path: P, perm: Permissions) -> io::Result /// * `path` does not exist. /// * The user lacks the permission to change attributes of the file. /// -/// Note: On Linux, this will result in a [`Unsupported`] error -/// if the final element is a symlink. On BSD-based systems, the -/// behavior can vary from symlink permission bits changing or -/// there being no effects on symlinks +/// Note: On Linux and other Unix-based platforms with symlinks (non-BSD-based), +/// this will result in an [`Unsupported`] error if the final element is a symlink. /// /// [`Unsupported`]: crate::io::ErrorKind::Unsupported /// @@ -3510,8 +3508,8 @@ pub fn set_permissions>(path: P, perm: Permissions) -> io::Result /// fn main() -> std::io::Result<()> { /// let mut perms = fs::symlink_metadata("foo.txt")?.permissions(); /// perms.set_readonly(true); -/// // This should result in an error on certain platforms -/// // or succeed in modifying the permissions of a symlink +/// // This should result in an error on certain platforms or +/// // succeed in modifying the permissions of a symlink /// fs::set_permissions_nofollow("foo.txt", perms)?; /// Ok(()) /// } diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 3a6c04146922a..320038f2b34b7 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -649,7 +649,10 @@ fn set_get_permissions_nofollows() { // Only Windows and Unix support `fs::set_permissions_nofollow` #[test] -#[cfg(all(any(windows, unix), not(any(target_os = "espidf", target_os = "horizon"))))] +#[cfg(all( + any(windows, unix), + not(any(target_os = "espidf", target_os = "horizon", target_os = "wasi")) +))] fn set_get_permissions_nofollows_symlink() { #[cfg(not(windows))] use crate::os::unix::fs::symlink as symlink_dir; @@ -670,23 +673,19 @@ fn set_get_permissions_nofollows_symlink() { cfg_select! { any( windows, - target_os = "android", target_os = "macos", target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", - target_os = "dragonfly" + target_os = "dragonfly", + target_os = "nto", + target_os = "qnx" ) => { assert_eq!(result.unwrap(), ()); let metadata0 = check!(fs::symlink_metadata(&symlink_name)); - // So seems like BSD-based systems trying to set permissions - // on symlinks could lead to no effect, so we should expect - // there being no change to BSD-based systems. + // On these systems, it's confirmed the symlink itself is marked readonly // https://superuser.com/questions/1099634/change-permissions-symbolic-link-mac-os - #[cfg(windows)] assert!(metadata0.permissions().readonly()); - #[cfg(not(windows))] - assert!(!metadata0.permissions().readonly()); // Reset the read-only bit under Windows 7: avoids the // `TempDir::drop` from crashing on a permission denial when diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 8b41a3792ac9a..e3d02af15a814 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -2377,7 +2377,7 @@ pub struct NormalizeError; impl Path { // The following (private!) function allows construction of a path from a u8 // slice, which is only safe when it is known to follow the OsStr encoding. - unsafe fn from_u8_slice(s: &[u8]) -> &Path { + pub(crate) unsafe fn from_u8_slice(s: &[u8]) -> &Path { unsafe { Path::new(OsStr::from_encoded_bytes_unchecked(s)) } } // The following (private!) function reveals the byte encoding used for OsStr. diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs index b33ebadebe4ad..69fe37fb86613 100644 --- a/library/std/src/sys/fs/unix.rs +++ b/library/std/src/sys/fs/unix.rs @@ -1885,30 +1885,78 @@ pub fn set_perm(p: &CStr, perm: FilePermissions) -> io::Result<()> { } pub fn set_perm_nofollow(p: &CStr, perm: FilePermissions) -> io::Result<()> { - // ESP-IDF and Horizon do not support O_NOFOLLOW, so we skip setting it. - // Their filesystems do not have symbolic links, so no special handling is required. - cfg_select! { - // wasm32-wasip1 targets do not support fchmodat, so we fall down to - // open + fchmod - target_os = "wasi" => { - use crate::fs::{OpenOptions, Permissions}; - use crate::os::wasi::ffi::OsStrExt; - use crate::os::wasi::fs::OpenOptionsExt; + #[inline] + /// Helper function for fallback open with `O_NOFOLLOW` + `fchmod` behavior + fn open_and_set_permissions(p: &CStr, perm: FilePermissions) -> io::Result<()> { + use crate::fs::{OpenOptions, Permissions}; - let mut options = OpenOptions::new(); - options.custom_flags(libc::O_NOFOLLOW); + let mut options = OpenOptions::new(); - let bytes = p.to_bytes(); - let os_str = OsStr::from_bytes(bytes); - options.open(Path::new(os_str))?.set_permissions(Permissions::from_inner(perm)) + // ESP-IDF and Horizon do not support O_NOFOLLOW, so we skip setting it. + // Their filesystems do not have symbolic links, so no special handling is required. + #[cfg(not(any(target_os = "espidf", target_os = "horizon")))] + { + #[cfg(not(target_os = "wasi"))] + use crate::os::unix::fs::OpenOptionsExt; + #[cfg(target_os = "wasi")] + use crate::os::wasi::fs::OpenOptionsExt; + options.read(true).custom_flags(libc::O_NOFOLLOW); } - all(target_os = "linux", not(any(target_os = "espidf", target_os = "horizon"))) => { - cvt_r(|| unsafe { - libc::fchmodat(libc::AT_FDCWD, p.as_ptr(), perm.mode, libc::AT_SYMLINK_NOFOLLOW) - }) - .map(|_| ()) + + #[cfg(not(target_os = "wasi"))] + use crate::os::unix::ffi::OsStrExt; + #[cfg(target_os = "wasi")] + use crate::os::wasi::ffi::OsStrExt; + + let os_str = OsStr::from_bytes(p.to_bytes()); + options.open(Path::new(os_str))?.set_permissions(Permissions::from_inner(perm)) + } + + let mut _res: Result<(), core::io::Error> = Err(crate::io::ErrorKind::Unsupported.into()); + + // These platforms support `fchmodat`, so utilize this syscall over `open` + `fchmod` + #[cfg(any( + target_os = "linux", + target_os = "macos", + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + target_os = "dragonfly", + target_os = "android", + target_os = "nto", + target_os = "qnx" + ))] + { + _res = cvt_r(|| unsafe { + libc::fchmodat(libc::AT_FDCWD, p.as_ptr(), perm.mode, libc::AT_SYMLINK_NOFOLLOW) + }) + .map(|_| ()); + } + + // If fchmodat fails with `ErrorKind::Unsupported` fallback to using open + fchmod. This is just in case + // for older systems like Ubuntu 20.04 where fchmodat fails with EOPNOTSUPP on both regular files and + // symlinks when AT_SYMLINK_NOFOLLOW is passed in. + match _res { + Ok(_) => Ok(()), + Err(err) => { + if err.kind() == crate::io::ErrorKind::Unsupported { + match open_and_set_permissions(p, perm) { + Ok(_) => return Ok(()), + Err(e) => { + if e.kind() == crate::io::ErrorKind::FilesystemLoop { + // When open is used with O_NOFOLLOW flag, if the trailing component of + // a path is a symbolic link, it should fail with ELOOP error. Instead of + // returning `FilesystemLoop`, this returns `Unsupported` to keep it consistent + // with what `fchmodat` would return when chmoding a symlink using AT_SYMLINK_NOFOLLOW. + return Err(err); + } + return Err(e); + } + } + } + + Err(err) } - _ => cvt_r(|| unsafe { libc::fchmodat(libc::AT_FDCWD, p.as_ptr(), perm.mode, 0) }).map(|_| ()), } }