From 8a8553ae58f1dbe83babc7684d6ed1d80eac9f15 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Mon, 19 Aug 2019 13:33:33 -0700 Subject: [PATCH] Abstract file access to use WasiPath trait --- lib/wasi/src/state/mod.rs | 45 +++++---- lib/wasi/src/state/types.rs | 180 +++++++++++++++++++++++++++++------ lib/wasi/src/syscalls/mod.rs | 57 ++++------- 3 files changed, 201 insertions(+), 81 deletions(-) diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index dfcf52741..13e336518 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -69,7 +69,7 @@ pub enum Kind { handle: Option>, /// 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, }, 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 { - Some(wf) => { - return Some(__wasi_filestat_t { - st_filetype: __WASI_FILETYPE_REGULAR_FILE, - st_size: wf.size(), - st_atim: wf.last_accessed(), - st_mtim: wf.last_modified(), - st_ctim: wf.created_time(), - - ..__wasi_filestat_t::default() - }) - } - None => path.metadata().ok()?, - }, + Kind::File { path, .. } => { + return Some(__wasi_filestat_t { + st_filetype: __WASI_FILETYPE_REGULAR_FILE, + st_size: path.len().ok()?, + st_atim: path + .accessed() + .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() + }) + } Kind::Dir { path, .. } => path.metadata().ok()?, Kind::Symlink { base_po_dir, diff --git a/lib/wasi/src/state/types.rs b/lib/wasi/src/state/types.rs index 09dfe4247..1831ada33 100644 --- a/lib/wasi/src/state/types.rs +++ b/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 { 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, 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, 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; + fn accessed(&self) -> Result; + fn modified(&self) -> Result; + fn created(&self) -> Result; +} + +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, 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) + } + + 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 { + self.metadata().map(|md| md.len()).map_err(Into::into) + } + fn accessed(&self) -> Result { + self.metadata() + .and_then(|md| md.accessed()) + .map_err(Into::into) + } + fn modified(&self) -> Result { + self.metadata() + .and_then(|md| md.modified()) + .map_err(Into::into) + } + fn created(&self) -> Result { + 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 { - Self { - inner: file, - host_path, - } + pub fn new(file: fs::File) -> Self { + Self { inner: file } } 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 { use std::os::unix::io::AsRawFd; diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index dc8bebff4..a3f80b756 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/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, - MAX_SYMLINKS, + InodeVal, Kind, PollEvent, PollEventBuilder, WasiFile, WasiFsError, WasiPath, + 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 = open_options + let open_options = WasiPathOpenOptions::new() .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( - wasi_try!(open_options.open(&path).map_err(|_| __WASI_EIO)), - path.to_path_buf(), - ))); + *handle = Some(wasi_try!(open_options + .open(path.as_ref()) + .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 = open_options + let open_options = WasiPathOpenOptions::new() .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( - wasi_try!(open_options.open(&new_file_host_path).map_err(|e| { - debug!("Error opening file {}", e); + Some(wasi_try!(open_options.open(&new_file_host_path).map_err( + |e| { + debug!("Error opening file {:?}", e); __WASI_EIO - })), - new_file_host_path.clone(), - )) as Box) + } + ))) }; 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 { - h.rename_file(&host_adjusted_target_path) - .map_err(|e| e.into_wasi_err()) - } else { - let out = - std::fs::rename(&path, &host_adjusted_target_path).map_err(|_| __WASI_EIO); - *path = host_adjusted_target_path; - out - }; + let result = path + .rename(&host_adjusted_target_path as &dyn WasiPath) + .map_err(|e| e.into_wasi_err()); + // TODO: double check the rollback code + *path = Box::new(host_adjusted_target_path) as Box; // 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 } => { - if let Some(h) = handle { - 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::File { path, .. } => { + wasi_try!(path.remove_file().map_err(WasiFsError::into_wasi_err)) } Kind::Dir { .. } | Kind::Root { .. } => return __WASI_EISDIR, Kind::Symlink { .. } => {