Merge pull request #65 from openssh-rust/refactor

jonhoo · web-flow · commit 85fa7e92476c · 2022-04-14T18:44:59.000-07:00
Refactor crate
diff --git a/Cargo.toml b/Cargo.toml
@@ -38,6 +38,8 @@ native-mux = ["openssh-mux-client"]
 [dependencies]
 tempfile = "3.1.0"
 shell-escape = "0.1.5"
+thiserror = "1.0.30"
+
 tokio = { version = "1", features = [ "process", "io-util", "macros" ] }
 tokio-pipe = "0.2.8"
 
diff --git a/src/error.rs b/src/error.rs
@@ -1,33 +1,38 @@
-use std::fmt;
 use std::io;
 
 /// Errors that occur when interacting with a remote process.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 #[non_exhaustive]
 pub enum Error {
     /// The master connection failed.
-    Master(io::Error),
+    #[error("the master connection failed")]
+    Master(#[source] io::Error),
 
     /// Failed to establish initial connection to the remote host.
-    Connect(io::Error),
+    #[error("failed to connect to the remote host")]
+    Connect(#[source] io::Error),
 
     /// Failed to run the `ssh` command locally.
     #[cfg(feature = "process-mux")]
     #[cfg_attr(docsrs, doc(cfg(feature = "process-mux")))]
-    Ssh(io::Error),
+    #[error("the local ssh command could not be executed")]
+    Ssh(#[source] io::Error),
 
     /// Failed to connect to the ssh multiplex server.
     #[cfg(feature = "native-mux")]
     #[cfg_attr(docsrs, doc(cfg(feature = "native-mux")))]
-    SshMux(openssh_mux_client::Error),
+    #[error("failed to connect to the ssh multiplex server")]
+    SshMux(#[source] openssh_mux_client::Error),
 
     /// Invalid command that contains null byte.
     #[cfg(feature = "native-mux")]
     #[cfg_attr(docsrs, doc(cfg(feature = "native-mux")))]
+    #[error("invalid command: Command contains null byte.")]
     InvalidCommand,
 
     /// The remote process failed.
-    Remote(io::Error),
+    #[error("the remote command could not be executed")]
+    Remote(#[source] io::Error),
 
     /// The connection to the remote host was severed.
     ///
@@ -36,6 +41,7 @@ pub enum Error {
     ///
     /// You should call [`Session::check`](crate::Session::check) to verify if you get
     /// this error back.
+    #[error("the connection was terminated")]
     Disconnected,
 
     /// Remote process is terminated.
@@ -51,13 +57,16 @@ pub enum Error {
     /// instead of `Disconnect`ed.
     ///
     /// It is thus recommended to create your own workaround for your particular use cases.
+    #[error("the remote process has terminated")]
     RemoteProcessTerminated,
 
     /// Failed to remove temporary dir where ssh socket and output is stored.
-    Cleanup(io::Error),
+    #[error("failed to remove temporary ssh session directory")]
+    Cleanup(#[source] io::Error),
 
     /// IO Error when creating/reading/writing from ChildStdin, ChildStdout, ChildStderr.
-    ChildIo(io::Error),
+    #[error("failure while accessing standard i/o of remote process")]
+    ChildIo(#[source] io::Error),
 }
 
 #[cfg(feature = "native-mux")]
@@ -83,56 +92,6 @@ impl From<openssh_mux_client::Error> for Error {
     }
 }
 
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match *self {
-            Error::Master(_) => write!(f, "the master connection failed"),
-            Error::Connect(_) => write!(f, "failed to connect to the remote host"),
-
-            #[cfg(feature = "process-mux")]
-            Error::Ssh(_) => write!(f, "the local ssh command could not be executed"),
-
-            Error::Remote(_) => write!(f, "the remote command could not be executed"),
-            Error::Disconnected => write!(f, "the connection was terminated"),
-            Error::Cleanup(_) => write!(f, "failed to remove temporary ssh session directory"),
-            Error::ChildIo(_) => {
-                write!(f, "failure while accessing standard I/O of remote process")
-            }
-
-            Error::RemoteProcessTerminated => write!(f, "the remote process has terminated"),
-
-            #[cfg(feature = "native-mux")]
-            Error::SshMux(_) => write!(f, "failed to connect to the ssh multiplex server"),
-
-            #[cfg(feature = "native-mux")]
-            Error::InvalidCommand => write!(f, "invalid command: Command contains null byte."),
-        }
-    }
-}
-
-impl std::error::Error for Error {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match *self {
-            Error::Master(ref e)
-            | Error::Connect(ref e)
-            | Error::Remote(ref e)
-            | Error::Cleanup(ref e)
-            | Error::ChildIo(ref e) => Some(e),
-
-            Error::RemoteProcessTerminated | Error::Disconnected => None,
-
-            #[cfg(feature = "native-mux")]
-            Error::InvalidCommand => None,
-
-            #[cfg(feature = "process-mux")]
-            Error::Ssh(ref e) => Some(e),
-
-            #[cfg(feature = "native-mux")]
-            Error::SshMux(ref e) => Some(e),
-        }
-    }
-}
-
 impl Error {
     pub(crate) fn interpret_ssh_error(stderr: &str) -> Self {
         // we want to turn the string-only ssh error into something a little more "handleable".
diff --git a/src/lib.rs b/src/lib.rs
@@ -153,13 +153,12 @@
 #[cfg(not(unix))]
 compile_error!("This crate can only be used on unix");
 
-use std::borrow::Cow;
-use std::ffi::OsStr;
-use std::path::Path;
-
 mod stdio;
 pub use stdio::{ChildStderr, ChildStdin, ChildStdout, Stdio};
 
+mod session;
+pub use session::Session;
+
 mod builder;
 pub use builder::{KnownHosts, SessionBuilder};
 
@@ -189,233 +188,3 @@ pub use port_forwarding::*;
 pub mod process {
     pub use super::{ChildStderr, ChildStdin, ChildStdout, Command, RemoteChild, Stdio};
 }
-
-#[derive(Debug)]
-pub(crate) enum SessionImp {
-    #[cfg(feature = "process-mux")]
-    ProcessImpl(process_impl::Session),
-
-    #[cfg(feature = "native-mux")]
-    NativeMuxImpl(native_mux_impl::Session),
-}
-
-#[cfg(any(feature = "process-mux", feature = "native-mux"))]
-macro_rules! delegate {
-    ($impl:expr, $var:ident, $then:block) => {{
-        match $impl {
-            #[cfg(feature = "process-mux")]
-            SessionImp::ProcessImpl($var) => $then,
-
-            #[cfg(feature = "native-mux")]
-            SessionImp::NativeMuxImpl($var) => $then,
-        }
-    }};
-}
-
-#[cfg(not(any(feature = "process-mux", feature = "native-mux")))]
-macro_rules! delegate {
-    ($impl:expr, $var:ident, $then:block) => {{
-        unreachable!("Neither feature process-mux nor native-mux is enabled")
-    }};
-}
-
-/// A single SSH session to a remote host.
-///
-/// You can use [`command`](Session::command) to start a new command on the connected machine.
-///
-/// When the `Session` is dropped, the connection to the remote host is severed, and any errors
-/// silently ignored. To disconnect and be alerted to errors, use [`close`](Session::close).
-#[derive(Debug)]
-pub struct Session(SessionImp);
-
-#[cfg(feature = "process-mux")]
-impl From<process_impl::Session> for Session {
-    fn from(imp: process_impl::Session) -> Self {
-        Self(SessionImp::ProcessImpl(imp))
-    }
-}
-
-#[cfg(feature = "native-mux")]
-impl From<native_mux_impl::Session> for Session {
-    fn from(imp: native_mux_impl::Session) -> Self {
-        Self(SessionImp::NativeMuxImpl(imp))
-    }
-}
-
-// TODO: UserKnownHostsFile for custom known host fingerprint.
-
-impl Session {
-    /// Connect to the host at the given `host` over SSH using process impl, which will
-    /// spawn a new ssh process for each `Child` created.
-    ///
-    /// The format of `destination` is the same as the `destination` argument to `ssh`. It may be
-    /// specified as either `[user@]hostname` or a URI of the form `ssh://[user@]hostname[:port]`.
-    ///
-    /// If connecting requires interactive authentication based on `STDIN` (such as reading a
-    /// password), the connection will fail. Consider setting up keypair-based authentication
-    /// instead.
-    ///
-    /// For more options, see [`SessionBuilder`].
-    #[cfg(feature = "process-mux")]
-    #[cfg_attr(docsrs, doc(cfg(feature = "process-mux")))]
-    pub async fn connect<S: AsRef<str>>(destination: S, check: KnownHosts) -> Result<Self, Error> {
-        let mut s = SessionBuilder::default();
-        s.known_hosts_check(check);
-        s.connect(destination.as_ref()).await
-    }
-
-    /// Connect to the host at the given `host` over SSH using native mux impl, which
-    /// will create a new socket connection for each `Child` created.
-    ///
-    /// See the crate-level documentation for more details on the difference between native and process-based mux.
-    ///
-    /// The format of `destination` is the same as the `destination` argument to `ssh`. It may be
-    /// specified as either `[user@]hostname` or a URI of the form `ssh://[user@]hostname[:port]`.
-    ///
-    /// If connecting requires interactive authentication based on `STDIN` (such as reading a
-    /// password), the connection will fail. Consider setting up keypair-based authentication
-    /// instead.
-    ///
-    /// For more options, see [`SessionBuilder`].
-    #[cfg(feature = "native-mux")]
-    #[cfg_attr(docsrs, doc(cfg(feature = "native-mux")))]
-    pub async fn connect_mux<S: AsRef<str>>(
-        destination: S,
-        check: KnownHosts,
-    ) -> Result<Self, Error> {
-        let mut s = SessionBuilder::default();
-        s.known_hosts_check(check);
-        s.connect_mux(destination.as_ref()).await
-    }
-
-    /// Check the status of the underlying SSH connection.
-    #[cfg(not(windows))]
-    #[cfg_attr(docsrs, doc(cfg(not(windows))))]
-    pub async fn check(&self) -> Result<(), Error> {
-        delegate!(&self.0, imp, { imp.check().await })
-    }
-
-    /// Get the SSH connection's control socket path.
-    #[cfg(not(windows))]
-    #[cfg_attr(docsrs, doc(cfg(not(windows))))]
-    pub fn control_socket(&self) -> &Path {
-        delegate!(&self.0, imp, { imp.ctl() })
-    }
-
-    /// Constructs a new [`Command`] for launching the program at path `program` on the remote
-    /// host.
-    ///
-    /// Before it is passed to the remote host, `program` is escaped so that special characters
-    /// aren't evaluated by the remote shell. If you do not want this behavior, use
-    /// [`raw_command`](Session::raw_command).
-    ///
-    /// The returned `Command` is a builder, with the following default configuration:
-    ///
-    /// * No arguments to the program
-    /// * Empty stdin and dsicard stdout/stderr for `spawn` or `status`, but create output pipes for
-    ///   `output`
-    ///
-    /// Builder methods are provided to change these defaults and otherwise configure the process.
-    ///
-    /// If `program` is not an absolute path, the `PATH` will be searched in an OS-defined way on
-    /// the host.
-    pub fn command<'a, S: Into<Cow<'a, str>>>(&self, program: S) -> Command<'_> {
-        self.raw_command(&*shell_escape::unix::escape(program.into()))
-    }
-
-    /// Constructs a new [`Command`] for launching the program at path `program` on the remote
-    /// host.
-    ///
-    /// Unlike [`command`](Session::command), this method does not shell-escape `program`, so it may be evaluated in
-    /// unforeseen ways by the remote shell.
-    ///
-    /// The returned `Command` is a builder, with the following default configuration:
-    ///
-    /// * No arguments to the program
-    /// * Empty stdin and dsicard stdout/stderr for `spawn` or `status`, but create output pipes for
-    ///   `output`
-    ///
-    /// Builder methods are provided to change these defaults and otherwise configure the process.
-    ///
-    /// If `program` is not an absolute path, the `PATH` will be searched in an OS-defined way on
-    /// the host.
-    pub fn raw_command<S: AsRef<OsStr>>(&self, program: S) -> Command<'_> {
-        Command::new(
-            self,
-            delegate!(&self.0, imp, { imp.raw_command(program).into() }),
-        )
-    }
-
-    /// Constructs a new [`Command`] that runs the provided shell command on the remote host.
-    ///
-    /// The provided command is passed as a single, escaped argument to `sh -c`, and from that
-    /// point forward the behavior is up to `sh`. Since this executes a shell command, keep in mind
-    /// that you are subject to the shell's rules around argument parsing, such as whitespace
-    /// splitting, variable expansion, and other funkyness. I _highly_ recommend you read
-    /// [this article] if you observe strange things.
-    ///
-    /// While the returned `Command` is a builder, like for [`command`](Session::command), you should not add
-    /// additional arguments to it, since the arguments are already passed within the shell
-    /// command.
-    ///
-    /// # Non-standard Remote Shells
-    ///
-    /// It is worth noting that there are really _two_ shells at work here: the one that sshd
-    /// launches for the session, and that launches are command; and the instance of `sh` that we
-    /// launch _in_ that session. This method tries hard to ensure that the provided `command` is
-    /// passed exactly as-is to `sh`, but this is complicated by the presence of the "outer" shell.
-    /// That outer shell may itself perform argument splitting, variable expansion, and the like,
-    /// which might produce unintuitive results. For example, the outer shell may try to expand a
-    /// variable that is only defined in the inner shell, and simply produce an empty string in the
-    /// variable's place by the time it gets to `sh`.
-    ///
-    /// To counter this, this method assumes that the remote shell (the one launched by `sshd`) is
-    /// [POSIX compliant]. This is more or less equivalent to "supports `bash` syntax" if you don't
-    /// look too closely. It uses [`shell-escape`] to escape `command` before sending it to the
-    /// remote shell, with the expectation that the remote shell will only end up undoing that one
-    /// "level" of escaping, thus producing the original `command` as an argument to `sh`. This
-    /// works _most of the time_.
-    ///
-    /// With sufficiently complex or weird commands, the escaping of `shell-escape` may not fully
-    /// match the "un-escaping" of the remote shell. This will manifest as escape characters
-    /// appearing in the `sh` command that you did not intend to be there. If this happens, try
-    /// changing the remote shell if you can, or fall back to [`command`](Session::command)
-    /// and do the escaping manually instead.
-    ///
-    ///   [POSIX compliant]: https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
-    ///   [this article]: https://mywiki.wooledge.org/Arguments
-    ///   [`shell-escape`]: https://crates.io/crates/shell-escape
-    pub fn shell<S: AsRef<str>>(&self, command: S) -> Command<'_> {
-        let mut cmd = self.command("sh");
-        cmd.arg("-c").arg(command);
-        cmd
-    }
-
-    /// Request to open a local/remote port forwarding.
-    /// The `Socket` can be either a unix socket or a tcp socket.
-    ///
-    /// If `forward_type` == Local, then `listen_socket` on local machine will be
-    /// forwarded to `connect_socket` on remote machine.
-    ///
-    /// Otherwise, `listen_socket` on the remote machine will be forwarded to `connect_socket`
-    /// on the local machine.
-    ///
-    /// Currently, there is no way of stopping a port forwarding due to the fact that
-    /// openssh multiplex server/master does not support this.
-    pub async fn request_port_forward(
-        &self,
-        forward_type: ForwardType,
-        listen_socket: Socket<'_>,
-        connect_socket: Socket<'_>,
-    ) -> Result<(), Error> {
-        delegate!(&self.0, imp, {
-            imp.request_port_forward(forward_type, listen_socket, connect_socket)
-                .await
-        })
-    }
-
-    /// Terminate the remote connection.
-    pub async fn close(self) -> Result<(), Error> {
-        delegate!(self.0, imp, { imp.close().await })
-    }
-}
diff --git a/src/session.rs b/src/session.rs
