Merge #850

850: Add builder API for WasiState r=MarkMcCaskey a=MarkMcCaskey

Nicer to use and it checks for errors!

# Review

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


Co-authored-by: Mark McCaskey <mark@wasmer.io>
Co-authored-by: Mark McCaskey <markmccaskey@users.noreply.github.com>

CHANGELOG.md

@@ -6,6 +6,7 @@ Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
 
+- [#850](https://github.com/wasmerio/wasmer/pull/850) New `WasiStateBuilder` API. small, add misc. breaking changes to existing API (for example, changing the preopen dirs arg on `wasi::generate_import_object` from `Vec<String>` to `Vec<Pathbuf>`)
 - [#852](https://github.com/wasmerio/wasmer/pull/852) Make minor grammar/capitalization fixes to README.md
 - [#841](https://github.com/wasmerio/wasmer/pull/841) Slightly improve rustdoc documentation and small updates to outdated info in readme files
 - [#835](https://github.com/wasmerio/wasmer/pull/836) Update Cranelift fork version to `0.44.0`

lib/wasi-tests/build/wasitests.rs

@@ -145,7 +145,7 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
         out_str.push_str("vec![");
 
         for entry in args.po_dirs {
-            out_str.push_str(&format!("\"{}\".to_string(),", entry));
+            out_str.push_str(&format!("std::path::PathBuf::from(\"{}\"),", entry));
         }
 
         out_str.push_str("]");

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

@@ -7,7 +7,7 @@ fn test_create_dir() {
     assert_wasi_output!(
         "../../wasitests/create_dir.wasm",
         "create_dir",
-        vec![".".to_string(),],
+        vec![std::path::PathBuf::from("."),],
         vec![],
         vec![],
         "../../wasitests/create_dir.out"

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

@@ -7,7 +7,7 @@ fn test_file_metadata() {
     assert_wasi_output!(
         "../../wasitests/file_metadata.wasm",
         "file_metadata",
-        vec![".".to_string(),],
+        vec![std::path::PathBuf::from("."),],
         vec![],
         vec![],
         "../../wasitests/file_metadata.out"

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

@@ -7,7 +7,7 @@ fn test_quine() {
     assert_wasi_output!(
         "../../wasitests/quine.wasm",
         "quine",
-        vec![".".to_string(),],
+        vec![std::path::PathBuf::from("."),],
         vec![],
         vec![],
         "../../wasitests/quine.out"

lib/wasi/src/lib.rs

@@ -47,11 +47,11 @@ pub struct ExitCode {
     pub code: syscalls::types::__wasi_exitcode_t,
 }
 
-/// Creates a WasiImport object with `WasiState`.
+/// Creates a Wasi [`ImportObject`] with [`WasiState`].
 pub fn generate_import_object(
     args: Vec<Vec<u8>>,
     envs: Vec<Vec<u8>>,
-    preopened_files: Vec<String>,
+    preopened_files: Vec<PathBuf>,
     mapped_dirs: Vec<(String, PathBuf)>,
 ) -> ImportObject {
     let state_gen = move || {
@@ -63,6 +63,7 @@ pub fn generate_import_object(
         }
         let preopened_files = preopened_files.clone();
         let mapped_dirs = mapped_dirs.clone();
+        //let wasi_builder = create_wasi_instance();
 
         let state = Box::new(WasiState {
             fs: WasiFs::new(&preopened_files, &mapped_dirs).expect("Could not create WASI FS"),

lib/wasi/src/state/builder.rs

@@ -0,0 +1,289 @@
+//! Builder code for [`WasiState`]
+
+use crate::state::{WasiFs, WasiState};
+use std::path::{Path, PathBuf};
+
+/// Creates an empty [`WasiStateBuilder`].
+pub(crate) fn create_wasi_state(program_name: &str) -> WasiStateBuilder {
+    WasiStateBuilder {
+        args: vec![program_name.bytes().collect()],
+        ..WasiStateBuilder::default()
+    }
+}
+
+/// Type for building an instance of [`WasiState`]
+#[derive(Debug, Default, Clone, PartialEq)]
+pub struct WasiStateBuilder {
+    args: Vec<Vec<u8>>,
+    envs: Vec<Vec<u8>>,
+    preopened_files: Vec<PathBuf>,
+    mapped_dirs: Vec<(String, PathBuf)>,
+}
+
+/// Error type returned when bad data is given to [`WasiStateBuilder`].
+#[derive(Debug, PartialEq, Eq)]
+pub enum WasiStateCreationError {
+    EnvironmentVariableFormatError(String),
+    ArgumentContainsNulByte(String),
+    PreopenedDirectoryNotFound(PathBuf),
+    MappedDirAliasFormattingError(String),
+    WasiFsCreationError(String),
+}
+
+fn validate_mapped_dir_alias(alias: &str) -> Result<(), WasiStateCreationError> {
+    for byte in alias.bytes() {
+        match byte {
+            b'/' => {
+                return Err(WasiStateCreationError::MappedDirAliasFormattingError(
+                    format!("Alias \"{}\" contains the character '/'", alias),
+                ));
+            }
+            b'\0' => {
+                return Err(WasiStateCreationError::MappedDirAliasFormattingError(
+                    format!("Alias \"{}\" contains a nul byte", alias),
+                ));
+            }
+            _ => (),
+        }
+    }
+
+    Ok(())
+}
+
+// TODO add other WasiFS APIs here like swapping out stdout, for example (though we need to
+// return stdout somehow, it's unclear what that API should look like)
+impl WasiStateBuilder {
+    /// Add an environment variable pair.
+    /// Environment variable keys and values must not contain the byte `=` (0x3d)
+    /// or nul (0x0).
+    pub fn env<Key, Value>(&mut self, key: Key, value: Value) -> &mut Self
+    where
+        Key: AsRef<[u8]>,
+        Value: AsRef<[u8]>,
+    {
+        let key_b = key.as_ref();
+        let val_b = value.as_ref();
+
+        let length = key_b.len() + val_b.len() + 1;
+        let mut byte_vec = Vec::with_capacity(length);
+
+        byte_vec.extend_from_slice(&key_b);
+        byte_vec.push(b'=');
+        byte_vec.extend_from_slice(&val_b);
+
+        self.envs.push(byte_vec);
+
+        self
+    }
+
+    /// Add an argument.
+    /// Arguments must not contain the nul (0x0) byte
+    pub fn arg<Arg>(&mut self, arg: Arg) -> &mut Self
+    where
+        Arg: AsRef<[u8]>,
+    {
+        let arg_b = arg.as_ref();
+        let mut byte_vec = Vec::with_capacity(arg_b.len());
+        byte_vec.extend_from_slice(&arg_b);
+        self.args.push(byte_vec);
+
+        self
+    }
+
+    /// Add multiple environment variable pairs.
+    /// Keys and values must not contain the `=` (0x3d) or nul (0x0) byte.
+    pub fn envs<I, Key, Value>(&mut self, env_pairs: I) -> &mut Self
+    where
+        I: IntoIterator<Item = (Key, Value)>,
+        Key: AsRef<[u8]>,
+        Value: AsRef<[u8]>,
+    {
+        for (key, value) in env_pairs {
+            let key_b = key.as_ref();
+            let val_b = value.as_ref();
+
+            let length = key_b.len() + val_b.len() + 1;
+            let mut byte_vec = Vec::with_capacity(length);
+
+            byte_vec.extend_from_slice(&key_b);
+            byte_vec.push(b'=');
+            byte_vec.extend_from_slice(&val_b);
+
+            self.envs.push(byte_vec);
+        }
+
+        self
+    }
+
+    /// Add multiple arguments.
+    /// Arguments must not contain the nul (0x0) byte
+    pub fn args<I, Arg>(&mut self, args: I) -> &mut Self
+    where
+        I: IntoIterator<Item = Arg>,
+        Arg: AsRef<[u8]>,
+    {
+        for arg in args {
+            let arg_b = arg.as_ref();
+            let mut byte_vec = Vec::with_capacity(arg_b.len());
+            byte_vec.extend_from_slice(&arg_b);
+            self.args.push(byte_vec);
+        }
+
+        self
+    }
+
+    /// Preopen a directory
+    /// This opens the given directory at the virtual root, `/`, and allows
+    /// the WASI module to read and write to the given directory.
+    // TODO: design a simple API for passing in permissions here (i.e. read-only)
+    pub fn preopen_dir<FilePath>(&mut self, po_dir: FilePath) -> &mut Self
+    where
+        FilePath: AsRef<Path>,
+    {
+        let path = po_dir.as_ref();
+        self.preopened_files.push(path.to_path_buf());
+
+        self
+    }
+
+    /// Preopen a directory
+    /// This opens the given directory at the virtual root, `/`, and allows
+    /// the WASI module to read and write to the given directory.
+    pub fn preopen_dirs<I, FilePath>(&mut self, po_dirs: I) -> &mut Self
+    where
+        I: IntoIterator<Item = FilePath>,
+        FilePath: AsRef<Path>,
+    {
+        for po_dir in po_dirs {
+            let path = po_dir.as_ref();
+            self.preopened_files.push(path.to_path_buf());
+        }
+
+        self
+    }
+
+    /// Preopen a directory with a different name exposed to the WASI.
+    pub fn map_dir<FilePath>(&mut self, alias: &str, po_dir: FilePath) -> &mut Self
+    where
+        FilePath: AsRef<Path>,
+    {
+        let path = po_dir.as_ref();
+        self.mapped_dirs
+            .push((alias.to_string(), path.to_path_buf()));
+
+        self
+    }
+
+    /// Consumes the [`WasiStateBuilder`] and produces a [`WasiState`]
+    ///
+    /// Returns the error from `WasiFs::new` if there's an error
+    pub fn build(&mut self) -> Result<WasiState, WasiStateCreationError> {
+        for (i, arg) in self.args.iter().enumerate() {
+            for b in arg.iter() {
+                if *b == 0 {
+                    return Err(WasiStateCreationError::ArgumentContainsNulByte(
+                        std::str::from_utf8(arg)
+                            .unwrap_or(if i == 0 {
+                                "Inner error: program name is invalid utf8!"
+                            } else {
+                                "Inner error: arg is invalid utf8!"
+                            })
+                            .to_string(),
+                    ));
+                }
+            }
+        }
+        for env in self.envs.iter() {
+            let mut eq_seen = false;
+            for b in env.iter() {
+                match *b {
+                    b'=' => {
+                        if eq_seen {
+                            return Err(WasiStateCreationError::EnvironmentVariableFormatError(
+                                format!(
+                                    "found '=' in env var string \"{}\" (key=value)",
+                                    std::str::from_utf8(env)
+                                        .unwrap_or("Inner error: env var is invalid_utf8!")
+                                ),
+                            ));
+                        }
+                        eq_seen = true;
+                    }
+                    0 => {
+                        return Err(WasiStateCreationError::EnvironmentVariableFormatError(
+                            format!(
+                                "found nul byte in env var string \"{}\" (key=value)",
+                                std::str::from_utf8(env)
+                                    .unwrap_or("Inner error: env var is invalid_utf8!")
+                            ),
+                        ));
+                    }
+                    _ => (),
+                }
+            }
+        }
+
+        for po_f in self.preopened_files.iter() {
+            if !po_f.exists() {
+                return Err(WasiStateCreationError::PreopenedDirectoryNotFound(
+                    po_f.clone(),
+                ));
+            }
+        }
+
+        for (alias, po_f) in self.mapped_dirs.iter() {
+            if !po_f.exists() {
+                return Err(WasiStateCreationError::PreopenedDirectoryNotFound(
+                    po_f.clone(),
+                ));
+            }
+            validate_mapped_dir_alias(&alias)?;
+        }
+        Ok(WasiState {
+            fs: WasiFs::new(&self.preopened_files, &self.mapped_dirs)
+                .map_err(WasiStateCreationError::WasiFsCreationError)?,
+            args: self.args.clone(),
+            envs: self.envs.clone(),
+        })
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn env_var_errors() {
+        let output = create_wasi_state("test_prog")
+            .env("HOM=E", "/home/home")
+            .build();
+        match output {
+            Err(WasiStateCreationError::EnvironmentVariableFormatError(_)) => assert!(true),
+            _ => assert!(false),
+        }
+
+        let output = create_wasi_state("test_prog")
+            .env("HOME\0", "/home/home")
+            .build();
+        match output {
+            Err(WasiStateCreationError::EnvironmentVariableFormatError(_)) => assert!(true),
+            _ => assert!(false),
+        }
+    }
+
+    #[test]
+    fn nul_character_in_args() {
+        let output = create_wasi_state("test_prog").arg("--h\0elp").build();
+        match output {
+            Err(WasiStateCreationError::ArgumentContainsNulByte(_)) => assert!(true),
+            _ => assert!(false),
+        }
+        let output = create_wasi_state("test_prog")
+            .args(&["--help", "--wat\0"])
+            .build();
+        match output {
+            Err(WasiStateCreationError::ArgumentContainsNulByte(_)) => assert!(true),
+            _ => assert!(false),
+        }
+    }
+}

lib/wasi/src/state/mod.rs

@@ -13,8 +13,10 @@
 //! You can implement `WasiFile` for your own types to get custom behavior and extend WASI, see the
 //! [WASI plugin example](https://github.com/wasmerio/wasmer/blob/master/examples/plugin.rs).
 
+mod builder;
 mod types;
 
+pub use self::builder::*;
 pub use self::types::*;
 use crate::syscalls::types::*;
 use generational_arena::Arena;
@@ -140,7 +142,7 @@ pub struct WasiFs {
 
 impl WasiFs {
     pub fn new(
-        preopened_dirs: &[String],
+        preopened_dirs: &[PathBuf],
         mapped_dirs: &[(String, PathBuf)],
     ) -> Result<Self, String> {
         debug!("wasi::fs::inodes");
@@ -188,11 +190,10 @@ impl WasiFs {
 
         debug!("wasi::fs::preopen_dirs");
         for dir in preopened_dirs {
-            debug!("Attempting to preopen {}", &dir);
+            debug!("Attempting to preopen {}", &dir.to_string_lossy());
             // TODO: think about this
             let default_rights = 0x1FFFFFFF; // all rights
-            let cur_dir = PathBuf::from(dir);
-            let cur_dir_metadata = cur_dir.metadata().map_err(|e| {
+            let cur_dir_metadata = dir.metadata().map_err(|e| {
                 format!(
                     "Could not get metadata for file {:?}: {}",
                     dir,
@@ -202,18 +203,18 @@ impl WasiFs {
             let kind = if cur_dir_metadata.is_dir() {
                 Kind::Dir {
                     parent: Some(root_inode),
-                    path: cur_dir.clone(),
+                    path: dir.clone(),
                     entries: Default::default(),
                 }
             } else {
                 return Err(format!(
                     "WASI only supports pre-opened directories right now; found \"{}\"",
-                    &dir
+                    &dir.to_string_lossy()
                 ));
             };
             // TODO: handle nested pats in `file`
             let inode = wasi_fs
-                .create_inode(kind, true, dir.to_string())
+                .create_inode(kind, true, dir.to_string_lossy().into_owned())
                 .map_err(|e| {
                     format!(
                         "Failed to create inode for preopened dir: WASI error code: {}",
@@ -231,7 +232,9 @@ impl WasiFs {
                 .map_err(|e| format!("Could not open fd for file {:?}: {}", dir, e))?;
             if let Kind::Root { entries } = &mut wasi_fs.inodes[root_inode].kind {
                 // todo handle collisions
-                assert!(entries.insert(dir.to_string(), inode).is_none())
+                assert!(entries
+                    .insert(dir.to_string_lossy().into_owned(), inode)
+                    .is_none())
             }
             wasi_fs.preopen_fds.push(fd);
         }
@@ -1025,6 +1028,31 @@ pub struct WasiState {
 }
 
 impl WasiState {
+    /// Create a [`WasiStateBuilder`] to construct a validated instance of
+    /// [`WasiState`].
+    ///
+    /// Usage:
+    ///
+    /// ```
+    /// # use wasmer_wasi::state::WasiState;
+    /// WasiState::new("program_name")
+    ///    .env(b"HOME", "/home/home".to_string())
+    ///    .arg("--help")
+    ///    .envs({ let mut hm = std::collections::HashMap::new();
+    ///            hm.insert("COLOR_OUTPUT", "TRUE");
+    ///            hm.insert("PATH", "/usr/bin");
+    ///            hm
+    ///          })
+    ///    .args(&["--verbose", "list"])
+    ///    .preopen_dir("src")
+    ///    .map_dir("dot", ".")
+    ///    .build()
+    ///    .unwrap();
+    /// ```
+    pub fn new(program_name: &str) -> WasiStateBuilder {
+        create_wasi_state(program_name)
+    }
+
     /// Turn the WasiState into bytes
     pub fn freeze(&self) -> Option<Vec<u8>> {
         bincode::serialize(self).ok()

src/bin/wasmer.rs

@@ -53,7 +53,7 @@ mod wasmer_wasi {
     pub fn generate_import_object(
         _args: Vec<Vec<u8>>,
         _envs: Vec<Vec<u8>>,
-        _preopened_files: Vec<String>,
+        _preopened_files: Vec<std::path::PathBuf>,
         _mapped_dirs: Vec<(String, std::path::PathBuf)>,
     ) -> ImportObject {
         unimplemented!()
@@ -142,7 +142,7 @@ struct Run {
 
     /// WASI pre-opened directory
     #[structopt(long = "dir", multiple = true, group = "wasi")]
-    pre_opened_directories: Vec<String>,
+    pre_opened_directories: Vec<PathBuf>,
 
     /// Map a host directory to a different location for the wasm module
     #[structopt(long = "mapdir", multiple = true)]