diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 65b8ed634bc05..afc421ab82e86 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -3645,7 +3645,7 @@ pub fn set_permissions>(path: P, perm: Permissions) -> io::Result /// This function currently corresponds to the following underlying operations: /// * Android: returns [`Unsupported`] on all files. /// * Linux, BSD-based platforms, QNX, NTO: `fchmodat` with `AT_SYMLINK_NOFOLLOW`. -/// If that is not supported, we fall back to: +/// If that is not supported, we fall back to: /// * Unix-based platforms with symlinks: `open` with `O_NOFOLLOW` followed by /// [`fs::set_permissions`]. /// * Unix-based platforms without symlinks: `open` followed by [`fs::set_permissions`]. diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index db9a9f3f20b88..05ab05e922a32 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -613,12 +613,13 @@ fn set_get_unix_permissions() { assert_eq!(mask & metadata1.permissions().mode(), 0o0777); } -#[cfg(not(target_os = "android"))] +/// Test set_permissions_nofollow on a regular file. #[test] fn set_get_permissions_nofollows() { let tmpdir = tmpdir(); let filename = tmpdir.join("set_get_unix_permissions_file"); check!(File::create(&filename)); + let file_metadata = check!(fs::metadata(&filename)); assert!(!file_metadata.permissions().readonly()); let mut permission_bits = file_metadata.permissions(); @@ -648,12 +649,8 @@ fn set_get_permissions_nofollows() { } } -// Only Windows and Unix support `fs::set_permissions_nofollow` +/// Test set_permissions_nofollow on a symlink. #[test] -#[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_file; @@ -667,14 +664,20 @@ fn set_get_permissions_nofollows_symlink() { check!(symlink_file(&filename, &symlink_name)); let init_symlink_metadata = check!(fs::symlink_metadata(&symlink_name)); - let mut init_symlink_permissions = init_symlink_metadata.permissions(); - + assert!(!init_symlink_metadata.permissions().readonly()); let init_target_metadata = check!(fs::metadata(&symlink_name)); - let init_target_permissions = init_target_metadata.permissions(); + assert!(!init_target_metadata.permissions().readonly()); // Set symlink permissions to readonly - init_symlink_permissions.set_readonly(true); - let result = fs::set_permissions_nofollow(&symlink_name, init_symlink_permissions); + let result = fs::set_permissions_nofollow(&symlink_name, { + let mut permissions = init_symlink_metadata.permissions(); + permissions.set_readonly(true); + permissions + }); + + // This should not change the permissions of the target! + let after_target_metadata = check!(fs::metadata(&symlink_name)); + assert_eq!(after_target_metadata.permissions(), init_target_metadata.permissions()); cfg_select! { any( @@ -687,16 +690,11 @@ fn set_get_permissions_nofollows_symlink() { target_os = "nto", target_os = "qnx" ) => { + // On these systems, it's confirmed the symlink itself is marked readonly + // https://superuser.com/questions/1099634/change-permissions-symbolic-link-mac-os assert_eq!(result.unwrap(), ()); - let after_target_metadata = check!(fs::metadata(&symlink_name)); - // We should expect the target file to not have its permission bits - // changed - assert_eq!(after_target_metadata.permissions(), init_target_permissions); - let after_symlink_metadata = check!(fs::symlink_metadata(&symlink_name)); - // On these systems, it's confirmed the symlink itself is marked readonly - // https://superuser.com/questions/1099634/change-permissions-symbolic-link-mac-os assert!(after_symlink_metadata.permissions().readonly()); // Reset the read-only bit under Windows 7: avoids the @@ -710,11 +708,7 @@ fn set_get_permissions_nofollows_symlink() { } } _ => { - let after_target_metadata = check!(fs::metadata(&symlink_name)); - // We should expect the target file to not have its permission bits - // changed - assert_eq!(after_target_metadata.permissions(), init_target_permissions); - + // Everywhere else, this just fails. let error_kind = result.unwrap_err().kind(); assert_eq!(error_kind, crate::io::ErrorKind::Unsupported); } diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs index 46d5d33d23ac2..7c29cacbe262d 100644 --- a/library/std/src/sys/fs/unix.rs +++ b/library/std/src/sys/fs/unix.rs @@ -1896,22 +1896,11 @@ pub fn set_perm_nofollow(_p: &CStr, _perm: FilePermissions) -> io::Result<()> { Err(crate::io::ErrorKind::Unsupported.into()) } -#[cfg(target_os = "android")] -pub fn set_perm_nofollow(_p: &CStr, _perm: FilePermissions) -> io::Result<()> { - // Currently Android seems to be having inconsistent behavior with fchmodat - // with `AT_SYMLINK_NOFOLLOW` or openat with `O_NOFOLLOW` + fchmod. - // See this issue here mentioning inconsistent behavior on fchmodat: - // https://github.com/android/ndk/issues/1258 - // On the arm-android CI job, using fchmodat with `AT_SYMLINK_NOFOLLOW` + - // fallback behavior on a symlink sets the target file's permissions, - // which is incorrect behavior. - Err(crate::io::ErrorKind::Unsupported.into()) -} - -#[cfg(not(any(target_os = "android", target_os = "vxworks")))] +#[cfg(not(target_os = "vxworks"))] pub fn set_perm_nofollow(p: &CStr, perm: FilePermissions) -> io::Result<()> { + /// Helper function for fallback open with `O_NOFOLLOW` + `fchmod` behavior. This will + /// successfully change the permissions of non-symlinks and fail when there's a symlink. #[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}; @@ -1926,6 +1915,7 @@ pub fn set_perm_nofollow(p: &CStr, perm: FilePermissions) -> io::Result<()> { use crate::os::unix::fs::OpenOptionsExt; #[cfg(target_os = "wasi")] use crate::os::wasi::fs::OpenOptionsExt; + options.custom_flags(libc::O_NOFOLLOW); } @@ -1940,7 +1930,8 @@ pub fn set_perm_nofollow(p: &CStr, perm: FilePermissions) -> io::Result<()> { #[allow(unused)] let mut res: Result<(), core::io::Error> = Err(crate::io::ErrorKind::Unsupported.into()); - // These platforms support `fchmodat`, so utilize this syscall over `open` + `fchmod` + // These platforms support `fchmodat`, so utilize this syscall over `open` + `fchmod`. + // We can *not* use this on Android due to . #[cfg(any( target_os = "linux", target_os = "macos",