From 9184be29b9077ef0d0bb2877a4d93c0ba8d8dd51 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 5 Feb 2024 16:49:48 -0800 Subject: [PATCH 1/8] fd_filestat_set test: assert that a file descriptor behaves the same... whether opened readonly or readwrite. This test now reproduces the behavior reported in https://github.com/bytecodealliance/wasmtime/issues/7829 Co-authored-by: Trevor Elliott --- .../src/bin/preview1_fd_filestat_set.rs | 73 +++++++++++++++++-- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs index 240664dd9cd3..c550c4502c84 100644 --- a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs +++ b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs @@ -1,9 +1,10 @@ use std::{env, process, time::Duration}; -use test_programs::preview1::{assert_fs_time_eq, open_scratch_directory, TestConfig}; +use test_programs::preview1::{ + assert_errno, assert_fs_time_eq, open_scratch_directory, TestConfig, +}; -unsafe fn test_fd_filestat_set(dir_fd: wasi::Fd) { - let cfg = TestConfig::from_env(); - // Create a file in the scratch directory. +unsafe fn test_fd_filestat_set_size_rw(dir_fd: wasi::Fd) { + // Create a file in the scratch directory, opened read/write let file_fd = wasi::path_open( dir_fd, 0, @@ -29,6 +30,55 @@ unsafe fn test_fd_filestat_set(dir_fd: wasi::Fd) { let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat 2"); assert_eq!(stat.size, 100, "file size should be 100"); + wasi::fd_close(file_fd).expect("failed to close fd"); + wasi::path_unlink_file(dir_fd, "file").expect("failed to remove file"); +} + +unsafe fn test_fd_filestat_set_size_ro(dir_fd: wasi::Fd) { + // Create a file in the scratch directory. Creating a file implies opening it for writing, so + // we have to close and re-open read-only to observe read-only behavior. + let file_fd = wasi::path_open(dir_fd, 0, "file", wasi::OFLAGS_CREAT, 0, 0, 0) + .expect("failed to create file"); + wasi::fd_close(file_fd).expect("failed to close fd"); + + // Open the created file read-only + let file_fd = wasi::path_open(dir_fd, 0, "file", 0, wasi::RIGHTS_FD_READ, 0, 0) + .expect("failed to create file"); + + // Check file size + let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat"); + assert_eq!(stat.size, 0, "file size should be 0"); + + // Check fd_filestat_set_size on a file opened read-only fails with EINVAL, like ftruncate is defined to do on posix + assert_errno!( + wasi::fd_filestat_set_size(file_fd, 100) + .expect_err("fd_filestat_set_size should error when file is opened read-only"), + wasi::ERRNO_INVAL + ); + + let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat 2"); + assert_eq!(stat.size, 0, "file size should remain 0"); + + wasi::fd_close(file_fd).expect("failed to close fd"); + wasi::path_unlink_file(dir_fd, "file").expect("failed to remove file"); +} + +unsafe fn test_fd_filestat_set_times(dir_fd: wasi::Fd, rights: wasi::Rights) { + let cfg = TestConfig::from_env(); + + // Create a file in the scratch directory. OFLAGS_CREAT implies opening for writing, so we will + // close it and re-open with the desired rights (FD_READ for read only, FD_READ | FD_WRITE for + // readwrite) + let file_fd = wasi::path_open(dir_fd, 0, "file", wasi::OFLAGS_CREAT, 0, 0, 0) + .expect("failed to create file"); + wasi::fd_close(file_fd).expect("failed to close fd"); + + // Open the file with the rights given. + let file_fd = + wasi::path_open(dir_fd, 0, "file", 0, rights, 0, 0).expect("failed to create file"); + + let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat 2"); + // Check fd_filestat_set_times let old_atim = Duration::from_nanos(stat.atim); let new_mtim = Duration::from_nanos(stat.mtim) - cfg.fs_time_precision() * 2; @@ -41,7 +91,7 @@ unsafe fn test_fd_filestat_set(dir_fd: wasi::Fd) { .expect("fd_filestat_set_times"); let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat 3"); - assert_eq!(stat.size, 100, "file size should remain unchanged at 100"); + assert_eq!(stat.size, 0, "file size should remain unchanged at 0"); // Support accuracy up to at least 1ms assert_fs_time_eq!( @@ -59,7 +109,7 @@ unsafe fn test_fd_filestat_set(dir_fd: wasi::Fd) { // assert_eq!(status, wasi::EINVAL, "ATIM & ATIM_NOW can't both be set"); wasi::fd_close(file_fd).expect("failed to close fd"); - wasi::path_unlink_file(dir_fd, "file").expect("failed to remove dir"); + wasi::path_unlink_file(dir_fd, "file").expect("failed to remove file"); } fn main() { let mut args = env::args(); @@ -81,5 +131,14 @@ fn main() { }; // Run the tests. - unsafe { test_fd_filestat_set(dir_fd) } + // + unsafe { test_fd_filestat_set_size_rw(dir_fd) } + unsafe { test_fd_filestat_set_size_ro(dir_fd) } + // + // The fd_filestat_set_times function should behave the same whether the file + // descriptor is open for read-only or read-write, because the underlying + // permissions of the file determine whether or not the filestat can be + // set or not, not than the open mode. + unsafe { test_fd_filestat_set_times(dir_fd, wasi::RIGHTS_FD_READ) } + unsafe { test_fd_filestat_set_times(dir_fd, wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE) } } From 3d40c1d0452a70b11e6583f24aa3f8bf4b627f6d Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 9 Feb 2024 16:26:08 -0800 Subject: [PATCH 2/8] preview1_path_link test: refactor; create and open file separately we create and open the file with separate descriptors because the creation descriptor will always imply writing, whereas the rest do not. there were a number of auxillary functions in here that were obscuring what was going on, and making errors harder to read, so i changed some assertions to use macro-rules so the panic message had the relevant line number, and i inlined the creation / opening functions. --- .../src/bin/preview1_path_link.rs | 124 +++++++++--------- 1 file changed, 59 insertions(+), 65 deletions(-) diff --git a/crates/test-programs/src/bin/preview1_path_link.rs b/crates/test-programs/src/bin/preview1_path_link.rs index b087e0740003..cc14a0a01ac5 100644 --- a/crates/test-programs/src/bin/preview1_path_link.rs +++ b/crates/test-programs/src/bin/preview1_path_link.rs @@ -1,89 +1,82 @@ use std::{env, process}; use test_programs::preview1::{assert_errno, config, create_file, open_scratch_directory}; -unsafe fn create_or_open(dir_fd: wasi::Fd, name: &str, flags: wasi::Oflags) -> wasi::Fd { - let file_fd = wasi::path_open(dir_fd, 0, name, flags, 0, 0, 0) - .unwrap_or_else(|_| panic!("opening '{}'", name)); - assert!( - file_fd > libc::STDERR_FILENO as wasi::Fd, - "file descriptor range check", - ); - file_fd -} - -unsafe fn open_link(dir_fd: wasi::Fd, name: &str) -> wasi::Fd { - let file_fd = wasi::path_open(dir_fd, 0, name, 0, 0, 0, 0) - .unwrap_or_else(|_| panic!("opening a link '{}'", name)); - assert!( - file_fd > libc::STDERR_FILENO as wasi::Fd, - "file descriptor range check", - ); - file_fd -} - -// This is temporary until `wasi` implements `Debug` and `PartialEq` for -// `wasi::Filestat`. -fn filestats_assert_eq(left: wasi::Filestat, right: wasi::Filestat) { - assert_eq!(left.dev, right.dev, "dev should be equal"); - assert_eq!(left.ino, right.ino, "ino should be equal"); - assert_eq!(left.atim, right.atim, "atim should be equal"); - assert_eq!(left.ctim, right.ctim, "ctim should be equal"); - assert_eq!(left.mtim, right.mtim, "mtim should be equal"); - assert_eq!(left.size, right.size, "size should be equal"); - assert_eq!(left.nlink, right.nlink, "nlink should be equal"); - assert_eq!(left.filetype, right.filetype, "filetype should be equal"); +// These are all macro-rules so the panic line number shows us where +// things went wrong. + +macro_rules! filestats_assert_eq { + ($left:ident, $right:ident) => { + assert_eq!($left.dev, $right.dev, "dev should be equal"); + assert_eq!($left.ino, $right.ino, "ino should be equal"); + assert_eq!($left.atim, $right.atim, "atim should be equal"); + assert_eq!($left.ctim, $right.ctim, "ctim should be equal"); + assert_eq!($left.mtim, $right.mtim, "mtim should be equal"); + assert_eq!($left.size, $right.size, "size should be equal"); + assert_eq!($left.nlink, $right.nlink, "nlink should be equal"); + assert_eq!($left.filetype, $right.filetype, "filetype should be equal"); + }; } -// This is temporary until `wasi` implements `Debug` and `PartialEq` for -// `wasi::Fdstat`. -fn fdstats_assert_eq(left: wasi::Fdstat, right: wasi::Fdstat) { - assert_eq!(left.fs_flags, right.fs_flags, "fs_flags should be equal"); - assert_eq!( - left.fs_filetype, right.fs_filetype, - "fs_filetype should be equal" - ); - assert_eq!( - left.fs_rights_base, right.fs_rights_base, - "fs_rights_base should be equal" - ); - assert_eq!( - left.fs_rights_inheriting, right.fs_rights_inheriting, - "fs_rights_inheriting should be equal" - ); +macro_rules! fdstats_assert_eq { + ($left:ident, $right:ident) => { + assert_eq!($left.fs_flags, $right.fs_flags, "fs_flags should be equal"); + assert_eq!( + $left.fs_filetype, $right.fs_filetype, + "fs_filetype should be equal" + ); + assert_eq!( + $left.fs_rights_base, $right.fs_rights_base, + "fs_rights_base should be equal" + ); + assert_eq!( + $left.fs_rights_inheriting, $right.fs_rights_inheriting, + "fs_rights_inheriting should be equal" + ); + }; } -unsafe fn check_rights(orig_fd: wasi::Fd, link_fd: wasi::Fd) { - // Compare Filestats - let orig_filestat = wasi::fd_filestat_get(orig_fd).expect("reading filestat of the source"); - let link_filestat = wasi::fd_filestat_get(link_fd).expect("reading filestat of the link"); - filestats_assert_eq(orig_filestat, link_filestat); - - // Compare Fdstats - let orig_fdstat = wasi::fd_fdstat_get(orig_fd).expect("reading fdstat of the source"); - let link_fdstat = wasi::fd_fdstat_get(link_fd).expect("reading fdstat of the link"); - fdstats_assert_eq(orig_fdstat, link_fdstat); +macro_rules! check_rights { + ($orig_fd:ident, $link_fd:ident) => { + let orig_filestat = + wasi::fd_filestat_get($orig_fd).expect("reading filestat of the source"); + let link_filestat = wasi::fd_filestat_get($link_fd).expect("reading filestat of the link"); + filestats_assert_eq!(orig_filestat, link_filestat); + + // Compare Fdstats + let orig_fdstat = wasi::fd_fdstat_get($orig_fd).expect("reading fdstat of the source"); + let link_fdstat = wasi::fd_fdstat_get($link_fd).expect("reading fdstat of the link"); + fdstats_assert_eq!(orig_fdstat, link_fdstat); + }; } - // Extra calls of fd_close are needed for Windows, which will not remove // the directory until all handles are closed. unsafe fn test_path_link(dir_fd: wasi::Fd) { // Create a file - let file_fd = create_or_open(dir_fd, "file", wasi::OFLAGS_CREAT); + let create_fd = + wasi::path_open(dir_fd, 0, "file", wasi::OFLAGS_CREAT, 0, 0, 0).expect("create file"); + wasi::fd_close(create_fd).unwrap(); + + // Open a fresh descriptor to the file. We won't have a write right that was implied by OFLAGS_CREAT + // above. + let file_fd = wasi::path_open(dir_fd, 0, "file", 0, 0, 0, 0).expect("open file"); // Create a link in the same directory and compare rights wasi::path_link(dir_fd, 0, "file", dir_fd, "link") .expect("creating a link in the same directory"); - let link_fd = open_link(dir_fd, "link"); - check_rights(file_fd, link_fd); + + let link_fd = wasi::path_open(dir_fd, 0, "link", 0, 0, 0, 0).expect("open link"); + + check_rights!(file_fd, link_fd); wasi::fd_close(link_fd).expect("Closing link_fd"); // needed for Windows wasi::path_unlink_file(dir_fd, "link").expect("removing a link"); // Create a link in a different directory and compare rights wasi::path_create_directory(dir_fd, "subdir").expect("creating a subdirectory"); - let subdir_fd = create_or_open(dir_fd, "subdir", wasi::OFLAGS_DIRECTORY); + let subdir_fd = wasi::path_open(dir_fd, 0, "subdir", wasi::OFLAGS_DIRECTORY, 0, 0, 0) + .expect("open subdir directory"); wasi::path_link(dir_fd, 0, "file", subdir_fd, "link").expect("creating a link in subdirectory"); - let link_fd = open_link(subdir_fd, "link"); - check_rights(file_fd, link_fd); + let link_fd = wasi::path_open(subdir_fd, 0, "link", 0, 0, 0, 0).expect("open link in subdir"); + check_rights!(file_fd, link_fd); wasi::fd_close(link_fd).expect("Closing link_fd"); // needed for Windows wasi::path_unlink_file(subdir_fd, "link").expect("removing a link"); wasi::fd_close(subdir_fd).expect("Closing subdir_fd"); // needed for Windows @@ -118,7 +111,8 @@ unsafe fn test_path_link(dir_fd: wasi::Fd) { // Create a link to a directory wasi::path_create_directory(dir_fd, "subdir").expect("creating a subdirectory"); - let subdir_fd = create_or_open(dir_fd, "subdir", wasi::OFLAGS_DIRECTORY); + let subdir_fd = wasi::path_open(dir_fd, 0, "subdir", wasi::OFLAGS_DIRECTORY, 0, 0, 0) + .expect("open new descriptor to subdir"); assert_errno!( wasi::path_link(dir_fd, 0, "subdir", dir_fd, "link") From 4eb836a508c4165c577d045b6d69c8fa90a4985e Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 5 Feb 2024 16:07:03 -0800 Subject: [PATCH 3/8] preview2 filesystem: track open mode instead of reporting permissions it n DescriptorFlags FilePerms/DirPerms is a way of having wasmtime-wasi, instead of the operating system, mediate permissions on a preview2 Descriptor. This was conflated with the open mode, which should determine whether DescriptorFlags contains READ or WRITE or MUTATE_DIRECTORY. We no longer mask FilePerms with the DescriptorFlags (oflags) in open_at - instead, track what open mode is requested, and see if the file and directory permissions permit it. Additionally, File and Dir now track their open_mode (represented using OpenMode, which just like FilePerms is just a read and write bit), and report that in DescriptorFlags. Previously, we reported the FilePerms or DirPerms in the DescriptorFlags, which was totally wrong. Co-authored-by: Trevor Elliott --- crates/wasi/src/preview2/ctx.rs | 15 +++- crates/wasi/src/preview2/filesystem.rs | 50 +++++++++++++- crates/wasi/src/preview2/host/filesystem.rs | 76 ++++++++++++--------- 3 files changed, 103 insertions(+), 38 deletions(-) diff --git a/crates/wasi/src/preview2/ctx.rs b/crates/wasi/src/preview2/ctx.rs index 97c0119f3878..f440fa4e5658 100644 --- a/crates/wasi/src/preview2/ctx.rs +++ b/crates/wasi/src/preview2/ctx.rs @@ -3,7 +3,7 @@ use crate::preview2::{ host::{monotonic_clock, wall_clock}, HostMonotonicClock, HostWallClock, }, - filesystem::Dir, + filesystem::{Dir, OpenMode}, network::{SocketAddrCheck, SocketAddrUse}, pipe, random, stdio, stdio::{StdinStream, StdoutStream}, @@ -144,8 +144,17 @@ impl WasiCtxBuilder { file_perms: FilePerms, path: impl AsRef, ) -> &mut Self { - self.preopens - .push((Dir::new(dir, perms, file_perms), path.as_ref().to_owned())); + let mut open_mode = OpenMode::empty(); + if perms.contains(DirPerms::READ) { + open_mode |= OpenMode::READ; + } + if perms.contains(DirPerms::MUTATE) { + open_mode |= OpenMode::WRITE; + } + self.preopens.push(( + Dir::new(dir, perms, file_perms, open_mode), + path.as_ref().to_owned(), + )); self } diff --git a/crates/wasi/src/preview2/filesystem.rs b/crates/wasi/src/preview2/filesystem.rs index 1f1d149dd93f..3b54797f65bd 100644 --- a/crates/wasi/src/preview2/filesystem.rs +++ b/crates/wasi/src/preview2/filesystem.rs @@ -67,20 +67,40 @@ bitflags::bitflags! { } } +bitflags::bitflags! { + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + pub struct OpenMode: usize { + const READ = 0b1; + const WRITE = 0b10; + } +} + pub struct File { + /// The operating system File this struct is mediating access to. + /// /// Wrapped in an Arc because the same underlying file is used for - /// implementing the stream types. Also needed for [`spawn_blocking`]. + /// implementing the stream types. A copy is also needed for + /// [`spawn_blocking`]. /// /// [`spawn_blocking`]: Self::spawn_blocking pub file: Arc, + /// Permissions to enforce on access to the file. These permissions are + /// specified by a user of the `crate::preview2::WasiCtxBuilder`, and are + /// enforced prior to any enforced by the underlying operating system. pub perms: FilePerms, + /// The mode the file was opened under: bits for reading, and writing. + /// Required to correctly report the DescriptorFlags, because cap-std + /// doesn't presently provide a cross-platform equivelant of reading the + /// oflags back out using fcntl. + pub open_mode: OpenMode, } impl File { - pub fn new(file: cap_std::fs::File, perms: FilePerms) -> Self { + pub fn new(file: cap_std::fs::File, perms: FilePerms, open_mode: OpenMode) -> Self { Self { file: Arc::new(file), perms, + open_mode, } } @@ -106,17 +126,41 @@ bitflags::bitflags! { #[derive(Clone)] pub struct Dir { + /// The operating system file descriptor this struct is mediating access + /// to. + /// + /// Wrapped in an Arc because a copy is needed for [`spawn_blocking`]. + /// + /// [`spawn_blocking`]: Self::spawn_blocking pub dir: Arc, + /// Permissions to enforce on access to this directory. These permissions + /// are specified by a user of the `crate::preview2::WasiCtxBuilder`, and + /// are enforced prior to any enforced by the underlying operating system. + /// + /// These permissions are also enforced on any directories opened under + /// this directory. pub perms: DirPerms, + /// Permissions to enforce on any files opened under this directory. pub file_perms: FilePerms, + /// The mode the directory was opened under: bits for reading, and writing. + /// Required to correctly report the DescriptorFlags, because cap-std + /// doesn't presently provide a cross-platform equivelant of reading the + /// oflags back out using fcntl. + pub open_mode: OpenMode, } impl Dir { - pub fn new(dir: cap_std::fs::Dir, perms: DirPerms, file_perms: FilePerms) -> Self { + pub fn new( + dir: cap_std::fs::Dir, + perms: DirPerms, + file_perms: FilePerms, + open_mode: OpenMode, + ) -> Self { Dir { dir: Arc::new(dir), perms, file_perms, + open_mode, } } diff --git a/crates/wasi/src/preview2/host/filesystem.rs b/crates/wasi/src/preview2/host/filesystem.rs index f2b67eca0de0..c6d93713f561 100644 --- a/crates/wasi/src/preview2/host/filesystem.rs +++ b/crates/wasi/src/preview2/host/filesystem.rs @@ -4,8 +4,9 @@ use crate::preview2::bindings::filesystem::types::{ self, ErrorCode, HostDescriptor, HostDirectoryEntryStream, }; use crate::preview2::bindings::io::streams::{InputStream, OutputStream}; -use crate::preview2::filesystem::{Descriptor, Dir, File, ReaddirIterator}; -use crate::preview2::filesystem::{FileInputStream, FileOutputStream}; +use crate::preview2::filesystem::{ + Descriptor, Dir, File, FileInputStream, FileOutputStream, OpenMode, ReaddirIterator, +}; use crate::preview2::{DirPerms, FilePerms, FsError, FsResult, WasiView}; use anyhow::Context; use wasmtime::component::Resource; @@ -130,10 +131,10 @@ impl HostDescriptor for T { Descriptor::File(f) => { let flags = f.spawn_blocking(|f| f.get_fd_flags()).await?; let mut flags = get_from_fdflags(flags); - if f.perms.contains(FilePerms::READ) { + if f.open_mode.contains(OpenMode::READ) { flags |= DescriptorFlags::READ; } - if f.perms.contains(FilePerms::WRITE) { + if f.open_mode.contains(OpenMode::WRITE) { flags |= DescriptorFlags::WRITE; } Ok(flags) @@ -141,10 +142,10 @@ impl HostDescriptor for T { Descriptor::Dir(d) => { let flags = d.spawn_blocking(|d| d.get_fd_flags()).await?; let mut flags = get_from_fdflags(flags); - if d.perms.contains(DirPerms::READ) { + if d.open_mode.contains(OpenMode::READ) { flags |= DescriptorFlags::READ; } - if d.perms.contains(DirPerms::MUTATE) { + if d.open_mode.contains(OpenMode::WRITE) { flags |= DescriptorFlags::MUTATE_DIRECTORY; } Ok(flags) @@ -507,28 +508,40 @@ impl HostDescriptor for T { } } + // Track whether we are creating file, for permission check: + let mut create = false; + // Track open mode, for permission check and recording in created descriptor: + let mut open_mode = OpenMode::empty(); + // Construct the OpenOptions to give the OS: let mut opts = cap_std::fs::OpenOptions::new(); opts.maybe_dir(true); - if oflags.contains(OpenFlags::CREATE | OpenFlags::EXCLUSIVE) { - opts.create_new(true); - opts.write(true); - } else if oflags.contains(OpenFlags::CREATE) { - opts.create(true); + if oflags.contains(OpenFlags::CREATE) { + if oflags.contains(OpenFlags::EXCLUSIVE) { + opts.create_new(true); + } else { + opts.create(true); + } + create = true; opts.write(true); + open_mode |= OpenMode::WRITE; } + if oflags.contains(OpenFlags::TRUNCATE) { opts.truncate(true); } if flags.contains(DescriptorFlags::READ) { opts.read(true); + open_mode |= OpenMode::READ; } if flags.contains(DescriptorFlags::WRITE) { opts.write(true); + open_mode |= OpenMode::WRITE; } else { // If not opened write, open read. This way the OS lets us open // the file, but we can use perms to reject use of the file later. opts.read(true); + open_mode |= OpenMode::READ; } if symlink_follow(path_flags) { opts.follow(FollowSymlinks::Yes); @@ -538,8 +551,8 @@ impl HostDescriptor for T { // These flags are not yet supported in cap-std: if flags.contains(DescriptorFlags::FILE_INTEGRITY_SYNC) - | flags.contains(DescriptorFlags::DATA_INTEGRITY_SYNC) - | flags.contains(DescriptorFlags::REQUESTED_WRITE_SYNC) + || flags.contains(DescriptorFlags::DATA_INTEGRITY_SYNC) + || flags.contains(DescriptorFlags::REQUESTED_WRITE_SYNC) { Err(ErrorCode::Unsupported)?; } @@ -553,6 +566,15 @@ impl HostDescriptor for T { } } + // Now enforce this WasiCtx's permissions before letting the OS have + // its shot: + if !d.perms.contains(DirPerms::MUTATE) && create { + Err(ErrorCode::NotPermitted)?; + } + if !d.file_perms.contains(FilePerms::WRITE) && open_mode.contains(OpenMode::WRITE) { + Err(ErrorCode::NotPermitted)?; + } + // Represents each possible outcome from the spawn_blocking operation. // This makes sure we don't have to give spawn_blocking any way to // manipulate the table. @@ -582,15 +604,17 @@ impl HostDescriptor for T { .await?; match opened { - OpenResult::Dir(dir) => { - Ok(table.push(Descriptor::Dir(Dir::new(dir, d.perms, d.file_perms)))?) - } - - OpenResult::File(file) => Ok(table.push(Descriptor::File(File::new( - file, - mask_file_perms(d.file_perms, flags), + OpenResult::Dir(dir) => Ok(table.push(Descriptor::Dir(Dir::new( + dir, + d.perms, + d.file_perms, + open_mode, )))?), + OpenResult::File(file) => { + Ok(table.push(Descriptor::File(File::new(file, d.file_perms, open_mode)))?) + } + OpenResult::NotDir => Err(ErrorCode::NotDirectory.into()), } } @@ -1040,18 +1064,6 @@ fn symlink_follow(path_flags: types::PathFlags) -> bool { path_flags.contains(types::PathFlags::SYMLINK_FOLLOW) } -fn mask_file_perms(p: FilePerms, flags: types::DescriptorFlags) -> FilePerms { - use types::DescriptorFlags; - let mut out = FilePerms::empty(); - if p.contains(FilePerms::READ) && flags.contains(DescriptorFlags::READ) { - out |= FilePerms::READ; - } - if p.contains(FilePerms::WRITE) && flags.contains(DescriptorFlags::WRITE) { - out |= FilePerms::WRITE; - } - out -} - #[cfg(test)] mod test { use super::*; From ecf8ecc9f695323bac75028019c3afa659020380 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Sun, 11 Feb 2024 10:40:53 -0800 Subject: [PATCH 4/8] different error on windows i guess? prtest:full --- crates/test-programs/src/bin/preview1_fd_filestat_set.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs index c550c4502c84..438e7cfbe045 100644 --- a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs +++ b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs @@ -53,6 +53,7 @@ unsafe fn test_fd_filestat_set_size_ro(dir_fd: wasi::Fd) { assert_errno!( wasi::fd_filestat_set_size(file_fd, 100) .expect_err("fd_filestat_set_size should error when file is opened read-only"), + windows => wasi::ERRNO_ACCES, wasi::ERRNO_INVAL ); From 848b67b680f0129dd03824e5246a5564b235a7e6 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 12 Feb 2024 11:04:18 -0800 Subject: [PATCH 5/8] apparently set-times of read-only is rejected on windows. just skip testing that --- .../test-programs/src/bin/preview1_fd_filestat_set.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs index 438e7cfbe045..d950d7df6d78 100644 --- a/crates/test-programs/src/bin/preview1_fd_filestat_set.rs +++ b/crates/test-programs/src/bin/preview1_fd_filestat_set.rs @@ -132,14 +132,18 @@ fn main() { }; // Run the tests. - // + unsafe { test_fd_filestat_set_size_rw(dir_fd) } unsafe { test_fd_filestat_set_size_ro(dir_fd) } - // + // The fd_filestat_set_times function should behave the same whether the file // descriptor is open for read-only or read-write, because the underlying // permissions of the file determine whether or not the filestat can be // set or not, not than the open mode. - unsafe { test_fd_filestat_set_times(dir_fd, wasi::RIGHTS_FD_READ) } + if test_programs::preview1::config().support_dangling_filesystem() { + // Guarding to run on non-windows filesystems. Windows rejects set-times on read-only + // files. + unsafe { test_fd_filestat_set_times(dir_fd, wasi::RIGHTS_FD_READ) } + } unsafe { test_fd_filestat_set_times(dir_fd, wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE) } } From 9be9282bba8de8ee5fbd52ee349018cc029e3c16 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 12 Feb 2024 11:35:37 -0800 Subject: [PATCH 6/8] path open read write: another alternate error code for windows --- crates/test-programs/src/bin/preview1_path_open_read_write.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/test-programs/src/bin/preview1_path_open_read_write.rs b/crates/test-programs/src/bin/preview1_path_open_read_write.rs index 3d19daffcbd9..eeede5be2146 100644 --- a/crates/test-programs/src/bin/preview1_path_open_read_write.rs +++ b/crates/test-programs/src/bin/preview1_path_open_read_write.rs @@ -34,6 +34,7 @@ unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) { wasi::fd_write(f_readonly, &[ciovec]) .err() .expect("read of writeonly fails"), + windows => wasi::ERRNO_PERM, wasi::ERRNO_BADF ); From 87a0b62eebd810a02c8fb00b11b929ad9493ca72 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 12 Feb 2024 12:01:12 -0800 Subject: [PATCH 7/8] wasi-common actually gives a badf, so accept either --- .../test-programs/src/bin/preview1_path_open_read_write.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/test-programs/src/bin/preview1_path_open_read_write.rs b/crates/test-programs/src/bin/preview1_path_open_read_write.rs index eeede5be2146..bdd80b508557 100644 --- a/crates/test-programs/src/bin/preview1_path_open_read_write.rs +++ b/crates/test-programs/src/bin/preview1_path_open_read_write.rs @@ -30,11 +30,14 @@ unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) { buf: write_buffer.as_ptr(), buf_len: write_buffer.len(), }; + // PERM is only the failure on windows under wasmtime-wasi. wasi-common + // fails on windows with BADF, so we cant use the `windows =>` syntax + // because that doesnt support alternatives like the agnostic syntax does. assert_errno!( wasi::fd_write(f_readonly, &[ciovec]) .err() .expect("read of writeonly fails"), - windows => wasi::ERRNO_PERM, + wasi::ERRNO_PERM, wasi::ERRNO_BADF ); From c3de888095b66ca6b350fd6ea0d8591e66de3415 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 12 Feb 2024 13:54:50 -0800 Subject: [PATCH 8/8] this case too --- crates/test-programs/src/bin/preview1_path_open_read_write.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/test-programs/src/bin/preview1_path_open_read_write.rs b/crates/test-programs/src/bin/preview1_path_open_read_write.rs index bdd80b508557..ccee4f5d626c 100644 --- a/crates/test-programs/src/bin/preview1_path_open_read_write.rs +++ b/crates/test-programs/src/bin/preview1_path_open_read_write.rs @@ -57,10 +57,12 @@ unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) { "writeonly has write right" ); + // See above for description of PERM assert_errno!( wasi::fd_read(f_writeonly, &[iovec]) .err() .expect("read of writeonly fails"), + wasi::ERRNO_PERM, wasi::ERRNO_BADF ); let bytes_written = wasi::fd_write(f_writeonly, &[ciovec]).expect("write to writeonly");