Refactored docs for std::fs::set_permissions_nofollow + fix BSD-based systems to use fchmodat with AT_SYMLINK_NOFOLLOW flag

library/std/src/fs.rs

@@ -3448,22 +3448,26 @@ pub fn set_permissions<P: AsRef<Path>>(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
-/// [`fs::set_permissions`].
-///
-/// [`fs::set_permissions`]: crate::fs::set_permissions
+/// This function currently corresponds to the following underlying operations:
+/// * Linux, BSD-based platforms, Android, QNX: `fchmodat` with `AT_SYMLINK_NOFOLLOW`
+/// with a fallback behavior to use `open` with `O_NOFOLLOW` followed by behavior
+/// denoted in [`fs::set_permissions`] when the former `fchmodat` call errors with `ENOTSUP`[^1].
+/// * Other Unix-based platforms with symlinks: `open` with `O_NOFOLLOW` followed by behavior
+///   denoted in [`fs::set_permissions`].
+/// * Other Unix-based platforms without symlinks: `open` followed by behavior
+///   denoted in [`fs::set_permissions`].
+/// * Windows: `CreateFileW` with `FILE_FLAG_OPEN_REPARSE_POINT` followed
+///   by `SetFileInformationByHandle`.
 ///
 /// Note that, this [may change in the future][changes].
 ///
+/// [^1]: Ubuntu 20.04, for example, makes `fchmodat` with `AT_SYMLINK_NOFOLLOW` return `ENOTSUP`
+/// on both symlinks and non-symlinks.
+///
 /// [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
@@ -3472,12 +3476,11 @@ pub fn set_permissions<P: AsRef<Path>>(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
+/// [`FilesystemLoop`]: crate::io::ErrorKind::FilesystemLoop
 ///
 /// # Examples
 ///
@@ -3488,8 +3491,8 @@ pub fn set_permissions<P: AsRef<Path>>(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(())
 /// }

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;
@@ -668,17 +671,12 @@ fn set_get_permissions_nofollows_symlink() {
     let result = fs::set_permissions_nofollow(&symlink_name, permission_bits);
 
     cfg_select! {
-        any(windows, target_os = "android", target_os = "macos", target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly") => {
+        any(windows, target_os = "macos", target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly") => {
             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

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.

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

@@ -2036,35 +2036,76 @@ 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;
-            use crate::fs::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(|_| ())
-        },
-        _ => {
-            cvt_r(|| unsafe {
-                libc::fchmodat(libc::AT_FDCWD, p.as_ptr(), perm.mode, 0)
-            })
-            .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 = "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 O_NOFOLLOW flag is enabled, if the trailing component of
+                            // a path is a symbolic link, open should fail with ELOOP error.
+                            // For consistency with other Linux distributions, we return
+                            // `ErrorKind::Unsupported`.
+                            return Err(err);
+                        }
+                        return Err(e);
+                    }
+                }
+            }
+
+            Err(err)
         }
     }
 }