stack overflow on URLs with an invalid port

[bcantrill]

Thanks for this crate! I had a stack overflow when attempting to parse a bad URL (https://github.com:crypto-browserify/browserify-rsa.git); here's a diff with both a test case and a potential fix:

diff --git a/src/lib.rs b/src/lib.rs
index a311e1f..cb0984d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -364,8 +364,7 @@ pub fn normalize_url(url: &str) -> Result<Url> {
                 }
             }
         }
-        Err(_e) => {
-            // e will most likely be url::ParseError::RelativeUrlWithoutBase
+        Err(url::ParseError::RelativeUrlWithoutBase) => {
             // If we're here, we're only looking for Scheme::Ssh or Scheme::File
 
             // Assuming we have found Scheme::Ssh if we can find an "@" before ":"
@@ -387,5 +386,8 @@ pub fn normalize_url(url: &str) -> Result<Url> {
                 }
             }
         }
+        Err(err) => {
+            return Err(eyre!("url parsing failed: {:?}", err));
+        }
     })
 }
diff --git a/tests/parse.rs b/tests/parse.rs
index 7619ca6..e6e6558 100644
--- a/tests/parse.rs
+++ b/tests/parse.rs
@@ -366,3 +366,15 @@ fn ssh_without_organization() {
 
     assert_eq!(parsed, expected);
 }
+
+#[test]
+fn bad_port_number() {
+    let test_url = "https://github.com:crypto-browserify/browserify-rsa.git";
+    let e = GitUrl::parse(test_url);
+
+    assert!(e.is_err());
+    assert_eq!(
+        format!("{}", e.err().unwrap()),
+        "Url normalization into url::Url failed"
+    );
+}

With that diff, cargo test runs without failures, but based on the uncertainty in the (deleted) comment around url::ParseError::RelativeUrlWithoutBase, there may be a reason why it wasn't done this way originally...

[tjtelan]

@bcantrill Hi, sorry I overlooked this issue. Thanks for the fix! I'll publish a new version of the crate soon.

    Err(_e) => {
        // e will most likely be url::ParseError::RelativeUrlWithoutBase

Some context behind that comment is this crate was my first "serious" Rust code. I was trying to speedrun translating a small library from another language into Rust.

I remember I didn't have a comfortable grasp on error handling with Result. So I probably got impatient and left this note with intent to handle the error properly in the future. Since the compiler warning was gone it got forgotten.

[bcantrill]

No need to apologize -- it was quick to debug, and glad the fix works! Thanks for integrating it!