Added implementation on set_permissions_nofollow for all platforms supported (windows, unix, uefi, etc.); modified the Unix implementation to use fchmodat instead of open + fchmod; clarified documentations for set_permissions_nofollow; added a test case for set_permissions_nofollow

Author: asder8215

library/std/src/fs.rs

@@ -3337,23 +3337,60 @@ pub fn set_permissions<P: AsRef<Path>>(path: P, perm: Permissions) -> io::Result
     fs_imp::set_permissions(path.as_ref(), perm.0)
 }
 
-/// Set the permissions of a file, unless it is a symlink.
+/// Changes the permissions found on a file or a directory. On certain platforms, if the file
+/// is a symlink, it will change the permissions bits on the symlink itself rather than
+/// the target (e.g. Windows, BSD, MacOS). On other platforms, this results in an error when
+/// attempting to change permissions on a symlink (e.g. Linux).
 ///
-/// Note that the non-final path elements are allowed to be symlinks.
+/// Note that non-final path elements are allowed to be symlinks.
 ///
 /// # Platform-specific behavior
 ///
-/// Currently unimplemented on Windows.
+/// This function currently corresponds to `open` with `O_NOFOLLOW` flag enabled
+/// + `fchmod` on WASI and the `fchmodat` function on all other Unix platforms with
+/// the flag `AT_SYMLINK_NOFOLLOW` enabled. On Windows, the file is opened
+/// with the flag `FILE_FLAG_OPEN_REPARSE_POINT` enabled and then the permissions
+/// is set through `SetFileInformationByHandle`. On all other platforms, the behavior
+/// remains the same with [`fs::set_permissions`].
 ///
-/// On Unix platforms, this results in a [`FilesystemLoop`] error if the last element is a symlink.
+/// [`fs::set_permissions`]: crate::fs::set_permissions
 ///
-/// This behavior may change in the future.
+/// Note that, this [may change in the future][changes].
 ///
-/// [`FilesystemLoop`]: crate::io::ErrorKind::FilesystemLoop
-#[doc(alias = "chmod", alias = "SetFileAttributes")]
+/// [changes]: io#platform-specific-behavior
+///
+/// # Errors
+///
+/// This function will return an error in the following situations, but is not
+/// limited to just these cases:
+///
+/// * `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.
+///
+/// [`Unsupported`]: crate::io::ErrorKind::Unsupported
+///
+/// # Examples
+///
+/// ```no_run
+/// #![feature(set_permissions_nofollow)]
+/// use std::fs;
+///
+/// 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
+///     fs::set_permissions_nofollow("foo.txt", perms)?;
+///     Ok(())
+/// }
+/// ```
+#[doc(alias = "fchmodat", alias = "SetFileInformationByHandle")]
 #[unstable(feature = "set_permissions_nofollow", issue = "141607")]
 pub fn set_permissions_nofollow<P: AsRef<Path>>(path: P, perm: Permissions) -> io::Result<()> {
-    fs_imp::set_permissions_nofollow(path.as_ref(), perm)
+    fs_imp::set_permissions_nofollow(path.as_ref(), perm.0)
 }
 
 impl DirBuilder {

library/std/src/fs/tests.rs

@@ -613,6 +613,66 @@ fn set_get_unix_permissions() {
     assert_eq!(mask & metadata1.permissions().mode(), 0o0777);
 }
 
+#[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();
+    permission_bits.set_readonly(true);
+    let result = fs::set_permissions_nofollow(&filename, permission_bits);
+
+    cfg_select! {
+        any(windows, unix, target_os = "uefi", target_os = "solid_asp3", target_os = "motor") => {
+            assert_eq!(result.unwrap(), ());
+            let metadata0 = check!(fs::metadata(&filename));
+            assert!(metadata0.permissions().readonly());
+        },
+        _ => {
+            let error_kind = result.unwrap_err().kind();
+            assert_eq!(error_kind, crate::io::ErrorKind::Unsupported);
+        }
+    }
+}
+
+// Only Windows and Unix support `fs::set_permissions_nofollow`
+#[test]
+#[cfg(any(windows, unix))]
+fn set_get_permissions_nofollows_symlink() {
+    #[cfg(not(windows))]
+    use crate::os::unix::fs::symlink;
+    #[cfg(windows)]
+    use crate::os::windows::fs::symlink_dir;
+
+    let tmpdir = tmpdir();
+    let filename = tmpdir.join("set_get_unix_permissions_file");
+    let symlink_name = tmpdir.join("set_get_unix_permissions");
+    check!(File::create(&filename));
+    #[cfg(not(windows))]
+    check!(symlink(&filename, &symlink_name));
+    #[cfg(windows)]
+    check!(symlink_dir(&filename, &symlink_name));
+
+    let sym_metadata = check!(fs::symlink_metadata(&symlink_name));
+    let mut permission_bits = sym_metadata.permissions();
+    permission_bits.set_readonly(true);
+    let result = fs::set_permissions_nofollow(&symlink_name, permission_bits);
+
+    cfg_select! {
+        any(target_os = "macos", target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly", target_os = "espidf", target_os = "horizon") => {
+            assert_eq!(result.unwrap(), ());
+            let metadata0 = check!(fs::symlink_metadata(&symlink_name));
+            assert!(metadata0.permissions().readonly());
+        },
+        _ => {
+            let error_kind = result.unwrap_err().kind();
+            assert_eq!(error_kind, crate::io::ErrorKind::Unsupported);
+        }
+    }
+}
+
 #[test]
 #[cfg(windows)]
 fn file_test_io_seek_read_write() {
@@ -1330,6 +1390,29 @@ fn fchmod_works() {
     check!(file.set_permissions(p));
 }
 
+#[test]
+fn fchmodat_works() {
+    let tmpdir = tmpdir();
+    let file = tmpdir.join("in.txt");
+
+    check!(File::create(&file));
+    let attr = check!(fs::metadata(&file));
+    assert!(!attr.permissions().readonly());
+    let mut p = attr.permissions();
+    p.set_readonly(true);
+    check!(fs::set_permissions_nofollow(&file, p.clone()));
+    let attr = check!(fs::metadata(&file));
+    assert!(attr.permissions().readonly());
+
+    match fs::set_permissions_nofollow(&tmpdir.join("foo"), p.clone()) {
+        Ok(..) => panic!("wanted an error"),
+        Err(..) => {}
+    }
+
+    p.set_readonly(false);
+    check!(fs::set_permissions_nofollow(&file, p));
+}
+
 #[test]
 fn sync_doesnt_kill_anything() {
     let tmpdir = tmpdir();

library/std/src/sys/fs/hermit.rs

@@ -566,6 +566,10 @@ pub fn set_perm(_p: &Path, _perm: FilePermissions) -> io::Result<()> {
     Err(Error::from_raw_os_error(22))
 }
 
+pub fn set_perm_nofollow(_p: &Path, _perm: FilePermissions) -> io::Result<()> {
+    unsupported()
+}
+
 pub fn set_times(_p: &Path, _times: FileTimes) -> io::Result<()> {
     Err(Error::from_raw_os_error(22))
 }

library/std/src/sys/fs/mod.rs

@@ -120,28 +120,8 @@ pub fn set_permissions(path: &Path, perm: FilePermissions) -> io::Result<()> {
     with_native_path(path, &|path| imp::set_perm(path, perm.clone()))
 }
 
-#[cfg(all(unix, not(target_os = "vxworks")))]
-pub fn set_permissions_nofollow(path: &Path, perm: crate::fs::Permissions) -> io::Result<()> {
-    use crate::fs::OpenOptions;
-
-    let mut options = OpenOptions::new();
-
-    // 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")))]
-    {
-        use crate::os::unix::fs::OpenOptionsExt;
-        options.custom_flags(libc::O_NOFOLLOW);
-    }
-
-    options.open(path)?.set_permissions(perm)
-}
-
-#[cfg(any(not(unix), target_os = "vxworks"))]
-pub fn set_permissions_nofollow(_path: &Path, _perm: crate::fs::Permissions) -> io::Result<()> {
-    crate::unimplemented!(
-        "`set_permissions_nofollow` is currently only implemented on Unix platforms"
-    )
+pub fn set_permissions_nofollow(path: &Path, perm: FilePermissions) -> io::Result<()> {
+    with_native_path(path, &|path| imp::set_perm_nofollow(path, perm.clone()))
 }
 
 pub fn canonicalize(path: &Path) -> io::Result<PathBuf> {

library/std/src/sys/fs/motor.rs

@@ -319,6 +319,11 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> {
 }
 
 pub fn set_perm(path: &Path, perm: FilePermissions) -> io::Result<()> {
+    // Motor does not support symlinks
+    set_perm_nofollow(path, perm)
+}
+
+pub fn set_perm_nofollow(path: &Path, perm: FilePermissions) -> io::Result<()> {
     let path = path.to_str().ok_or(io::Error::from(io::ErrorKind::InvalidFilename))?;
     moto_rt::fs::set_perm(path, perm.rt_perm).map_err(map_motor_error)
 }

library/std/src/sys/fs/solid.rs

@@ -531,6 +531,11 @@ pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
 }
 
 pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
+    // Solid does not support symlinks
+    set_perm_nofollow(p, perm)
+}
+
+pub fn set_perm_nofollow(p: &Path, perm: FilePermissions) -> io::Result<()> {
     error::SolidError::err_if_negative(unsafe {
         abi::SOLID_FS_Chmod(cstr(p)?.as_ptr(), perm.0.into())
     })

library/std/src/sys/fs/uefi.rs

@@ -480,6 +480,11 @@ pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
 }
 
 pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
+    // UEFI does not support symlinks
+    set_perm_nofollow(p, perm);
+}
+
+pub fn set_perm_nofollow(p: &Path, perm: FilePermissions) -> io::Result<()> {
     let f = uefi_fs::File::from_path(p, file::MODE_READ | file::MODE_WRITE, 0)?;
     set_perm_inner(&f, perm)
 }

library/std/src/sys/fs/unix.rs

@@ -1989,6 +1989,43 @@ pub fn set_perm(p: &CStr, perm: FilePermissions) -> io::Result<()> {
     cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ())
 }
 
+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;
+            use crate::fs::Permissions;
+            use crate::os::wasi::ffi::OsStrExt;
+            let mut options = OpenOptions::new();
+
+            #[cfg(not(any(target_os = "espidf", target_os = "horizon")))]
+            {
+                use crate::os::wasi::fs::OpenOptionsExt;
+                options.custom_flags(libc::O_NOFOLLOW);
+            }
+
+            let bytes = p.to_bytes();
+            let os_str = OsStr::from_bytes(bytes);
+            options.open(Path::new(os_str))?.set_permissions(Permissions::from_inner(perm))
+        }
+        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(|_| ())
+        },
+        _ => {
+            cvt_r(|| unsafe {
+                libc::fchmodat(libc::AT_FDCWD, p.as_ptr(), perm.mode, 0)
+            })
+            .map(|_| ())
+        }
+    }
+}
+
 pub fn rmdir(p: &CStr) -> io::Result<()> {
     cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ())
 }

library/std/src/sys/fs/unsupported.rs

@@ -313,6 +313,10 @@ pub fn set_perm(_p: &Path, perm: FilePermissions) -> io::Result<()> {
     match perm.0 {}
 }
 
+pub fn set_perm_nofollow(_p: &Path, perm: FilePermissions) -> io::Result<()> {
+    match perm.0 {}
+}
+
 pub fn set_times(_p: &Path, _times: FileTimes) -> io::Result<()> {
     unsupported()
 }

library/std/src/sys/fs/vexos.rs

@@ -492,6 +492,10 @@ pub fn set_perm(_p: &Path, _perm: FilePermissions) -> io::Result<()> {
     unsupported()
 }
 
+pub fn set_perm_nofollow(_p: &Path, _perm: FilePermissions) -> io::Result<()> {
+    unsupported()
+}
+
 pub fn set_times(_p: &Path, _times: FileTimes) -> io::Result<()> {
     unsupported()
 }