Rewrite Destination to no longer use uriparse library

CHANGELOG.md

@@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - New `ClientConnectConfig` to support connect settings, specifically for ssh
+- `Host` with `HostParseError` that follows the 
+  [DoD Internet Host Table Specification](https://www.ietf.org/rfc/rfc0952.txt)
+  and subsequent [RFC-1123](https://www.rfc-editor.org/rfc/rfc1123)
+
+### Changed
+
+- `Destination` now has direct fields for scheme, username, password, host, and
+  port that are populated from parsing
+- `Destination` no longer wraps `uriparse::URI` and all references to
+  implementing/wrapping have been removed
 
 ### Fixed
 
@@ -18,6 +28,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   checks being incorrect (e.g. `backend` instead of `ssh.backend`). This has
   now been corrected and settings now properly get applied
 
+### Removed
+
+- The ssh settings of `ssh.user` and `ssh.port` were unused as these were now
+  being taking from the destination `ssh://[username:]host[:port]`, so they
+  have now been removed to avoid confusion
+- Remove `uriparse` dependency
+
 ## [0.17.2] - 2022-08-16
 ### Added
 

Cargo.toml

@@ -49,7 +49,6 @@ tokio = { version = "1.20.1", features = ["full"] }
 toml_edit = { version = "0.14.4", features = ["serde"] }
 terminal_size = "0.2.1"
 termwiz = "0.17.1"
-uriparse = "0.6.4"
 which = "4.2.5"
 winsplit = "0.1.0"
 whoami = "1.2.1"

distant-core/Cargo.toml

@@ -34,7 +34,6 @@ shell-words = "1.1.0"
 strum = { version = "0.24.1", features = ["derive"] }
 tokio = { version = "1.20.1", features = ["full"] }
 tokio-util = { version = "0.7.3", features = ["codec"] }
-uriparse = "0.6.4"
 walkdir = "2.3.2"
 winsplit = "0.1.0"
 

distant-core/src/credentials.rs

@@ -4,12 +4,10 @@ use crate::{
 };
 use distant_net::SecretKey32;
 use serde::{de::Deserializer, ser::Serializer, Deserialize, Serialize};
-use std::{
-    convert::{TryFrom, TryInto},
-    fmt, io,
-    str::FromStr,
-};
-use uriparse::{URIReference, URI};
+use std::{convert::TryFrom, fmt, io, str::FromStr};
+
+const SCHEME: &str = "distant";
+const SCHEME_WITH_SEP: &str = "distant://";
 
 /// Represents credentials used for a distant server that is maintaining a single key
 /// across all connections
@@ -24,7 +22,7 @@ pub struct DistantSingleKeyCredentials {
 impl fmt::Display for DistantSingleKeyCredentials {
     /// Converts credentials into string in the form of `distant://[username]:{key}@{host}:{port}`
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "distant://")?;
+        write!(f, "{SCHEME}://")?;
         if let Some(username) = self.username.as_ref() {
             write!(f, "{}", username)?;
         }
@@ -35,12 +33,37 @@ impl fmt::Display for DistantSingleKeyCredentials {
 impl FromStr for DistantSingleKeyCredentials {
     type Err = io::Error;
 
-    /// Parse `distant://[username]:{key}@{host}` as credentials. Note that this requires the
+    /// Parse `distant://[username]:{key}@{host}:{port}` as credentials. Note that this requires the
     /// `distant` scheme to be included. If parsing without scheme is desired, call the
     /// [`DistantSingleKeyCredentials::try_from_uri_ref`] method instead with `require_scheme`
     /// set to false
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Self::try_from_uri_ref(s, true)
+        let destination: Destination = s
+            .parse()
+            .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))?;
+
+        // Can be scheme-less or explicitly distant
+        if let Some(scheme) = destination.scheme.as_deref() {
+            if scheme != SCHEME {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidData,
+                    format!("Unexpected scheme: {scheme}"),
+                ));
+            }
+        }
+
+        Ok(Self {
+            host: destination.host.to_string(),
+            port: destination
+                .port
+                .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing port"))?,
+            key: destination
+                .password
+                .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing key"))?
+                .parse()
+                .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))?,
+            username: destination.username,
+        })
     }
 }
 
@@ -63,71 +86,157 @@ impl<'de> Deserialize<'de> for DistantSingleKeyCredentials {
 }
 
 impl DistantSingleKeyCredentials {
-    /// Converts credentials into a [`Destination`] of the form `distant://[username]:{key}@{host}`,
-    /// failing if the credentials would not produce a valid [`Destination`]
+    /// Searches a str for `distant://[username]:{key}@{host}:{port}`, returning the first matching
+    /// credentials set if found
+    pub fn find(s: &str) -> Option<DistantSingleKeyCredentials> {
+        let is_boundary = |c| char::is_whitespace(c) || char::is_control(c);
+
+        for (i, _) in s.match_indices(SCHEME_WITH_SEP) {
+            // Start at the scheme
+            let (before, s) = s.split_at(i);
+
+            // Check character preceding the scheme to make sure it isn't a different scheme
+            // Only whitespace or control characters preceding are okay, anything else is skipped
+            if !before.is_empty() && !before.ends_with(is_boundary) {
+                continue;
+            }
+
+            // Consume until we reach whitespace, which indicates the potential end
+            let s = match s.find(is_boundary) {
+                Some(i) => &s[..i],
+                None => s,
+            };
+
+            match s.parse::<Self>() {
+                Ok(this) => return Some(this),
+                Err(_) => continue,
+            }
+        }
+
+        None
+    }
+
+    /// Converts credentials into a [`Destination`] of the form
+    /// `distant://[username]:{key}@{host}:{port}`, failing if the credentials would not produce a
+    /// valid [`Destination`]
     pub fn try_to_destination(&self) -> io::Result<Destination> {
-        let uri = self.try_to_uri()?;
-        Destination::try_from(uri.as_uri_reference().to_borrowed())
-            .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))
+        TryFrom::try_from(self.clone())
     }
+}
+
+impl TryFrom<DistantSingleKeyCredentials> for Destination {
+    type Error = io::Error;
 
-    /// Converts credentials into a [`URI`] of the form `distant://[username]:{key}@{host}`,
-    /// failing if the credentials would not produce a valid [`URI`]
-    pub fn try_to_uri(&self) -> io::Result<URI<'static>> {
-        let uri_string = self.to_string();
-        URI::try_from(uri_string.as_str())
-            .map(URI::into_owned)
-            .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))
+    fn try_from(credentials: DistantSingleKeyCredentials) -> Result<Self, Self::Error> {
+        Ok(Destination {
+            scheme: Some("distant".to_string()),
+            username: credentials.username,
+            password: Some(credentials.key.to_string()),
+            host: credentials
+                .host
+                .parse()
+                .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))?,
+            port: Some(credentials.port),
+        })
     }
+}
 
-    /// Parses credentials from a [`URIReference`], failing if the input was not a valid
-    /// [`URIReference`] or if required parameters like `host` or `password` are missing or bad
-    /// format
-    ///
-    /// If `require_scheme` is true, will enforce that a scheme is provided. Regardless, if a
-    /// scheme is provided that is not `distant`, this will also fail
-    pub fn try_from_uri_ref<'a, E>(
-        uri_ref: impl TryInto<URIReference<'a>, Error = E>,
-        require_scheme: bool,
-    ) -> io::Result<Self>
-    where
-        E: std::error::Error + Send + Sync + 'static,
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use once_cell::sync::Lazy;
+
+    const HOST: &str = "testhost";
+    const PORT: u16 = 12345;
+
+    const USER: &str = "testuser";
+    static KEY: Lazy<String> = Lazy::new(|| SecretKey32::default().to_string());
+
+    static CREDENTIALS_STR_NO_USER: Lazy<String> = Lazy::new(|| {
+        let key = KEY.as_str();
+        format!("distant://:{key}@{HOST}:{PORT}")
+    });
+    static CREDENTIALS_STR_USER: Lazy<String> = Lazy::new(|| {
+        let key = KEY.as_str();
+        format!("distant://{USER}:{key}@{HOST}:{PORT}")
+    });
+
+    static CREDENTIALS_NO_USER: Lazy<DistantSingleKeyCredentials> =
+        Lazy::new(|| CREDENTIALS_STR_NO_USER.parse().unwrap());
+    static CREDENTIALS_USER: Lazy<DistantSingleKeyCredentials> =
+        Lazy::new(|| CREDENTIALS_STR_USER.parse().unwrap());
+
+    #[test]
+    fn find_should_return_some_key_if_string_is_exact_match() {
+        let credentials = DistantSingleKeyCredentials::find(CREDENTIALS_STR_NO_USER.as_str());
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+
+        let credentials = DistantSingleKeyCredentials::find(CREDENTIALS_STR_USER.as_str());
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_USER);
+    }
+
+    #[test]
+    fn find_should_return_some_key_if_there_is_a_match_with_only_whitespace_on_either_side() {
+        let s = format!(" {} ", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+
+        let s = format!("\r{}\r", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+
+        let s = format!("\t{}\t", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+
+        let s = format!("\n{}\n", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+    }
+
+    #[test]
+    fn find_should_return_some_key_if_there_is_a_match_with_only_control_characters_on_either_side()
     {
-        let uri_ref = uri_ref
-            .try_into()
-            .map_err(|x| io::Error::new(io::ErrorKind::Other, x))?;
+        let s = format!("\x1b{} \x1b", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+    }
 
-        // Check if the scheme is correct, and if missing if we require it
-        if let Some(scheme) = uri_ref.scheme() {
-            if !scheme.as_str().eq_ignore_ascii_case("distant") {
-                return Err(io::Error::new(
-                    io::ErrorKind::InvalidInput,
-                    "Scheme is not distant",
-                ));
-            }
-        } else if require_scheme {
-            return Err(io::Error::new(
-                io::ErrorKind::InvalidInput,
-                "Missing scheme",
-            ));
-        }
+    #[test]
+    fn find_should_return_first_match_found_in_str() {
+        let s = format!(
+            "{} {}",
+            CREDENTIALS_STR_NO_USER.as_str(),
+            CREDENTIALS_STR_USER.as_str()
+        );
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+    }
 
-        Ok(Self {
-            host: uri_ref
-                .host()
-                .map(ToString::to_string)
-                .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "Missing host"))?,
-            port: uri_ref
-                .port()
-                .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "Missing port"))?,
-            key: uri_ref
-                .password()
-                .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "Missing password"))
-                .and_then(|x| {
-                    x.parse()
-                        .map_err(|x| io::Error::new(io::ErrorKind::InvalidInput, x))
-                })?,
-            username: uri_ref.username().map(ToString::to_string),
-        })
+    #[test]
+    fn find_should_return_first_valid_match_found_in_str() {
+        let s = format!(
+            "a{}a {} b{}b",
+            CREDENTIALS_STR_NO_USER.as_str(),
+            CREDENTIALS_STR_NO_USER.as_str(),
+            CREDENTIALS_STR_NO_USER.as_str()
+        );
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials.unwrap(), *CREDENTIALS_NO_USER);
+    }
+
+    #[test]
+    fn find_should_return_none_if_no_match_found() {
+        let s = format!("a{}", CREDENTIALS_STR_NO_USER.as_str());
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials, None);
+
+        let s = format!(
+            "a{} b{}",
+            CREDENTIALS_STR_NO_USER.as_str(),
+            CREDENTIALS_STR_NO_USER.as_str()
+        );
+        let credentials = DistantSingleKeyCredentials::find(&s);
+        assert_eq!(credentials, None);
     }
 }