Clean up testing for TTY and escaping writes

Author: Jakub Konka

crates/wasi-common/src/entry.rs

@@ -1,5 +1,5 @@
 use crate::handle::{Handle, HandleRights};
-use crate::wasi::types::{Filetype, Rights};
+use crate::wasi::types::Filetype;
 use crate::wasi::{Errno, Result};
 use std::ops::Deref;
 use std::path::PathBuf;
@@ -90,16 +90,4 @@ impl Entry {
             Err(Errno::Notcapable)
         }
     }
-
-    // TODO we should probably have a separate struct representing
-    // char device
-    /// Test whether this descriptor is considered a tty within WASI.
-    /// Note that since WASI itself lacks an `isatty` syscall and relies
-    /// on a conservative approximation, we use the same approximation here.
-    pub(crate) fn isatty(&self) -> bool {
-        let file_type = self.handle.get_file_type();
-        let rights = self.handle.get_rights();
-        let required_rights = HandleRights::from_base(Rights::FD_SEEK | Rights::FD_TELL);
-        file_type == Filetype::CharacterDevice && rights.contains(&required_rights)
-    }
 }

crates/wasi-common/src/handle.rs

@@ -61,6 +61,15 @@ pub(crate) trait Handle {
     fn is_directory(&self) -> bool {
         self.get_file_type() == types::Filetype::Directory
     }
+    /// Test whether this descriptor is considered a tty within WASI.
+    /// Note that since WASI itself lacks an `isatty` syscall and relies
+    /// on a conservative approximation, we use the same approximation here.
+    fn is_tty(&self) -> bool {
+        let file_type = self.get_file_type();
+        let rights = self.get_rights();
+        let required_rights = HandleRights::from_base(Rights::FD_SEEK | Rights::FD_TELL);
+        file_type == types::Filetype::CharacterDevice && rights.contains(&required_rights)
+    }
     // TODO perhaps should be a separate trait?
     // FdOps
     fn advise(
@@ -118,7 +127,7 @@ pub(crate) trait Handle {
     fn sync(&self) -> Result<()> {
         Ok(())
     }
-    fn write_vectored(&self, _iovs: &[io::IoSlice], _isatty: bool) -> Result<usize> {
+    fn write_vectored(&self, _iovs: &[io::IoSlice]) -> Result<usize> {
         Err(Errno::Badf)
     }
     // TODO perhaps should be a separate trait?

crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs

@@ -438,10 +438,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
         }
         let required_rights = HandleRights::from_base(types::Rights::FD_WRITE);
         let entry = self.get_entry(fd)?;
-        let isatty = entry.isatty();
         let host_nwritten = entry
             .as_handle(&required_rights)?
-            .write_vectored(&slices, isatty)?
+            .write_vectored(&slices)?
             .try_into()?;
         Ok(host_nwritten)
     }

crates/wasi-common/src/sys/osfile.rs

@@ -1,7 +1,6 @@
 use super::sys_impl::oshandle::OsHandle;
 use super::{fd, AsFile};
 use crate::handle::{Handle, HandleRights};
-use crate::sandboxed_tty_writer::SandboxedTTYWriter;
 use crate::wasi::{types, Errno, Result};
 use std::any::Any;
 use std::cell::Cell;
@@ -115,7 +114,7 @@ impl Handle for OsFile {
         let mut fd: &File = &self.as_file();
         let cur_pos = fd.seek(SeekFrom::Current(0))?;
         fd.seek(SeekFrom::Start(offset))?;
-        let nwritten = self.write_vectored(&buf, false)?;
+        let nwritten = self.write_vectored(&buf)?;
         fd.seek(SeekFrom::Start(cur_pos))?;
         Ok(nwritten)
     }
@@ -131,13 +130,8 @@ impl Handle for OsFile {
         self.as_file().sync_all()?;
         Ok(())
     }
-    fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result<usize> {
-        let mut file: &File = &self.as_file();
-        let nwritten = if isatty {
-            SandboxedTTYWriter::new(&mut file).write_vectored(&iovs)?
-        } else {
-            file.write_vectored(&iovs)?
-        };
+    fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result<usize> {
+        let nwritten = self.as_file().write_vectored(&iovs)?;
         Ok(nwritten)
     }
 }

crates/wasi-common/src/sys/osother.rs

@@ -73,9 +73,9 @@ impl Handle for OsOther {
         let nread = self.as_file().read_vectored(iovs)?;
         Ok(nread)
     }
-    fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result<usize> {
+    fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result<usize> {
         let mut fd: &File = &self.as_file();
-        let nwritten = if isatty {
+        let nwritten = if self.is_tty() {
             SandboxedTTYWriter::new(&mut fd).write_vectored(&iovs)?
         } else {
             fd.write_vectored(iovs)?

crates/wasi-common/src/sys/stdio.rs

@@ -65,14 +65,14 @@ impl Handle for Stdio {
         };
         Ok(nread)
     }
-    fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result<usize> {
+    fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result<usize> {
         let nwritten = match self {
             Self::In { .. } => return Err(Errno::Badf),
             Self::Out { .. } => {
                 // lock for the duration of the scope
                 let stdout = io::stdout();
                 let mut stdout = stdout.lock();
-                let nwritten = if isatty {
+                let nwritten = if self.is_tty() {
                     SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)?
                 } else {
                     stdout.write_vectored(&iovs)?

crates/wasi-common/src/virtfs.rs

@@ -282,11 +282,7 @@ impl Handle for InMemoryFile {
 
         Ok(self.cursor.get())
     }
-    fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result<usize> {
-        if isatty {
-            unimplemented!("writes to virtual tty");
-        }
-
+    fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result<usize> {
         trace!("write_vectored(iovs={:?})", iovs);
         let mut data = self.data.borrow_mut();