fix: prevent panic when parsing a URL with no path (#55)

* chore: sate clippy

* fix: prevent panic when parsing a URL with no path

Prior to this change `parse` would panic at `src/lib.rs:203:29` with
`index out of bounds: the len is 0 but the index is 0` if given an input
like "git:".

* feat: improve error handling

This removes a seemingly redundant and not very illuminating error
variant. It also ensures that the error message from the `Url` crate is
displayed when displaying `UrlParseError`.

Author: CardboardTurkey

src/lib.rs

@@ -107,7 +107,7 @@ impl fmt::Display for GitUrl {
                     format!(":{}", &self.path)
                 }
             }
-            _ => (&self.path).to_string(),
+            _ => self.path.to_string(),
         };
 
         let git_url_str = format!("{}{}{}{}{}", scheme_prefix, auth_info, host, port, path);
@@ -156,11 +156,7 @@ impl GitUrl {
     /// Returns a `Result<GitUrl>` after normalizing and parsing `url` for metadata
     pub fn parse(url: &str) -> Result<GitUrl, GitUrlParseError> {
         // Normalize the url so we can use Url crate to process ssh urls
-        let normalized = if let Ok(url) = normalize_url(url) {
-            url
-        } else {
-            return Err(GitUrlParseError::UrlNormalizeFailed);
-        };
+        let normalized = normalize_url(url)?;
 
         // Some pre-processing for paths
         let scheme = if let Ok(scheme) = Scheme::from_str(normalized.scheme()) {
@@ -170,6 +166,9 @@ impl GitUrl {
                 normalized.scheme().to_string(),
             ));
         };
+        if normalized.path().is_empty() {
+            return Err(GitUrlParseError::EmptyPath);
+        }
 
         // Normalized ssh urls can always have their first '/' removed
         let urlpath = match &scheme {
@@ -211,7 +210,7 @@ impl GitUrl {
                 let mut fullname: Vec<&str> = Vec::new();
 
                 // TODO: Add support for parsing out orgs from these urls
-                let hosts_w_organization_in_path = vec!["dev.azure.com", "ssh.dev.azure.com"];
+                let hosts_w_organization_in_path = ["dev.azure.com", "ssh.dev.azure.com"];
                 //vec!["dev.azure.com", "ssh.dev.azure.com", "visualstudio.com"];
 
                 let host_str = if let Some(host) = normalized.host_str() {
@@ -362,7 +361,7 @@ fn normalize_file_path(filepath: &str) -> Result<Url, GitUrlParseError> {
             if let Ok(file_url) = normalize_url(&format!("file://{}", filepath)) {
                 Ok(file_url)
             } else {
-                return Err(GitUrlParseError::FileUrlNormalizeFailedSchemeAdded);
+                Err(GitUrlParseError::FileUrlNormalizeFailedSchemeAdded)
             }
         }
     }
@@ -463,11 +462,7 @@ fn is_ssh_url(url: &str) -> bool {
 
         // Make sure we provided a username, and not just `@`
         let parts: Vec<&str> = url.split('@').collect();
-        if parts.len() != 2 && !parts[0].is_empty() {
-            return false;
-        } else {
-            return true;
-        }
+        return parts.len() == 2 || parts[0].is_empty();
     }
 
     // it's an ssh url if we have a domain:path pattern
@@ -481,21 +476,14 @@ fn is_ssh_url(url: &str) -> bool {
     // This should also handle if a port is specified
     // no port example: ssh://user@domain:path/to/repo.git
     // port example: ssh://user@domain:port/path/to/repo.git
-    if parts.len() != 2 && !parts[0].is_empty() && !parts[1].is_empty() {
-        return false;
-    } else {
-        return true;
-    }
+    parts.len() == 2 && parts[0].is_empty() && parts[1].is_empty()
 }
 
 #[derive(Error, Debug, PartialEq, Eq)]
 pub enum GitUrlParseError {
-    #[error("Error from Url crate")]
+    #[error("Error from Url crate: {0}")]
     UrlParseError(#[from] url::ParseError),
 
-    #[error("Url normalization into url::Url failed")]
-    UrlNormalizeFailed,
-
     #[error("No url scheme was found, then failed to normalize as ssh url.")]
     SshUrlNormalizeFailedNoScheme,
 
@@ -526,6 +514,8 @@ pub enum GitUrlParseError {
     UnsupportedUrlHostFormat,
     #[error("Git Url not in expected format for SSH")]
     UnsupportedSshUrlFormat,
+    #[error("Normalized URL has no path")]
+    EmptyPath,
 
     #[error("Found null bytes within input url before parsing")]
     FoundNullBytes,

tests/normalize.rs

@@ -169,7 +169,7 @@ fn null_in_input2() {
 // From https://github.com/tjtelan/git-url-parse-rs/issues/51
 #[test]
 fn large_bad_input1() {
-    let test_url = std::iter::repeat("g@1::::").take(10000).collect::<String>();
+    let test_url = "g@1::::".repeat(10000);
     let normalized = normalize_url(&test_url);
 
     assert!(normalized.is_err());
@@ -178,7 +178,7 @@ fn large_bad_input1() {
 // From https://github.com/tjtelan/git-url-parse-rs/issues/51
 #[test]
 fn large_bad_input2() {
-    let test_url = std::iter::repeat(":").take(10000).collect::<String>();
+    let test_url = ":".repeat(10000);
     let normalized = normalize_url(&test_url);
 
     assert!(normalized.is_err());

tests/parse.rs

@@ -367,6 +367,14 @@ fn ssh_without_organization() {
     assert_eq!(parsed, expected);
 }
 
+#[test]
+fn empty_path() {
+    assert_eq!(
+        GitUrlParseError::EmptyPath,
+        GitUrl::parse("git:").unwrap_err()
+    )
+}
+
 #[test]
 fn bad_port_number() {
     let test_url = "https://github.com:crypto-browserify/browserify-rsa.git";
@@ -375,7 +383,7 @@ fn bad_port_number() {
     assert!(e.is_err());
     assert_eq!(
         format!("{}", e.err().unwrap()),
-        "Url normalization into url::Url failed"
+        "Error from Url crate: invalid port number"
     );
 }