Merge #1764

1764: Fix issue with `path_rename` moving down 1 directory r=MarkMcCaskey a=MarkMcCaskey

Resolves #1759 

Due to file metadata caching in WasiFS this behavior isn't easily observable with tests because as far as the rest of the WASI calls are concerned the file is in the right place:

perhaps we should consider removing some of that caching, but it's a large change and may have other implications...
Ideally we do want to not be dependent on the file system when dealing with open files... the issue here is that when we rename it, we still consider it to be a file that WasiFS is actively managing. This maybe relatively easy to fix on its own

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>

CHANGELOG.md

@@ -15,6 +15,8 @@
 
 ### Fixed
 
+- [#1764](https://github.com/wasmerio/wasmer/pull/1764) Fix bug in WASI `path_rename` allowing renamed files to be 1 directory below a preopened directory.
+
 ## 1.0.0-alpha5 - 2020-11-06
 
 ### Added

lib/wasi/src/syscalls/mod.rs

@@ -2030,12 +2030,16 @@ pub fn path_rename(
     new_path: WasmPtr<u8, Array>,
     new_path_len: u32,
 ) -> __wasi_errno_t {
-    debug!("wasi::path_rename");
+    debug!(
+        "wasi::path_rename: old_fd = {}, new_fd = {}",
+        old_fd, new_fd
+    );
     let (memory, mut state) = env.get_memory_and_wasi_state(0);
     let source_str = get_input_str!(memory, old_path, old_path_len);
     let source_path = std::path::Path::new(source_str);
     let target_str = get_input_str!(memory, new_path, new_path_len);
     let target_path = std::path::Path::new(target_str);
+    debug!("=> rename from {} to {}", source_str, target_str);
 
     {
         let source_fd = wasi_try!(state.fs.get_fd(old_fd));
@@ -2059,8 +2063,6 @@ pub fn path_rename(
                 return __WASI_EEXIST;
             }
             let mut out_path = path.clone();
-            // remove fd's own name which will be double counted
-            out_path.pop();
             out_path.push(target_path);
             out_path
         }

tests/wasi-wast/README.md

@@ -7,7 +7,7 @@ WASI test files with expected output in a custom WAST format.
 In order to run the test generator properly you will need:
 
 - `rustup` installed and on your PATH
-- `wasm-opt` and `wasm-strip` from `wabt` installed and on your PATH
+- `wasm-opt` from `binaryen` and `wasm-strip` from `wabt` are installed and on your PATH
 
 ## Usage
 

tests/wasi-wast/wasi/snapshot1/path_rename.wast

@@ -3,5 +3,5 @@
 (wasi_test "path_rename.wasm"
   (temp_dirs "temp")
   (assert_return (i64.const 0))
-  (assert_stdout "The original file does not still exist!\n柴犬\n")
+  (assert_stdout "The original file does not still exist!\nFound item: path_renamed_file.txt\n柴犬\n")
 )

tests/wasi-wast/wasi/tests/path_rename.rs

@@ -43,6 +43,25 @@ fn main() {
         println!("The original file does not still exist!");
     }
 
+    if !file_to_rename_to.exists() {
+        println!("The moved file does not exist!");
+        return;
+    }
+
+    // TODO: add temp directory suport for native execution...
+    // until then, don't actually inspect the directory when running native code.
+    #[cfg(target_os = "wasi")]
+    for item in fs::read_dir(&base).unwrap() {
+        println!(
+            "Found item: {}",
+            item.unwrap().path().file_name().unwrap().to_str().unwrap()
+        );
+    }
+    #[cfg(not(target_os = "wasi"))]
+    {
+        println!("Found item: path_renamed_file.txt");
+    }
+
     let mut out_str = String::new();
     file.read_to_string(&mut out_str).unwrap();
     let mut test_str = String::new();

tests/wasi-wast/wasi/unstable/path_rename.wast

@@ -3,5 +3,5 @@
 (wasi_test "path_rename.wasm"
   (temp_dirs "temp")
   (assert_return (i64.const 0))
-  (assert_stdout "The original file does not still exist!\n柴犬\n")
+  (assert_stdout "The original file does not still exist!\nFound item: path_renamed_file.txt\n柴犬\n")
 )