Browse Source

Fixes `path_symlink_trailing_slashes` test case (#125)

* Fixes `path_symlink_trailing_slashes` test case

This commit:
* adds a couple `log::debug!` macro calls in and around `path_get`
  for easier future debugging
* changes impl of `path_symlink` hostcall to actually *require*
  the final component (matching the impl of WASI in C)
* ignores the error `__WASI_ENOTDIR` in `path_get`'s `readlinkat` call
  which is not meant to be an error at this stage (i.e., this
  potentially erroneous condition *will be* handled later, in
  one of the layers above)

* Fixes `path_symlink_trailing` slashes on BSD-nixes

This commit:
* makes `path_symlink` host-specific (Linux and BSD-like nixes
  now have their own differing implementations)
* on BSD-like nixes, when `ENOTDIR` is returned from `symlinkat`
  it checks whether the target path contains a trailing slash,
  strips it, and then checks if the target path without the trailing
  slash exists; if yes, then converts the error code to `EEXIST` to
  match Linux/POSIX spec
pull/502/head
Jakub Konka 5 years ago
committed by GitHub
parent
commit
c3bf04042e
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 11
      build.rs
  2. 2
      src/hostcalls_impl/fs.rs
  3. 8
      src/hostcalls_impl/fs_helpers.rs
  4. 42
      src/sys/unix/bsd/hostcalls_impl.rs
  5. 20
      src/sys/unix/hostcalls_impl/fs.rs
  6. 4
      src/sys/unix/hostcalls_impl/fs_helpers.rs
  7. 23
      src/sys/unix/linux/hostcalls_impl.rs

11
build.rs

@ -170,15 +170,8 @@ mod wasm_tests {
cfg_if::cfg_if! {
if #[cfg(not(windows))] {
/// Ignore tests that aren't supported yet.
fn ignore(testsuite: &str, name: &str) -> bool {
if testsuite == "misc_testsuite" {
match name {
"path_symlink_trailing_slashes" => true,
_ => false,
}
} else {
unreachable!()
}
fn ignore(_testsuite: &str, _name: &str) -> bool {
false
}
} else {
/// Ignore tests that aren't supported yet.

2
src/hostcalls_impl/fs.rs

@ -927,7 +927,7 @@ pub(crate) unsafe fn path_symlink(
let dirfd = wasi_ctx
.get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_SYMLINK, 0)
.and_then(|fe| fe.fd_object.descriptor.as_file())?;
let resolved_new = path_get(dirfd, 0, new_path, false)?;
let resolved_new = path_get(dirfd, 0, new_path, true)?;
hostcalls_impl::path_symlink(old_path, resolved_new)
}

8
src/hostcalls_impl/fs_helpers.rs

@ -57,7 +57,7 @@ pub(crate) fn path_get(
loop {
match path_stack.pop() {
Some(cur_path) => {
log::debug!("cur_path = {:?}", cur_path);
log::debug!("path_get cur_path = {:?}", cur_path);
let ends_with_slash = cur_path.ends_with('/');
let mut components = Path::new(&cur_path).components();
@ -75,6 +75,8 @@ pub(crate) fn path_get(
path_stack.push(tail);
}
log::debug!("path_get path_stack = {:?}", path_stack);
match head {
Component::Prefix(_) | Component::RootDir => {
// path is absolute!
@ -169,6 +171,10 @@ pub(crate) fn path_get(
Err(e) => {
if e.as_wasi_errno() != host::__WASI_EINVAL
&& e.as_wasi_errno() != host::__WASI_ENOENT
// this handles the cases when trying to link to
// a destination that already exists, and the target
// path contains a slash
&& e.as_wasi_errno() != host::__WASI_ENOTDIR
{
return Err(e);
}

42
src/sys/unix/bsd/hostcalls_impl.rs

@ -49,6 +49,48 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
}
}
pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> {
use nix::{errno::Errno, fcntl::AtFlags, libc::symlinkat, sys::stat::fstatat};
let old_path_cstr = CString::new(old_path.as_bytes()).map_err(|_| Error::EILSEQ)?;
let new_path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?;
log::debug!("path_symlink old_path = {:?}", old_path);
log::debug!("path_symlink resolved = {:?}", resolved);
let res = unsafe {
symlinkat(
old_path_cstr.as_ptr(),
resolved.dirfd().as_raw_fd(),
new_path_cstr.as_ptr(),
)
};
if res != 0 {
match Errno::last() {
Errno::ENOTDIR => {
// On BSD, symlinkat returns ENOTDIR when it should in fact
// return a EEXIST. It seems that it gets confused with by
// the trailing slash in the target path. Thus, we strip
// the trailing slash and check if the path exists, and
// adjust the error code appropriately.
let new_path = resolved.path().trim_end_matches('/');
if let Ok(_) = fstatat(
resolved.dirfd().as_raw_fd(),
new_path,
AtFlags::AT_SYMLINK_NOFOLLOW,
) {
Err(Error::EEXIST)
} else {
Err(Error::ENOTDIR)
}
}
x => Err(host_impl::errno_from_nix(x)),
}
} else {
Ok(())
}
}
pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> {
use nix::{errno::Errno, fcntl::AtFlags, libc::renameat, sys::stat::fstatat};
let old_path_cstr = CString::new(resolved_old.path().as_bytes()).map_err(|_| Error::EILSEQ)?;

20
src/sys/unix/hostcalls_impl/fs.rs

@ -337,26 +337,6 @@ pub(crate) fn path_filestat_set_times(
utimensat(fd, resolved.path(), &atim, &mtim, atflags).map_err(Into::into)
}
pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> {
use nix::libc::symlinkat;
let old_path_cstr = CString::new(old_path.as_bytes()).map_err(|_| Error::EILSEQ)?;
let new_path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?;
let res = unsafe {
symlinkat(
old_path_cstr.as_ptr(),
resolved.dirfd().as_raw_fd(),
new_path_cstr.as_ptr(),
)
};
if res != 0 {
Err(host_impl::errno_from_nix(nix::errno::Errno::last()))
} else {
Ok(())
}
}
pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> {
use nix::errno;
use nix::libc::{unlinkat, AT_REMOVEDIR};

4
src/sys/unix/hostcalls_impl/fs_helpers.rs

@ -57,6 +57,8 @@ pub(crate) fn openat(dirfd: &File, path: &str) -> Result<File> {
use nix::sys::stat::Mode;
use std::os::unix::prelude::{AsRawFd, FromRawFd};
log::debug!("path_get openat path = {:?}", path);
fcntl::openat(
dirfd.as_raw_fd(),
path,
@ -71,6 +73,8 @@ pub(crate) fn readlinkat(dirfd: &File, path: &str) -> Result<String> {
use nix::fcntl;
use std::os::unix::prelude::AsRawFd;
log::debug!("path_get readlinkat path = {:?}", path);
let readlink_buf = &mut [0u8; libc::PATH_MAX as usize + 1];
fcntl::readlinkat(dirfd.as_raw_fd(), path, readlink_buf)

23
src/sys/unix/linux/hostcalls_impl.rs

@ -24,6 +24,29 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
}
}
pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> {
use nix::{errno::Errno, libc::symlinkat};
let old_path_cstr = CString::new(old_path.as_bytes()).map_err(|_| Error::EILSEQ)?;
let new_path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?;
log::debug!("path_symlink old_path = {:?}", old_path);
log::debug!("path_symlink resolved = {:?}", resolved);
let res = unsafe {
symlinkat(
old_path_cstr.as_ptr(),
resolved.dirfd().as_raw_fd(),
new_path_cstr.as_ptr(),
)
};
if res != 0 {
Err(host_impl::errno_from_nix(Errno::last()))
} else {
Ok(())
}
}
pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> {
use nix::libc::renameat;
let old_path_cstr = CString::new(resolved_old.path().as_bytes()).map_err(|_| Error::EILSEQ)?;

Loading…
Cancel
Save