Merge #939

939: Fix WASI append bug, add test r=MarkMcCaskey a=MarkMcCaskey

resolves #936 

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

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## **[Unreleased]**
 
+- [#936](https://github.com/wasmerio/wasmer/pull/939) Fix bug causing attempts to append to files with WASI to delete the contents of the file
 - [#940](https://github.com/wasmerio/wasmer/pull/940) Update supported Rust version to 1.38+
 - [#921](https://github.com/wasmerio/wasmer/pull/921) In LLVM backend, annotate all memory accesses with TBAA metadata.
 - [#883](https://github.com/wasmerio/wasmer/pull/883) Allow floating point operations to have arbitrary inputs, even including SNaNs.

lib/wasi-tests/build/wasitests.rs

@@ -83,7 +83,7 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
         .arg("+nightly")
         .arg("--target=wasm32-wasi")
         .arg("-C")
-        .arg("opt-level=s")
+        .arg("opt-level=z")
         .arg(file)
         .arg("-o")
         .arg(&wasm_out_name)

lib/wasi-tests/tests/wasitests/fd_append.rs

@@ -0,0 +1,18 @@
+// !!! THIS IS A GENERATED FILE !!!
+// ANY MANUAL EDITS MAY BE OVERWRITTEN AT ANY TIME
+// Files autogenerated with cargo build (build/wasitests.rs).
+
+#[test]
+fn test_fd_append() {
+    assert_wasi_output!(
+        "../../wasitests/fd_append.wasm",
+        "fd_append",
+        vec![],
+        vec![(
+            ".".to_string(),
+            ::std::path::PathBuf::from("wasitests/test_fs/temp")
+        ),],
+        vec![],
+        "../../wasitests/fd_append.out"
+    );
+}

lib/wasi-tests/tests/wasitests/mod.rs

@@ -9,6 +9,7 @@ mod close_preopen_fd;
 mod create_dir;
 mod envvar;
 mod fd_allocate;
+mod fd_append;
 mod fd_close;
 mod fd_pread;
 mod fd_read;

lib/wasi-tests/wasitests/fd_append.out

@@ -0,0 +1 @@
+"Hello, world!\nGoodbye, world!\n"

lib/wasi-tests/wasitests/fd_append.rs

@@ -0,0 +1,48 @@
+// Args:
+// mapdir: .:wasitests/test_fs/temp
+
+use std::fs::OpenOptions;
+use std::io::{Read, Write};
+use std::path::PathBuf;
+
+static STR1: &str = "Hello, world!\n";
+static STR2: &str = "Goodbye, world!\n";
+
+fn main() {
+    let file = {
+        #[cfg(not(target_os = "wasi"))]
+        let mut base = PathBuf::from("wasitests/test_fs/temp");
+        #[cfg(target_os = "wasi")]
+        let mut base = PathBuf::from(".");
+
+        base.push("fd_append_test");
+        base
+    };
+
+    {
+        let mut file_handle = OpenOptions::new()
+            .create_new(true)
+            .append(true)
+            .open(&file)
+            .expect("Couldn't create file");
+        file_handle.write(STR1.as_bytes()).unwrap();
+    }
+    {
+        let mut file_handle = OpenOptions::new()
+            .append(true)
+            .open(&file)
+            .expect("Couldn't reopen file to append");
+        file_handle.write(STR2.as_bytes()).unwrap();
+    }
+
+    let mut file_handle = OpenOptions::new()
+        .read(true)
+        .open(&file)
+        .expect("Couldn't reopen file to read");
+
+    let mut test = String::new();
+    file_handle.read_to_string(&mut test);
+
+    assert_eq!(&test, &format!("{}{}", STR1, STR2));
+    println!("{:?}", &test);
+}

lib/wasi/src/syscalls/mod.rs

@@ -1724,12 +1724,25 @@ pub fn path_open(
                     }
                 }
                 let mut open_options = std::fs::OpenOptions::new();
+                let write_permission = adjusted_rights & __WASI_RIGHT_FD_WRITE != 0;
+                // append, truncate, and create all require the permission to write
+                let (append_permission, truncate_permission, create_permission) =
+                    if write_permission {
+                        (
+                            fs_flags & __WASI_FDFLAG_APPEND != 0,
+                            o_flags & __WASI_O_TRUNC != 0,
+                            o_flags & __WASI_O_CREAT != 0,
+                        )
+                    } else {
+                        (false, false, false)
+                    };
                 let open_options = open_options
                     .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);
+                    .write(write_permission)
+                    .create(create_permission)
+                    .append(append_permission)
+                    .truncate(truncate_permission);
                 open_flags |= Fd::READ;
                 if adjusted_rights & __WASI_RIGHT_FD_WRITE != 0 {
                     open_flags |= Fd::WRITE;
@@ -1798,6 +1811,7 @@ pub fn path_open(
                 let mut open_options = std::fs::OpenOptions::new();
                 let open_options = open_options
                     .read(true)
+                    .append(fs_flags & __WASI_FDFLAG_APPEND != 0)
                     // TODO: ensure these rights are actually valid given parent, etc.
                     // write access is required for creating a file
                     .write(true)