From c88685bae6b9f1548cdbfba3ce089ab5f47ea2db Mon Sep 17 00:00:00 2001 From: Kiran Ostrolenk Date: Fri, 25 Oct 2024 10:56:06 +0100 Subject: [PATCH 1/3] chore: sate clippy --- src/lib.rs | 18 +++++------------- tests/normalize.rs | 4 ++-- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 08e4b93..9dcc043 100644 --- a/src/lib.rs +++ b/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); @@ -211,7 +211,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 +362,7 @@ fn normalize_file_path(filepath: &str) -> Result { if let Ok(file_url) = normalize_url(&format!("file://{}", filepath)) { Ok(file_url) } else { - return Err(GitUrlParseError::FileUrlNormalizeFailedSchemeAdded); + Err(GitUrlParseError::FileUrlNormalizeFailedSchemeAdded) } } } @@ -463,11 +463,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,11 +477,7 @@ 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)] diff --git a/tests/normalize.rs b/tests/normalize.rs index 1ffd37b..ea1b174 100644 --- a/tests/normalize.rs +++ b/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::(); + 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::(); + let test_url = ":".repeat(10000); let normalized = normalize_url(&test_url); assert!(normalized.is_err()); From dab8527c01c94b3f1d58ce1e9099f4b212e08778 Mon Sep 17 00:00:00 2001 From: Kiran Ostrolenk Date: Fri, 25 Oct 2024 11:08:29 +0100 Subject: [PATCH 2/3] 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:". --- src/lib.rs | 5 +++++ tests/parse.rs | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 9dcc043..0e71d85 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -170,6 +170,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 { @@ -518,6 +521,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, diff --git a/tests/parse.rs b/tests/parse.rs index deabb37..0fc0b8f 100644 --- a/tests/parse.rs +++ b/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"; From f87f247ac65d5c1a5595e7d70bd6dd503a007ece Mon Sep 17 00:00:00 2001 From: Kiran Ostrolenk Date: Fri, 25 Oct 2024 11:22:10 +0100 Subject: [PATCH 3/3] 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`. --- src/lib.rs | 11 ++--------- tests/parse.rs | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0e71d85..773f35d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -156,11 +156,7 @@ impl GitUrl { /// Returns a `Result` after normalizing and parsing `url` for metadata pub fn parse(url: &str) -> Result { // 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()) { @@ -485,12 +481,9 @@ fn is_ssh_url(url: &str) -> bool { #[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, diff --git a/tests/parse.rs b/tests/parse.rs index 0fc0b8f..ae0118c 100644 --- a/tests/parse.rs +++ b/tests/parse.rs @@ -383,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" ); }