Fix tests on FreeBSD (#341)

* Disable tests that fail on FreeBSD. Disable tests that are failing on FreeBSD. The "ambient" versions of these tests are also failing, so this appears to be platform-specific behavior rather than cap-std's behavior. * Don't install `curl`; just use the default one. * Check for opening `..` in the FreeBSD `RESOLVE_BENEATH` check. Change the `RESOLVE_BENEATH` check to check for the behavior of opening `..` on FreeBSD. * Add a freebsd-14-0-snap entry to the Cirrus CI config. FreeBSD 14 supports the RESOLVE_BENEATH semantics where `..` is a capability error even when the base fd is the root directory.
bytecodealliance · Jan 2, 2024 · 465c70b · 465c70b
1 parent 5e32356
commit 465c70b
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 47 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -1,12 +1,24 @@
 # Implementation derived from `.cirrus.yml` in Rust's libc bindings
 # at revision 7f4774e76bd5cb9ccb7140d71ef9be9c16009cdf.
 
+task:
+  name: stable x86_64-unknown-freebsd-14-snap
+  freebsd_instance:
+    image_family: freebsd-14-0-snap
+  setup_script:
+    - curl https://sh.rustup.rs -sSf --output rustup.sh
+    - sh rustup.sh --default-toolchain stable -y --profile=minimal
+    - . $HOME/.cargo/env
+    - rustup default stable
+  test_script:
+    - . $HOME/.cargo/env
+    - cargo test --features=fs_utf8 --workspace
+
 task:
   name: stable x86_64-unknown-freebsd-13
   freebsd_instance:
     image_family: freebsd-13-2
   setup_script:
-    - pkg install -y curl
     - curl https://sh.rustup.rs -sSf --output rustup.sh
     - sh rustup.sh --default-toolchain stable -y --profile=minimal
     - . $HOME/.cargo/env
@@ -20,7 +32,6 @@ task:
   freebsd_instance:
     image_family: freebsd-12-4
   setup_script:
-    - pkg install -y curl
     - curl https://sh.rustup.rs -sSf --output rustup.sh
     - sh rustup.sh --default-toolchain stable -y --profile=minimal
     - . $HOME/.cargo/env

diff --git a/cap-primitives/src/rustix/freebsd/fs/check.rs b/cap-primitives/src/rustix/freebsd/fs/check.rs
@@ -1,26 +1,40 @@
-use rustix::fs::{statat, AtFlags};
-use std::fs;
+use rustix::cstr;
+use rustix::fs::{openat, statat, AtFlags, Mode, OFlags, CWD};
+use rustix::io::Errno;
 use std::sync::atomic::{AtomicBool, Ordering::Relaxed};
 
 static WORKING: AtomicBool = AtomicBool::new(false);
 static CHECKED: AtomicBool = AtomicBool::new(false);
 
 #[inline]
-pub(crate) fn beneath_supported(start: &fs::File) -> bool {
+pub(crate) fn beneath_supported() -> bool {
     if WORKING.load(Relaxed) {
         return true;
     }
     if CHECKED.load(Relaxed) {
         return false;
     }
-    // Unknown O_ flags get ignored but AT_ flags have strict checks, so we use that.
-    if let Err(rustix::io::Errno::INVAL) =
-        statat(start, "", AtFlags::EMPTY_PATH | AtFlags::RESOLVE_BENEATH)
-    {
-        CHECKED.store(true, Relaxed);
-        false
-    } else {
-        WORKING.store(true, Relaxed);
-        true
+    check_beneath_supported()
+}
+
+#[cold]
+fn check_beneath_supported() -> bool {
+    // `RESOLVE_BENEATH` was introduced in FreeBSD 13, but opening `..` within
+    // the root directory re-opened the root directory. In FreeBSD 14, it fails
+    // as cap-std expects.
+    if let Ok(root) = openat(
+        CWD,
+        cstr!("/"),
+        OFlags::RDONLY | OFlags::CLOEXEC,
+        Mode::empty(),
+    ) {
+        // Unknown O_ flags get ignored but AT_ flags have strict checks, so we use that.
+        if let Err(Errno::NOTCAPABLE) = statat(root, cstr!(".."), AtFlags::RESOLVE_BENEATH) {
+            WORKING.store(true, Relaxed);
+            return true;
+        }
     }
+
+    CHECKED.store(true, Relaxed);
+    false
 }
diff --git a/cap-primitives/src/rustix/freebsd/fs/open_impl.rs b/cap-primitives/src/rustix/freebsd/fs/open_impl.rs
@@ -10,7 +10,7 @@ pub(crate) fn open_impl(
     path: &Path,
     options: &OpenOptions,
 ) -> io::Result<fs::File> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return manually::open(start, path, options);
     }
 

diff --git a/cap-primitives/src/rustix/freebsd/fs/remove_dir_impl.rs b/cap-primitives/src/rustix/freebsd/fs/remove_dir_impl.rs
@@ -4,7 +4,7 @@ use std::path::Path;
 use std::{fs, io};
 
 pub(crate) fn remove_dir_impl(start: &fs::File, path: &Path) -> io::Result<()> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return via_parent::remove_dir(start, path);
     }
 

diff --git a/cap-primitives/src/rustix/freebsd/fs/remove_file_impl.rs b/cap-primitives/src/rustix/freebsd/fs/remove_file_impl.rs
@@ -4,7 +4,7 @@ use std::path::Path;
 use std::{fs, io};
 
 pub(crate) fn remove_file_impl(start: &fs::File, path: &Path) -> io::Result<()> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return via_parent::remove_file(start, path);
     }
 

diff --git a/cap-primitives/src/rustix/freebsd/fs/set_permissions_impl.rs b/cap-primitives/src/rustix/freebsd/fs/set_permissions_impl.rs
@@ -9,7 +9,7 @@ pub(crate) fn set_permissions_impl(
     path: &Path,
     perm: Permissions,
 ) -> io::Result<()> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return super::super::super::fs::set_permissions_manually(start, path, perm);
     }
 

diff --git a/cap-primitives/src/rustix/freebsd/fs/set_times_impl.rs b/cap-primitives/src/rustix/freebsd/fs/set_times_impl.rs
@@ -9,7 +9,7 @@ pub(crate) fn set_times_impl(
     atime: Option<SystemTimeSpec>,
     mtime: Option<SystemTimeSpec>,
 ) -> io::Result<()> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return super::super::super::fs::set_times_manually(start, path, atime, mtime);
     }
 
@@ -27,7 +27,7 @@ pub(crate) fn set_times_nofollow_impl(
     atime: Option<SystemTimeSpec>,
     mtime: Option<SystemTimeSpec>,
 ) -> io::Result<()> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return via_parent::set_times_nofollow(start, path, atime, mtime);
     }
 

diff --git a/cap-primitives/src/rustix/freebsd/fs/stat_impl.rs b/cap-primitives/src/rustix/freebsd/fs/stat_impl.rs
@@ -8,7 +8,7 @@ pub(crate) fn stat_impl(
     path: &Path,
     follow: FollowSymlinks,
 ) -> io::Result<Metadata> {
-    if !super::beneath_supported(start) {
+    if !super::beneath_supported() {
         return manually::stat(start, path, follow);
     }
 

diff --git a/tests/fs_additional.rs b/tests/fs_additional.rs
@@ -454,16 +454,18 @@ fn check_dot_access() {
     check!(tmpdir.metadata("dir/"));
     check!(tmpdir.metadata("dir//"));
 
-    assert!(tmpdir.metadata("dir/.").is_err());
-    assert!(tmpdir.metadata("dir/./").is_err());
-    assert!(tmpdir.metadata("dir/.//").is_err());
-    assert!(tmpdir.metadata("dir/./.").is_err());
-    assert!(tmpdir.metadata("dir/.//.").is_err());
-    assert!(tmpdir.metadata("dir/..").is_err());
-    assert!(tmpdir.metadata("dir/../").is_err());
-    assert!(tmpdir.metadata("dir/..//").is_err());
-    assert!(tmpdir.metadata("dir/../.").is_err());
-    assert!(tmpdir.metadata("dir/..//.").is_err());
+    if !cfg!(target_os = "freebsd") {
+        assert!(tmpdir.metadata("dir/.").is_err());
+        assert!(tmpdir.metadata("dir/./").is_err());
+        assert!(tmpdir.metadata("dir/.//").is_err());
+        assert!(tmpdir.metadata("dir/./.").is_err());
+        assert!(tmpdir.metadata("dir/.//.").is_err());
+        assert!(tmpdir.metadata("dir/..").is_err());
+        assert!(tmpdir.metadata("dir/../").is_err());
+        assert!(tmpdir.metadata("dir/..//").is_err());
+        assert!(tmpdir.metadata("dir/../.").is_err());
+        assert!(tmpdir.metadata("dir/..//.").is_err());
+    }
 }
 
 /// This test is the same as `check_dot_access` but uses `std::fs`'
@@ -486,16 +488,18 @@ fn check_dot_access_ambient() {
     check!(fs::metadata(dir.path().join("dir/")));
     check!(fs::metadata(dir.path().join("dir//")));
 
-    assert!(fs::metadata(dir.path().join("dir/.")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/./")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/.//")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/./.")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/.//.")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/..")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/../")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/..//")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/../.")).is_err());
-    assert!(fs::metadata(dir.path().join("dir/..//.")).is_err());
+    if !cfg!(target_os = "freebsd") {
+        assert!(fs::metadata(dir.path().join("dir/.")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/./")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/.//")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/./.")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/.//.")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/..")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/../")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/..//")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/../.")).is_err());
+        assert!(fs::metadata(dir.path().join("dir/..//.")).is_err());
+    }
 }
 
 // Windows allows one to open "file/." and "file/.." and similar, however it
@@ -650,19 +654,16 @@ fn dir_unsearchable_unreadable() {
     options.mode(0o000);
     check!(tmpdir.create_dir_with("dir", &options));
 
-    // Platforms with `O_PATH` can open a directory with no permissions. And
-    // somehow FreeBSD can too; see `dir_unsearchable_unreadable_ambient`
-    // below confirming this.
+    // Platforms with `O_PATH` can open a directory with no permissions.
     if cfg!(any(
         target_os = "android",
-        target_os = "freebsd",
         target_os = "linux",
         target_os = "redox",
     )) {
         let dir = check!(tmpdir.open_dir("dir"));
         assert!(dir.entries().is_err());
         assert!(dir.open_dir(".").is_err());
-    } else {
+    } else if !cfg!(target_os = "freebsd") {
         assert!(tmpdir.open_dir("dir").is_err());
     }
 }
@@ -684,7 +685,6 @@ fn dir_unsearchable_unreadable_ambient() {
     if cfg!(any(
         target_os = "android",
         target_os = "linux",
-        target_os = "freebsd",
         target_os = "redox",
     )) {
         assert!(std::fs::File::open(dir.path().join("dir")).is_err());