From d33036a3b5cec8d72419b7737167f47090da1ecf Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 17 Sep 2019 23:03:31 +0200 Subject: [PATCH] Fix path_rename on *nix hosts The fix contains an errno remapping in macOS case where in case when we try to rename a file into a path with a trailing slash an ENOENT is returned. In this case, if the destination does not exist, an ENOTDIR should be thrown as is thrown correctly on Linux hosts. Thus, as a fix, if an ENOENT is thrown, an additional check is performed to see whether the destination path indeed contains a trailing slash, and if so, the errno is adjusted to ENOTDIR to match the POSIX/WASI spec. --- build.rs | 1 - src/hostcalls_impl/fs.rs | 7 ++-- src/sys/unix/bsd/hostcalls_impl.rs | 50 ++++++++++++++++++++++++++++ src/sys/unix/hostcalls_impl/fs.rs | 20 ----------- src/sys/unix/linux/hostcalls_impl.rs | 22 ++++++++++++ 5 files changed, 77 insertions(+), 23 deletions(-) diff --git a/build.rs b/build.rs index 777027b94a..caf01c8fc9 100644 --- a/build.rs +++ b/build.rs @@ -163,7 +163,6 @@ cfg_if::cfg_if! { fn ignore(testsuite: &str, name: &str) -> bool { if testsuite == "misc_testsuite" { match name { - "path_rename_trailing_slashes" => true, "path_symlink_trailing_slashes" => true, _ => false, } diff --git a/src/hostcalls_impl/fs.rs b/src/hostcalls_impl/fs.rs index e2d725871e..619ed8cc52 100644 --- a/src/hostcalls_impl/fs.rs +++ b/src/hostcalls_impl/fs.rs @@ -707,8 +707,11 @@ pub(crate) unsafe fn path_rename( let new_dirfd = wasi_ctx .get_fd_entry(new_dirfd, host::__WASI_RIGHT_PATH_RENAME_TARGET, 0) .and_then(|fe| fe.fd_object.descriptor.as_file())?; - let resolved_old = path_get(old_dirfd, 0, old_path, false)?; - let resolved_new = path_get(new_dirfd, 0, new_path, false)?; + let resolved_old = path_get(old_dirfd, 0, old_path, true)?; + let resolved_new = path_get(new_dirfd, 0, new_path, true)?; + + log::debug!("path_rename resolved_old={:?}", resolved_old); + log::debug!("path_rename resolved_new={:?}", resolved_new); hostcalls_impl::path_rename(resolved_old, resolved_new) } diff --git a/src/sys/unix/bsd/hostcalls_impl.rs b/src/sys/unix/bsd/hostcalls_impl.rs index 324e720bbd..b79e7c8a75 100644 --- a/src/sys/unix/bsd/hostcalls_impl.rs +++ b/src/sys/unix/bsd/hostcalls_impl.rs @@ -1,11 +1,61 @@ use super::osfile::OsFile; +use crate::hostcalls_impl::PathGet; use crate::sys::host_impl; use crate::{host, Error, Result}; use nix::libc::{self, c_long, c_void}; use std::convert::TryInto; +use std::ffi::CString; use std::fs::File; use std::os::unix::prelude::AsRawFd; +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)?; + let new_path_cstr = CString::new(resolved_new.path().as_bytes()).map_err(|_| Error::EILSEQ)?; + + let res = unsafe { + renameat( + resolved_old.dirfd().as_raw_fd(), + old_path_cstr.as_ptr(), + resolved_new.dirfd().as_raw_fd(), + new_path_cstr.as_ptr(), + ) + }; + if res != 0 { + // Currently, this is verified to be correct on macOS, where + // ENOENT can be returned in case when we try to rename a file + // into a name with a trailing slash. On macOS, if the latter does + // not exist, an ENOENT is thrown, whereas on Linux we observe the + // correct behaviour of throwing an ENOTDIR since the destination is + // indeed not a directory. + // + // TODO + // Verify on other BSD-based OSes. + match Errno::last() { + Errno::ENOENT => { + // check if the source path exists + if let Ok(_) = fstatat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + AtFlags::AT_SYMLINK_NOFOLLOW, + ) { + // check if destination contains a trailing slash + if resolved_new.path().contains('/') { + Err(Error::ENOTDIR) + } else { + Err(Error::ENOENT) + } + } else { + Err(Error::ENOENT) + } + } + x => Err(host_impl::errno_from_nix(x)), + } + } else { + Ok(()) + } +} + pub(crate) fn fd_readdir( os_file: &mut OsFile, host_buf: &mut [u8], diff --git a/src/sys/unix/hostcalls_impl/fs.rs b/src/sys/unix/hostcalls_impl/fs.rs index 2deb820559..f1a2bb1e5c 100644 --- a/src/sys/unix/hostcalls_impl/fs.rs +++ b/src/sys/unix/hostcalls_impl/fs.rs @@ -207,26 +207,6 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result } } -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)?; - let new_path_cstr = CString::new(resolved_new.path().as_bytes()).map_err(|_| Error::EILSEQ)?; - - let res = unsafe { - renameat( - resolved_old.dirfd().as_raw_fd(), - old_path_cstr.as_ptr(), - resolved_new.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 fd_filestat_get_impl(file: &std::fs::File) -> Result { use std::os::unix::fs::MetadataExt; diff --git a/src/sys/unix/linux/hostcalls_impl.rs b/src/sys/unix/linux/hostcalls_impl.rs index bd9508c8f8..632cb6917d 100644 --- a/src/sys/unix/linux/hostcalls_impl.rs +++ b/src/sys/unix/linux/hostcalls_impl.rs @@ -1,11 +1,33 @@ use super::osfile::OsFile; +use crate::hostcalls_impl::PathGet; use crate::sys::host_impl; use crate::{host, Error, Result}; use nix::libc::{self, c_long, c_void}; use std::convert::TryInto; +use std::ffi::CString; use std::fs::File; use std::os::unix::prelude::AsRawFd; +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)?; + let new_path_cstr = CString::new(resolved_new.path().as_bytes()).map_err(|_| Error::EILSEQ)?; + + let res = unsafe { + renameat( + resolved_old.dirfd().as_raw_fd(), + old_path_cstr.as_ptr(), + resolved_new.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 fd_readdir( os_file: &mut OsFile, host_buf: &mut [u8],