T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make Handle a trait required for any WASI-compatible handle (#1443)

Browse Source 
* Make Handle a trait required for any WASI-compatible handle

OK, so this PR is a bit of an experiment that came about somewhat itself
when I was looking at refactoring use of `Rc<RefCell<Descriptor>>` inside
`Entry` struct. I've noticed that since we've placed `VirtualFile` on the
same level as `OsHandle` and `Stdin` etc., we've ended up necessiitating
checks for different combinations such as "is a real OS resource being mixed
up with a virtual resource?", and if that was the case, we'd panic since
this was clearly not allowed (e.g., symlinking, or worse renaming).
Therefore, it seemed natural for virtual file to be on the same level
as _any_ OS handle (regardless of whether it's an actual file, socket,
or stdio handle). In other words, we should ideally envision the following
hierarchy:

```
\-- OsHandle \-- OsFile
              -- Stdio
\-- Virtual
```

This way, we can deal with the mix up at a level above which cleans up
our logic significantly.

On the other hand, when looking through the `virtfs`, the trait approach
to some type that's a valid `Handle` grew on me, and I think this
is the way to go. And this is what this PR is proposing, a trait
`Handle` which features enough functionality to make both virtual and
OS ops to work. Now, inside `Entry` we can safely store something like
`Rc<dyn Handle>` where `Handle` can downcast to either `VirtualFile` or
`VirtualDir`, or `OsHandle` if its an actual OS resource. Note that
I've left `Handle` as one massive trait, but I reckon we could split
it up into several smaller traits, each dealing with some bit of WASI
functionality. I'm hoping this would perhaps make it easier to figure
out polyfilling between snapshots and the new upcoming ephemeral
snapshot since a lot of boilerplate functionality is now done as part
of the `Handle` trait implementation.

Next, I've redone the original `OsHandle` to be an `OsFile` which
now stores a raw descriptor/handle (`RawFd`/`RawHandle`) inside a
`Cell` so that we can handle interior mutability in an easy (read,
non-panicky) way. In order not to lose the perks of derefercing to
`std::fs::File`, I've added a convenience trait `AsFile` which
will take `OsFile` by reference (or the stdio handles) and create
a non-owned `ManuallyDrop<File>` resource which can be passed around
and acted upon the way we'd normally do on `&File`. This change of
course implies that we now have to worry about properly closing all
OS resources stored as part of `OsFile`, thus this type now implements
`Drop` trait which essentially speaking moves the raw descriptor/handle
into a `File` and drops it.

Finally, I've redone setting time info on relative paths on *nix using
the same approach as advocated in the virtual fs. Namely, we do an
`openat` followed by `filestat_set_times` on the obtained descriptor.
This effectively removes the need for custom `filetime` module in
`yanix`. However, this does probably incur additional cost of at least
one additional syscall, and I haven't checked whether this approach
performs as expected on platforms such as NixOS which as far as I remember
had some weirdness todo with linking `utimensat` symbols, etc. Still,
this change is worth considering given that the implementation of
`path_filestat_set_times` cleans up a lot, albeit with some additional
cost.

* Fix tests on Windows

* Address comments plus minor consistency cleanup

* Address comments

* Fix formatting
This commit is contained in:
Jakub Konka
2020-04-09 22:18:19 +02:00
committed by GitHub
parent 6fd0451bc3
commit de919382b3
34 changed files with 1807 additions and 1964 deletions
Download Patch File Download Diff File Expand all files Collapse all files

343
crates/wasi-common/src/sys/windows/path.rs
View File

@@ -1,18 +1,83 @@
use crate::entry::{Descriptor, EntryRights};
use crate::fd;
use crate::path::PathGet;
use crate::sys::entry::OsHandle;
use super::oshandle::OsFile;
use crate::entry::EntryRights;
use crate::sys::oshandle::{AsFile, OsHandle};
use crate::wasi::{types, Errno, Result};
use log::debug;
use std::convert::TryInto;
use std::ffi::{OsStr, OsString};
use std::fs::{File, Metadata, OpenOptions};
use std::fs::{self, Metadata, OpenOptions};
use std::os::windows::ffi::{OsStrExt, OsStringExt};
use std::os::windows::fs::OpenOptionsExt;
use std::path::{Path, PathBuf};
use winapi::shared::winerror;
use winx::file::AccessMode;
fn strip_trailing_slashes_and_concatenate(dirfd: &OsFile, path: &str) -> Result<Option<PathBuf>> {
if path.ends_with('/') {
let suffix = path.trim_end_matches('/');
concatenate(dirfd, Path::new(suffix)).map(Some)
} else {
Ok(None)
}
}
fn strip_extended_prefix<P: AsRef<OsStr>>(path: P) -> OsString {
let path: Vec<u16> = path.as_ref().encode_wide().collect();
if &[92, 92, 63, 92] == &path[0..4] {
OsString::from_wide(&path[4..])
} else {
OsString::from_wide(&path)
}
}
fn concatenate<P: AsRef<Path>>(file: &OsFile, path: P) -> Result<PathBuf> {
use winx::file::get_file_path;
// WASI is not able to deal with absolute paths
// so error out if absolute
if path.as_ref().is_absolute() {
return Err(Errno::Notcapable);
}
let dir_path = get_file_path(&file.as_file())?;
// concatenate paths
let mut out_path = PathBuf::from(dir_path);
out_path.push(path.as_ref());
// strip extended prefix; otherwise we will error out on any relative
// components with `out_path`
let out_path = PathBuf::from(strip_extended_prefix(out_path));
log::debug!("out_path={:?}", out_path);
Ok(out_path)
}
fn file_access_mode_from_fdflags(fdflags: types::Fdflags, read: bool, write: bool) -> AccessMode {
let mut access_mode = AccessMode::READ_CONTROL;
// We always need `FILE_WRITE_ATTRIBUTES` so that we can set attributes such as filetimes, etc.
access_mode.insert(AccessMode::FILE_WRITE_ATTRIBUTES);
// Note that `GENERIC_READ` and `GENERIC_WRITE` cannot be used to properly support append-only mode
// The file-specific flags `FILE_GENERIC_READ` and `FILE_GENERIC_WRITE` are used here instead
// These flags have the same semantic meaning for file objects, but allow removal of specific permissions (see below)
if read {
access_mode.insert(AccessMode::FILE_GENERIC_READ);
}
if write {
access_mode.insert(AccessMode::FILE_GENERIC_WRITE);
}
// For append, grant the handle FILE_APPEND_DATA access but *not* FILE_WRITE_DATA.
// This makes the handle "append only".
// Changes to the file pointer will be ignored (like POSIX's O_APPEND behavior).
if fdflags.contains(&types::Fdflags::APPEND) {
access_mode.insert(AccessMode::FILE_APPEND_DATA);
access_mode.remove(AccessMode::FILE_WRITE_DATA);
}
access_mode
}
/// Creates owned WASI path from OS string.
///
/// NB WASI spec requires OS string to be valid UTF-8. Otherwise,
@@ -23,24 +88,6 @@ pub(crate) fn from_host<S: AsRef<OsStr>>(s: S) -> Result<String> {
Ok(s)
}
pub(crate) trait PathGetExt {
fn concatenate(&self) -> Result<PathBuf>;
}
impl PathGetExt for PathGet {
fn concatenate(&self) -> Result<PathBuf> {
match self.dirfd() {
Descriptor::OsHandle(file) => concatenate(file, Path::new(self.path())),
Descriptor::VirtualFile(_virt) => {
panic!("concatenate on a virtual base");
}
Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => {
unreachable!("streams do not have paths and should not be accessible via PathGet");
}
}
}
}
pub(crate) fn open_rights(
input_rights: &EntryRights,
oflags: types::Oflags,
@@ -69,30 +116,7 @@ pub(crate) fn open_rights(
EntryRights::new(needed_base, needed_inheriting)
}
pub(crate) fn openat(dirfd: &File, path: &str) -> Result<File> {
use std::fs::OpenOptions;
use std::os::windows::fs::OpenOptionsExt;
use winx::file::Flags;
let path = concatenate(dirfd, Path::new(path))?;
let err = match OpenOptions::new()
.read(true)
.custom_flags(Flags::FILE_FLAG_BACKUP_SEMANTICS.bits())
.open(&path)
{
Ok(file) => return Ok(file),
Err(e) => e,
};
if let Some(code) = err.raw_os_error() {
log::debug!("openat error={:?}", code);
if code as u32 == winerror::ERROR_INVALID_NAME {
return Err(Errno::Notdir);
}
}
Err(err.into())
}
pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> {
pub(crate) fn readlinkat(dirfd: &OsFile, s_path: &str) -> Result<String> {
use winx::file::get_file_path;
let path = concatenate(dirfd, Path::new(s_path))?;
@@ -102,7 +126,7 @@ pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> {
// we need to strip the prefix from the absolute path
// as otherwise we will error out since WASI is not capable
// of dealing with absolute paths
let dir_path = get_file_path(dirfd)?;
let dir_path = get_file_path(&dirfd.as_file())?;
let dir_path = PathBuf::from(strip_extended_prefix(dir_path));
let target_path = target_path
.strip_prefix(dir_path)
@@ -127,54 +151,25 @@ pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> {
Err(err.into())
}
fn strip_extended_prefix<P: AsRef<OsStr>>(path: P) -> OsString {
let path: Vec<u16> = path.as_ref().encode_wide().collect();
if &[92, 92, 63, 92] == &path[0..4] {
OsString::from_wide(&path[4..])
} else {
OsString::from_wide(&path)
}
}
fn concatenate<P: AsRef<Path>>(file: &File, path: P) -> Result<PathBuf> {
use winx::file::get_file_path;
// WASI is not able to deal with absolute paths
// so error out if absolute
if path.as_ref().is_absolute() {
return Err(Errno::Notcapable);
}
let dir_path = get_file_path(file)?;
// concatenate paths
let mut out_path = PathBuf::from(dir_path);
out_path.push(path.as_ref());
// strip extended prefix; otherwise we will error out on any relative
// components with `out_path`
let out_path = PathBuf::from(strip_extended_prefix(out_path));
log::debug!("out_path={:?}", out_path);
Ok(out_path)
}
pub(crate) fn create_directory(file: &File, path: &str) -> Result<()> {
pub(crate) fn create_directory(file: &OsFile, path: &str) -> Result<()> {
let path = concatenate(file, path)?;
std::fs::create_dir(&path)?;
Ok(())
}
pub(crate) fn link(
resolved_old: PathGet,
resolved_new: PathGet,
old_dirfd: &OsFile,
old_path: &str,
new_dirfd: &OsFile,
new_path: &str,
follow_symlinks: bool,
) -> Result<()> {
use std::fs;
let mut old_path = resolved_old.concatenate()?;
let new_path = resolved_new.concatenate()?;
let mut old_path = concatenate(old_dirfd, old_path)?;
let new_path = concatenate(new_dirfd, new_path)?;
if follow_symlinks {
// in particular, this will return an error if the target path doesn't exist
debug!("Following symlinks for path: {:?}", old_path);
log::debug!("Following symlinks for path: {:?}", old_path);
old_path = fs::canonicalize(&old_path).map_err(|e| match e.raw_os_error() {
// fs::canonicalize under Windows will return:
// * ERROR_FILE_NOT_FOUND, if it encounters a dangling symlink
@@ -183,39 +178,32 @@ pub(crate) fn link(
_ => e.into(),
})?;
}
fs::hard_link(&old_path, &new_path).or_else(|err| {
match err.raw_os_error() {
Some(code) => {
debug!("path_link at fs::hard_link error code={:?}", code);
match code as u32 {
winerror::ERROR_ACCESS_DENIED => {
// If an attempt is made to create a hard link to a directory, POSIX-compliant
// implementations of link return `EPERM`, but `ERROR_ACCESS_DENIED` is converted
// to `EACCES`. We detect and correct this case here.
if fs::metadata(&old_path).map(|m| m.is_dir()).unwrap_or(false) {
return Err(Errno::Perm);
}
}
_ => {}
}
Err(err.into())
}
None => {
log::debug!("Inconvertible OS error: {}", err);
Err(Errno::Io)
let err = match fs::hard_link(&old_path, &new_path) {
Ok(()) => return Ok(()),
Err(e) => e,
};
if let Some(code) = err.raw_os_error() {
log::debug!("path_link at fs::hard_link error code={:?}", code);
if code as u32 == winerror::ERROR_ACCESS_DENIED {
// If an attempt is made to create a hard link to a directory, POSIX-compliant
// implementations of link return `EPERM`, but `ERROR_ACCESS_DENIED` is converted
// to `EACCES`. We detect and correct this case here.
if fs::metadata(&old_path).map(|m| m.is_dir()).unwrap_or(false) {
return Err(Errno::Perm);
}
}
})
}
Err(err.into())
}
pub(crate) fn open(
resolved: PathGet,
dirfd: &OsFile,
path: &str,
read: bool,
write: bool,
oflags: types::Oflags,
fdflags: types::Fdflags,
) -> Result<Descriptor> {
) -> Result<OsHandle> {
use winx::file::{AccessMode, CreationDisposition, Flags};
let is_trunc = oflags.contains(&types::Oflags::TRUNC);
@@ -245,9 +233,7 @@ pub(crate) fn open(
}
_ => {}
}
let path = resolved.concatenate()?;
let path = concatenate(dirfd, path)?;
match path.symlink_metadata().map(|metadata| metadata.file_type()) {
Ok(file_type) => {
// check if we are trying to open a symlink
@@ -262,12 +248,18 @@ pub(crate) fn open(
Err(err) => match err.raw_os_error() {
Some(code) => {
log::debug!("path_open at symlink_metadata error code={:?}", code);
if code as u32 != winerror::ERROR_FILE_NOT_FOUND {
return Err(err.into());
}
// file not found, let it proceed to actually
// trying to open it
match code as u32 {
winerror::ERROR_FILE_NOT_FOUND => {
// file not found, let it proceed to actually
// trying to open it
}
winerror::ERROR_INVALID_NAME => {
// TODO rethink this. For now, migrate how we handled
// it in `path::openat` on Windows.
return Err(Errno::Notdir);
}
_ => return Err(err.into()),
};
}
None => {
log::debug!("Inconvertible OS error: {}", err);
@@ -284,49 +276,25 @@ pub(crate) fn open(
}
let flags: Flags = fdflags.into();
opts.access_mode(access_mode.bits())
let file = opts
.access_mode(access_mode.bits())
.custom_flags(flags.bits())
.open(&path)
.map(|f| OsHandle::from(f).into())
.map_err(Into::into)
.open(&path)?;
let handle = OsHandle::from(file);
Ok(handle)
}
fn file_access_mode_from_fdflags(fdflags: types::Fdflags, read: bool, write: bool) -> AccessMode {
let mut access_mode = AccessMode::READ_CONTROL;
// Note that `GENERIC_READ` and `GENERIC_WRITE` cannot be used to properly support append-only mode
// The file-specific flags `FILE_GENERIC_READ` and `FILE_GENERIC_WRITE` are used here instead
// These flags have the same semantic meaning for file objects, but allow removal of specific permissions (see below)
if read {
access_mode.insert(AccessMode::FILE_GENERIC_READ);
}
if write {
access_mode.insert(AccessMode::FILE_GENERIC_WRITE);
}
// For append, grant the handle FILE_APPEND_DATA access but *not* FILE_WRITE_DATA.
// This makes the handle "append only".
// Changes to the file pointer will be ignored (like POSIX's O_APPEND behavior).
if fdflags.contains(&types::Fdflags::APPEND) {
access_mode.insert(AccessMode::FILE_APPEND_DATA);
access_mode.remove(AccessMode::FILE_WRITE_DATA);
}
access_mode
}
pub(crate) fn readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize> {
pub(crate) fn readlink(dirfd: &OsFile, path: &str, buf: &mut [u8]) -> Result<usize> {
use winx::file::get_file_path;
let path = resolved.concatenate()?;
let path = concatenate(dirfd, path)?;
let target_path = path.read_link()?;
// since on Windows we are effectively emulating 'at' syscalls
// we need to strip the prefix from the absolute path
// as otherwise we will error out since WASI is not capable
// of dealing with absolute paths
let dir_path = get_file_path(&resolved.dirfd().as_os_handle())?;
let dir_path = get_file_path(&dirfd.as_file())?;
let dir_path = PathBuf::from(strip_extended_prefix(dir_path));
let target_path = target_path
.strip_prefix(dir_path)
@@ -353,20 +321,16 @@ pub(crate) fn readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize> {
}
}
fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result<Option<PathBuf>> {
if resolved.path().ends_with('/') {
let suffix = resolved.path().trim_end_matches('/');
concatenate(&resolved.dirfd().as_os_handle(), Path::new(suffix)).map(Some)
} else {
Ok(None)
}
}
pub(crate) fn rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> {
pub(crate) fn rename(
old_dirfd: &OsFile,
old_path_: &str,
new_dirfd: &OsFile,
new_path_: &str,
) -> Result<()> {
use std::fs;
let old_path = resolved_old.concatenate()?;
let new_path = resolved_new.concatenate()?;
let old_path = concatenate(old_dirfd, old_path_)?;
let new_path = concatenate(new_dirfd, new_path_)?;
// First sanity check: check we're not trying to rename dir to file or vice versa.
// NB on Windows, the former is actually permitted [std::fs::rename].
@@ -377,7 +341,7 @@ pub(crate) fn rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()>
}
// Second sanity check: check we're not trying to rename a file into a path
// ending in a trailing slash.
if old_path.is_file() && resolved_new.path().ends_with('/') {
if old_path.is_file() && new_path_.ends_with('/') {
return Err(Errno::Notdir);
}
@@ -406,7 +370,9 @@ pub(crate) fn rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()>
winerror::ERROR_INVALID_NAME => {
// If source contains trailing slashes, check if we are dealing with
// a file instead of a dir, and if so, throw ENOTDIR.
if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved_old)? {
if let Some(path) =
strip_trailing_slashes_and_concatenate(old_dirfd, old_path_)?
{
if path.is_file() {
return Err(Errno::Notdir);
}
@@ -424,38 +390,11 @@ pub(crate) fn rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()>
}
}
pub(crate) fn filestat_get(
resolved: PathGet,
_dirflags: types::Lookupflags,
) -> Result<types::Filestat> {
let path = resolved.concatenate()?;
let file = File::open(path)?;
let filestat = (&file).try_into()?;
Ok(filestat)
}
pub(crate) fn filestat_set_times(
resolved: PathGet,
_dirflags: types::Lookupflags,
st_atim: types::Timestamp,
st_mtim: types::Timestamp,
fst_flags: types::Fstflags,
) -> Result<()> {
use winx::file::AccessMode;
let path = resolved.concatenate()?;
let file = OpenOptions::new()
.access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits())
.open(path)?;
let modifiable_fd = Descriptor::OsHandle(OsHandle::from(file));
fd::filestat_set_times_impl(&modifiable_fd, st_atim, st_mtim, fst_flags)
}
pub(crate) fn symlink(old_path: &str, resolved: PathGet) -> Result<()> {
use std::fs;
pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path_: &str) -> Result<()> {
use std::os::windows::fs::{symlink_dir, symlink_file};
let old_path = concatenate(&resolved.dirfd().as_os_handle(), Path::new(old_path))?;
let new_path = resolved.concatenate()?;
let old_path = concatenate(new_dirfd, Path::new(old_path))?;
let new_path = concatenate(new_dirfd, new_path_)?;
// Windows distinguishes between file and directory symlinks.
// If the source doesn't exist or is an exotic file type, we fall back
@@ -488,7 +427,9 @@ pub(crate) fn symlink(old_path: &str, resolved: PathGet) -> Result<()> {
//
// Since POSIX will return EEXIST in such case, we simulate this behavior
winerror::ERROR_INVALID_NAME => {
if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved)? {
if let Some(path) =
strip_trailing_slashes_and_concatenate(new_dirfd, new_path_)?
{
if path.exists() {
return Err(Errno::Exist);
}
@@ -506,10 +447,10 @@ pub(crate) fn symlink(old_path: &str, resolved: PathGet) -> Result<()> {
}
}
pub(crate) fn unlink_file(resolved: PathGet) -> Result<()> {
pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> {
use std::fs;
let path = resolved.concatenate()?;
let path = concatenate(dirfd, path)?;
let file_type = path
.symlink_metadata()
.map(|metadata| metadata.file_type())?;
@@ -548,7 +489,7 @@ pub(crate) fn unlink_file(resolved: PathGet) -> Result<()> {
}
}
pub(crate) fn remove_directory(resolved: PathGet) -> Result<()> {
let path = resolved.concatenate()?;
pub(crate) fn remove_directory(dirfd: &OsFile, path: &str) -> Result<()> {
let path = concatenate(dirfd, path)?;
std::fs::remove_dir(&path).map_err(Into::into)
}