Abstract file access to use WasiPath trait

lib/wasi/src/state/mod.rs

@@ -69,7 +69,7 @@ pub enum Kind {
         handle: Option<Box<dyn WasiFile>>,
         /// The path on the host system where the file is located
         /// This is deprecated and will be removed in 0.7.0 or a shortly thereafter
-        path: PathBuf,
+        path: Box<dyn WasiPath>,
     },
     Dir {
         /// Parent directory
@@ -295,7 +295,7 @@ impl WasiFs {
 
                 let kind = Kind::File {
                     handle: Some(file),
-                    path: PathBuf::from(""),
+                    path: Box::new(PathBuf::from("")),
                 };
 
                 let inode = self
@@ -455,7 +455,7 @@ impl WasiFs {
                                 // load file
                                 Kind::File {
                                     handle: None,
-                                    path: file.clone(),
+                                    path: Box::new(file.clone()),
                                 }
                             } else if file_type.is_symlink() {
                                 let link_value = file.read_link().ok().ok_or(__WASI_EIO)?;
@@ -855,20 +855,31 @@ impl WasiFs {
 
     pub fn get_stat_for_kind(&self, kind: &Kind) -> Option<__wasi_filestat_t> {
         let md = match kind {
-            Kind::File { handle, path } => match handle {
+            Kind::File { path, .. } => {
-                Some(wf) => {
                 return Some(__wasi_filestat_t {
                     st_filetype: __WASI_FILETYPE_REGULAR_FILE,
-                        st_size: wf.size(),
+                    st_size: path.len().ok()?,
-                        st_atim: wf.last_accessed(),
+                    st_atim: path
-                        st_mtim: wf.last_modified(),
+                        .accessed()
-                        st_ctim: wf.created_time(),
+                        .ok()
-
+                        .and_then(|t| t.duration_since(SystemTime::UNIX_EPOCH).ok())
+                        .map(|t| t.as_nanos() as u64)
+                        .unwrap_or_default(),
+                    st_mtim: path
+                        .modified()
+                        .ok()
+                        .and_then(|t| t.duration_since(SystemTime::UNIX_EPOCH).ok())
+                        .map(|t| t.as_nanos() as u64)
+                        .unwrap_or_default(),
+                    st_ctim: path
+                        .created()
+                        .ok()
+                        .and_then(|t| t.duration_since(SystemTime::UNIX_EPOCH).ok())
+                        .map(|t| t.as_nanos() as u64)
+                        .unwrap_or_default(),
                     ..__wasi_filestat_t::default()
                 })
             }
-                None => path.metadata().ok()?,
-            },
             Kind::Dir { path, .. } => path.metadata().ok()?,
             Kind::Symlink {
                 base_po_dir,

lib/wasi/src/state/types.rs

@@ -148,26 +148,12 @@ pub trait WasiFile: std::fmt::Debug + Write + Read + Seek {
         panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0.  Please implement WasiFile::allocate for your type before then");
     }
 
-    /// Request deletion of the file
-    // TODO: break this out into a WasiPath trait which is dynamically in Kind::File
-    // this change can't be done until before release
-    fn unlink(&mut self) -> Result<(), WasiFsError> {
-        panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0.  Please implement WasiFile::unlink for your type before then");
-    }
-
     /// Store file contents and metadata to disk
     // TODO: stablize this in 0.7.0 by removing default impl
     fn sync_to_disk(&self) -> Result<(), WasiFsError> {
         panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0.  Please implement WasiFile::sync_to_disk for your type before then");
     }
 
-    /// Moves the file to a new location
-    /// NOTE: the signature of this function will change before stabilization
-    // TODO: stablizie this in 0.7.0 or 0.8.0 by removing default impl
-    fn rename_file(&self, _new_name: &std::path::Path) -> Result<(), WasiFsError> {
-        panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0 or 0.8.0.  Please implement WasiFile::rename_file for your type before then");
-    }
-
     /// Returns the number of bytes available.  This function must not block
     fn bytes_available(&self) -> Result<usize, WasiFsError> {
         panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0 or 0.8.0.  Please implement WasiFile::bytes_available for your type before then");
@@ -336,22 +322,167 @@ pub(crate) fn poll(
     unimplemented!("HostFile::poll in WasiFile is not implemented for non-Unix-like targets yet");
 }
 
-pub trait WasiPath {}
+#[derive(Debug)]
+pub struct WasiPathOpenOptions {
+    read: bool,
+    write: bool,
+    truncate: bool,
+    create: bool,
+    create_new: bool,
+}
+
+impl WasiPathOpenOptions {
+    pub fn new() -> Self {
+        Self {
+            read: true,
+            write: false,
+            truncate: false,
+            create: false,
+            create_new: false,
+        }
+    }
+
+    pub fn read(mut self, toggle: bool) -> Self {
+        self.read = toggle;
+        self
+    }
+
+    pub fn write(mut self, toggle: bool) -> Self {
+        self.write = toggle;
+        self
+    }
+
+    pub fn truncate(mut self, toggle: bool) -> Self {
+        self.truncate = toggle;
+        self
+    }
+
+    pub fn create(mut self, toggle: bool) -> Self {
+        self.create = toggle;
+        self
+    }
+
+    pub fn create_new(mut self, toggle: bool) -> Self {
+        self.create_new = toggle;
+        self
+    }
+
+    pub fn open(self, wasi_path: &dyn WasiPath) -> Result<Box<dyn WasiFile>, WasiFsError> {
+        wasi_path.open_with_options(
+            self.read,
+            self.write,
+            self.truncate,
+            self.create,
+            self.create_new,
+        )
+    }
+}
+
+pub trait WasiPath: std::fmt::Debug {
+    /// Get all entries in a directory.  Non-directories should return `WasiFsError::BaseNotDirectory`
+    fn read_dir(&self) -> Result<(), WasiFsError> {
+        Err(WasiFsError::BaseNotDirectory)
+    }
+
+    /// Create a directory at the given path
+    fn create_directory(&self) -> Result<(), WasiFsError>;
+
+    fn open_with_options(
+        &self,
+        read: bool,
+        write: bool,
+        truncate: bool,
+        create: bool,
+        create_new: bool,
+    ) -> Result<Box<dyn WasiFile>, WasiFsError>;
+
+    fn rename(&mut self, other: &dyn WasiPath) -> Result<(), WasiFsError>;
+
+    /// because issue with infinite recursion
+    fn path_exists(&self) -> bool;
+    fn as_string(&self) -> String;
+
+    fn remove_file(&self) -> Result<(), WasiFsError>;
+    fn remove_dir(&self) -> Result<(), WasiFsError>;
+
+    fn len(&self) -> Result<u64, WasiFsError>;
+    fn accessed(&self) -> Result<std::time::SystemTime, WasiFsError>;
+    fn modified(&self) -> Result<std::time::SystemTime, WasiFsError>;
+    fn created(&self) -> Result<std::time::SystemTime, WasiFsError>;
+}
+
+impl WasiPath for PathBuf {
+    fn create_directory(&self) -> Result<(), WasiFsError> {
+        std::fs::create_dir(self).map_err(Into::into)
+    }
+
+    fn open_with_options(
+        &self,
+        read: bool,
+        write: bool,
+        truncate: bool,
+        create: bool,
+        create_new: bool,
+    ) -> Result<Box<dyn WasiFile>, WasiFsError> {
+        std::fs::OpenOptions::new()
+            .read(read)
+            .write(write)
+            .truncate(truncate)
+            .create(create)
+            .create_new(create_new)
+            .open(&self)
+            .map_err(Into::into)
+            .map(|f| Box::new(HostFile::new(f)) as Box<dyn WasiFile>)
+    }
+
+    fn path_exists(&self) -> bool {
+        self.exists()
+    }
+
+    fn as_string(&self) -> String {
+        self.to_string_lossy().to_string()
+    }
+
+    fn rename(&mut self, other: &dyn WasiPath) -> Result<(), WasiFsError> {
+        std::fs::rename(self, other.as_string()).map_err(Into::into)
+    }
+
+    fn remove_file(&self) -> Result<(), WasiFsError> {
+        std::fs::remove_file(self).map_err(Into::into)
+    }
+    fn remove_dir(&self) -> Result<(), WasiFsError> {
+        std::fs::remove_dir(self).map_err(Into::into)
+    }
+    fn len(&self) -> Result<u64, WasiFsError> {
+        self.metadata().map(|md| md.len()).map_err(Into::into)
+    }
+    fn accessed(&self) -> Result<std::time::SystemTime, WasiFsError> {
+        self.metadata()
+            .and_then(|md| md.accessed())
+            .map_err(Into::into)
+    }
+    fn modified(&self) -> Result<std::time::SystemTime, WasiFsError> {
+        self.metadata()
+            .and_then(|md| md.modified())
+            .map_err(Into::into)
+    }
+    fn created(&self) -> Result<std::time::SystemTime, WasiFsError> {
+        self.metadata()
+            .and_then(|md| md.created())
+            .map_err(Into::into)
+    }
+}
 
 /// A thin wrapper around `std::fs::File`
 #[derive(Debug)]
 pub struct HostFile {
     pub inner: fs::File,
-    pub host_path: PathBuf,
 }
 
 impl HostFile {
     /// creates a new host file from a `std::fs::File` and a path
-    pub fn new(file: fs::File, host_path: PathBuf) -> Self {
+    pub fn new(file: fs::File) -> Self {
-        Self {
+        Self { inner: file }
-            inner: file,
-            host_path,
-        }
     }
 
     pub fn metadata(&self) -> fs::Metadata {
@@ -441,17 +572,10 @@ impl WasiFile for HostFile {
         fs::File::set_len(&self.inner, new_size).map_err(Into::into)
     }
 
-    fn unlink(&mut self) -> Result<(), WasiFsError> {
-        std::fs::remove_file(&self.host_path).map_err(Into::into)
-    }
     fn sync_to_disk(&self) -> Result<(), WasiFsError> {
         self.inner.sync_all().map_err(Into::into)
     }
 
-    fn rename_file(&self, new_name: &std::path::Path) -> Result<(), WasiFsError> {
-        std::fs::rename(&self.host_path, new_name).map_err(Into::into)
-    }
-
     #[cfg(unix)]
     fn bytes_available(&self) -> Result<usize, WasiFsError> {
         use std::os::unix::io::AsRawFd;

lib/wasi/src/syscalls/mod.rs

@@ -10,8 +10,8 @@ use crate::{
     ptr::{Array, WasmPtr},
     state::{
         self, host_file_type_to_wasi_file_type, iterate_poll_events, poll, Fd, HostFile, Inode,
-        InodeVal, Kind, PollEvent, PollEventBuilder, WasiFile, WasiFsError, WasiState,
+        InodeVal, Kind, PollEvent, PollEventBuilder, WasiFile, WasiFsError, WasiPath,
-        MAX_SYMLINKS,
+        WasiPathOpenOptions, WasiState, MAX_SYMLINKS,
     },
     ExitCode,
 };
@@ -1691,22 +1691,20 @@ pub fn path_open(
                     return __WASI_ENOTDIR;
                 }
                 if o_flags & __WASI_O_EXCL != 0 {
-                    if path.exists() {
+                    if path.path_exists() {
                         return __WASI_EEXIST;
                     }
                 }
-                let mut open_options = std::fs::OpenOptions::new();
+                let open_options = WasiPathOpenOptions::new()
-                let open_options = open_options
                     .read(true)
                     // TODO: ensure these rights are actually valid given parent, etc.
                     .write(adjusted_rights & __WASI_RIGHT_FD_WRITE != 0)
                     .create(o_flags & __WASI_O_CREAT != 0)
                     .truncate(o_flags & __WASI_O_TRUNC != 0);
 
-                *handle = Some(Box::new(HostFile::new(
+                *handle = Some(wasi_try!(open_options
-                    wasi_try!(open_options.open(&path).map_err(|_| __WASI_EIO)),
+                    .open(path.as_ref())
-                    path.to_path_buf(),
+                    .map_err(|e| e.into_wasi_err())));
-                )));
             }
             Kind::Buffer { .. } => unimplemented!("wasi::path_open for Buffer type files"),
             Kind::Dir { .. } | Kind::Root { .. } => {
@@ -1755,27 +1753,25 @@ pub fn path_open(
             // once we got the data we need from the parent, we lookup the host file
             // todo: extra check that opening with write access is okay
             let handle = {
-                let mut open_options = std::fs::OpenOptions::new();
+                let open_options = WasiPathOpenOptions::new()
-                let open_options = open_options
                     .read(true)
                     // TODO: ensure these rights are actually valid given parent, etc.
                     // write access is required for creating a file
                     .write(true)
                     .create_new(true);
 
-                Some(Box::new(HostFile::new(
+                Some(wasi_try!(open_options.open(&new_file_host_path).map_err(
-                    wasi_try!(open_options.open(&new_file_host_path).map_err(|e| {
+                    |e| {
-                        debug!("Error opening file {}", e);
+                        debug!("Error opening file {:?}", e);
                         __WASI_EIO
-                    })),
+                    }
-                    new_file_host_path.clone(),
+                )))
-                )) as Box<dyn WasiFile>)
             };
 
             let new_inode = {
                 let kind = Kind::File {
                     handle,
-                    path: new_file_host_path,
+                    path: Box::new(new_file_host_path),
                 };
                 wasi_try!(state.fs.create_inode(kind, false, new_entity_name.clone()))
             };
@@ -2013,15 +2009,11 @@ pub fn path_rename(
             handle,
             ref mut path,
         } => {
-            let result = if let Some(h) = handle {
+            let result = path
-                h.rename_file(&host_adjusted_target_path)
+                .rename(&host_adjusted_target_path as &dyn WasiPath)
-                    .map_err(|e| e.into_wasi_err())
+                .map_err(|e| e.into_wasi_err());
-            } else {
+            // TODO: double check the rollback code
-                let out =
+            *path = Box::new(host_adjusted_target_path) as Box<dyn WasiPath>;
-                    std::fs::rename(&path, &host_adjusted_target_path).map_err(|_| __WASI_EIO);
-                *path = host_adjusted_target_path;
-                out
-            };
             // if the above operation failed we have to revert the previous change and then fail
             if let Err(e) = result {
                 if let Kind::Dir { entries, .. } = &mut state.fs.inodes[source_parent_inode].kind {
@@ -2182,15 +2174,8 @@ pub fn path_unlink_file(
     state.fs.inodes[removed_inode].stat.st_nlink -= 1;
     if state.fs.inodes[removed_inode].stat.st_nlink == 0 {
         match &mut state.fs.inodes[removed_inode].kind {
-            Kind::File { handle, path } => {
+            Kind::File { path, .. } => {
-                if let Some(h) = handle {
+                wasi_try!(path.remove_file().map_err(WasiFsError::into_wasi_err))
-                    wasi_try!(h.unlink().map_err(WasiFsError::into_wasi_err));
-                } else {
-                    // File is closed
-                    // problem with the abstraction, we can't call unlink because there's no handle
-                    // TODO: replace this code in 0.7.0
-                    wasi_try!(std::fs::remove_file(path).map_err(|_| __WASI_EIO));
-                }
             }
             Kind::Dir { .. } | Kind::Root { .. } => return __WASI_EISDIR,
             Kind::Symlink { .. } => {