fix: reject empty paths where needed, add Url::from_parts_as_alternative_form().

The new constructor allows to create URLs that represent paths which otherwise couldn't
be valid URLs.

Author: Byron

Diff for: git-url/src/lib.rs

@@ -71,6 +71,28 @@ impl Url {
             .as_ref(),
         )
     }
+
+    /// Create a new instance from the given parts, which will be validated by parsing them back from its alternative form.
+    pub fn from_parts_as_alternative_form(
+        scheme: Scheme,
+        user: Option<String>,
+        host: Option<String>,
+        port: Option<u16>,
+        path: BString,
+    ) -> Result<Self, parse::Error> {
+        parse(
+            Url {
+                scheme,
+                user,
+                host,
+                port,
+                path,
+                serialize_alternative_form: true,
+            }
+            .to_bstring()
+            .as_ref(),
+        )
+    }
 }
 
 /// Modification

Diff for: git-url/src/parse.rs

@@ -18,8 +18,10 @@ pub enum Error {
     Url(#[from] url::ParseError),
     #[error("Protocol {protocol:?} is not supported")]
     UnsupportedProtocol { protocol: String },
-    #[error("Paths cannot be empty")]
-    EmptyPath,
+    #[error("urls require the path to the repository")]
+    MissingResourceLocation,
+    #[error("file urls require an absolute or relative path to the repository repository")]
+    MissingRepositoryPath,
     #[error("\"{url}\" is not a valid local path")]
     NotALocalFile { url: BString },
     #[error("Relative URLs are not permitted: {url:?}")]
@@ -90,32 +92,43 @@ pub fn parse(input: &BStr) -> Result<crate::Url, Error> {
     let guessed_protocol = guess_protocol(input).ok_or_else(|| Error::NotALocalFile { url: input.into() })?;
     let path_without_file_protocol = input.strip_prefix(b"file://");
     if path_without_file_protocol.is_some() || (has_no_explicit_protocol(input) && guessed_protocol == "file") {
-        return Ok(crate::Url {
-            scheme: Scheme::File,
-            path: path_without_file_protocol
-                .map(|stripped_path| {
-                    #[cfg(windows)]
-                    {
-                        if stripped_path.starts_with(b"/") {
-                            input
-                                .to_str()
-                                .ok()
-                                .and_then(|url| {
-                                    let path = url::Url::parse(url).ok()?.to_file_path().ok()?;
-                                    path.is_absolute().then(|| git_path::into_bstr(path).into_owned())
-                                })
-                                .unwrap_or_else(|| stripped_path.into())
-                        } else {
-                            stripped_path.into()
-                        }
-                    }
-                    #[cfg(not(windows))]
-                    {
+        let path: BString = path_without_file_protocol
+            .map(|stripped_path| {
+                #[cfg(windows)]
+                {
+                    if stripped_path.starts_with(b"/") {
+                        input
+                            .to_str()
+                            .ok()
+                            .and_then(|url| {
+                                let path = url::Url::parse(url).ok()?.to_file_path().ok()?;
+                                path.is_absolute().then(|| git_path::into_bstr(path).into_owned())
+                            })
+                            .unwrap_or_else(|| stripped_path.into())
+                    } else {
                         stripped_path.into()
                     }
-                })
-                .unwrap_or_else(|| input.into()),
-            serialize_alternative_form: !input.starts_with(b"file://"),
+                }
+                #[cfg(not(windows))]
+                {
+                    stripped_path.into()
+                }
+            })
+            .unwrap_or_else(|| input.into());
+        if path.is_empty() {
+            return Err(Error::MissingRepositoryPath);
+        }
+        let input_starts_with_file_protocol = input.starts_with(b"file://");
+        if input_starts_with_file_protocol {
+            let wanted = cfg!(windows).then(|| &[b'\\', b'/'] as &[_]).unwrap_or(&[b'/']);
+            if !wanted.iter().any(|w| path.contains(w)) {
+                return Err(Error::MissingRepositoryPath);
+            }
+        }
+        return Ok(crate::Url {
+            scheme: Scheme::File,
+            path,
+            serialize_alternative_form: !input_starts_with_file_protocol,
             ..Default::default()
         });
     }
@@ -143,8 +156,8 @@ pub fn parse(input: &BStr) -> Result<crate::Url, Error> {
         url = url::Url::parse(&format!("ssh://{}", sanitize_for_protocol("ssh", url_str)))?;
         sanitized_scp = true;
     }
-    if url.scheme() != "rad" && url.path().is_empty() {
-        return Err(Error::EmptyPath);
+    if url.path().is_empty() && ["ssh", "git"].contains(&url.scheme()) {
+        return Err(Error::MissingResourceLocation);
     }
     if url.cannot_be_a_base() {
         return Err(Error::RelativeUrl { url: url.into() });

Diff for: git-url/tests/parse/file.rs

@@ -79,6 +79,43 @@ fn relative_file_path_without_protocol() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn shortest_possible_absolute_path() -> crate::Result {
+    let parsed = assert_url("/", url_alternate(Scheme::File, None, None, None, b"/"))?.to_bstring();
+    assert_eq!(parsed, "/");
+    let parsed = assert_url("file:///", url(Scheme::File, None, None, None, b"/"))?.to_bstring();
+    assert_eq!(parsed, "file:///");
+    Ok(())
+}
+
+#[test]
+fn shortest_possible_relative_path() -> crate::Result {
+    let parsed = assert_url("a", url_alternate(Scheme::File, None, None, None, b"a"))?.to_bstring();
+    assert_eq!(parsed, "a");
+    let parsed = assert_url("../", url_alternate(Scheme::File, None, None, None, b"../"))?.to_bstring();
+    assert_eq!(parsed, "../");
+    let parsed = assert_url("..\\", url_alternate(Scheme::File, None, None, None, b"..\\"))?.to_bstring();
+    assert_eq!(parsed, "..\\");
+    let parsed = assert_url("./", url_alternate(Scheme::File, None, None, None, b"./"))?.to_bstring();
+    assert_eq!(parsed, "./");
+    let parsed = assert_url(".", url_alternate(Scheme::File, None, None, None, b"."))?.to_bstring();
+    assert_eq!(parsed, ".");
+    let parsed = assert_url("..", url_alternate(Scheme::File, None, None, None, b".."))?.to_bstring();
+    assert_eq!(parsed, "..");
+    let parsed = assert_url("file://../", url(Scheme::File, None, None, None, b"../"))?.to_bstring();
+    assert_eq!(parsed, "file://../");
+    let parsed = assert_url("file://./", url(Scheme::File, None, None, None, b"./"))?.to_bstring();
+    assert_eq!(parsed, "file://./");
+    #[cfg(windows)]
+    {
+        let parsed = assert_url("file://.\\", url(Scheme::File, None, None, None, b".\\"))?.to_bstring();
+        assert_eq!(parsed, "file://.\\");
+    }
+    let parsed = assert_url("file://a/", url(Scheme::File, None, None, None, b"a/"))?.to_bstring();
+    assert_eq!(parsed, "file://a/");
+    Ok(())
+}
+
 #[test]
 fn interior_relative_file_path_without_protocol() -> crate::Result {
     let url = assert_url(

Diff for: git-url/tests/parse/invalid.rs

@@ -1,18 +1,41 @@
 use crate::parse::assert_failure;
+use git_url::parse::Error;
 
 #[test]
 fn unknown_protocol() {
     assert_failure("foo://host.xz/path/to/repo.git/", "Protocol \"foo\" is not supported")
 }
 
 #[test]
-fn missing_path() {
-    assert_failure("ssh://host.xz", "Paths cannot be empty")
+fn ssh_missing_path() {
+    assert_failure("ssh://host.xz", Error::MissingResourceLocation)
+}
+
+#[test]
+fn git_missing_path() {
+    assert_failure("git://host.xz", Error::MissingResourceLocation)
+}
+
+#[test]
+fn file_missing_path() {
+    assert_failure("file://", Error::MissingRepositoryPath);
+}
+
+#[test]
+fn empty() {
+    assert_failure("", Error::MissingRepositoryPath);
+    assert_failure("file://..", Error::MissingRepositoryPath);
+    assert_failure("file://.", Error::MissingRepositoryPath);
+    #[cfg(not(windows))]
+    {
+        assert_failure("file://.\\", Error::MissingRepositoryPath);
+    }
+    assert_failure("file://a", Error::MissingRepositoryPath);
 }
 
 #[test]
 fn missing_port_despite_indication() {
-    assert_failure("ssh://host.xz:", "Paths cannot be empty")
+    assert_failure("ssh://host.xz:", Error::MissingResourceLocation)
 }
 
 #[test]

Diff for: git-url/tests/parse/mod.rs

@@ -22,8 +22,11 @@ fn assert_url_roundtrip(url: &str, expected: git_url::Url) -> crate::Result {
     Ok(())
 }
 
-fn assert_failure(url: &str, expected_err: &str) {
-    assert_eq!(git_url::parse(url.into()).unwrap_err().to_string(), expected_err);
+fn assert_failure(url: &str, expected_err: impl ToString) {
+    assert_eq!(
+        git_url::parse(url.into()).unwrap_err().to_string(),
+        expected_err.to_string()
+    );
 }
 
 fn url<'a, 'b>(
@@ -40,7 +43,7 @@ fn url<'a, 'b>(
         port.into(),
         path.into(),
     )
-    .expect("valid")
+    .unwrap_or_else(|err| panic!("'{}' failed: {err:?}", path.as_bstr()))
 }
 
 fn url_alternate<'a, 'b>(
@@ -50,7 +53,14 @@ fn url_alternate<'a, 'b>(
     port: impl Into<Option<u16>>,
     path: &[u8],
 ) -> git_url::Url {
-    url(protocol, user, host, port, path).serialize_alternate_form(true)
+    git_url::Url::from_parts_as_alternative_form(
+        protocol,
+        user.into().map(Into::into),
+        host.into().map(Into::into),
+        port.into(),
+        path.into(),
+    )
+    .expect("valid")
 }
 
 mod file;
@@ -80,7 +90,7 @@ mod radicle {
 mod http {
     use git_url::Scheme;
 
-    use crate::parse::{assert_url_roundtrip, url};
+    use crate::parse::{assert_url, assert_url_roundtrip, url};
 
     #[test]
     fn username_expansion_is_unsupported() -> crate::Result {
@@ -96,6 +106,13 @@ mod http {
             url(Scheme::Https, None, "github.com", None, b"/byron/gitoxide"),
         )
     }
+
+    #[test]
+    fn http_missing_path() -> crate::Result {
+        assert_url_roundtrip("http://host.xz/", url(Scheme::Http, None, "host.xz", None, b"/"))?;
+        assert_url("http://host.xz", url(Scheme::Http, None, "host.xz", None, b"/"))?;
+        Ok(())
+    }
 }
 mod git {
     use git_url::Scheme;