fix: username in scp-like url is no longer percent-encoded (#2056)

Since Git doesn't percent-decode characters in scp-like URL, we shouldn't encode
username at all.

https://github.com/git/git/blob/v2.50.0/connect.c#L1081

I've split write_to() function to clarify that any non-path components that
should be separated by ":" cannot be serialized in alternative form. I've made
it fall back to the URL syntax if password or port number was set. Maybe we can
also check if the user or host includes ":", but I'm not sure how much foolproof
we should add here.

Author: yuja

gix-url/src/lib.rs

@@ -315,11 +315,23 @@ fn percent_encode(s: &str) -> Cow<'_, str> {
 /// Serialization
 impl Url {
     /// Write this URL losslessly to `out`, ready to be parsed again.
-    pub fn write_to(&self, mut out: &mut dyn std::io::Write) -> std::io::Result<()> {
-        if !(self.serialize_alternative_form && (self.scheme == Scheme::File || self.scheme == Scheme::Ssh)) {
-            out.write_all(self.scheme.as_str().as_bytes())?;
-            out.write_all(b"://")?;
+    pub fn write_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
+        // Since alternative form doesn't employ any escape syntax, password and
+        // port number cannot be encoded.
+        if self.serialize_alternative_form
+            && (self.scheme == Scheme::File || self.scheme == Scheme::Ssh)
+            && self.password.is_none()
+            && self.port.is_none()
+        {
+            self.write_alternative_form_to(out)
+        } else {
+            self.write_canonical_form_to(out)
         }
+    }
+
+    fn write_canonical_form_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
+        out.write_all(self.scheme.as_str().as_bytes())?;
+        out.write_all(b"://")?;
         match (&self.user, &self.host) {
             (Some(user), Some(host)) => {
                 out.write_all(percent_encode(user).as_bytes())?;
@@ -337,9 +349,31 @@ impl Url {
             (Some(_user), None) => unreachable!("BUG: should not be possible to have a user but no host"),
         }
         if let Some(port) = &self.port {
-            write!(&mut out, ":{port}")?;
+            write!(out, ":{port}")?;
+        }
+        out.write_all(&self.path)?;
+        Ok(())
+    }
+
+    fn write_alternative_form_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
+        match (&self.user, &self.host) {
+            (Some(user), Some(host)) => {
+                out.write_all(user.as_bytes())?;
+                assert!(
+                    self.password.is_none(),
+                    "BUG: cannot serialize password in alternative form"
+                );
+                out.write_all(b"@")?;
+                out.write_all(host.as_bytes())?;
+            }
+            (None, Some(host)) => {
+                out.write_all(host.as_bytes())?;
+            }
+            (None, None) => {}
+            (Some(_user), None) => unreachable!("BUG: should not be possible to have a user but no host"),
         }
-        if self.serialize_alternative_form && self.scheme == Scheme::Ssh {
+        assert!(self.port.is_none(), "BUG: cannot serialize port in alternative form");
+        if self.scheme == Scheme::Ssh {
             out.write_all(b":")?;
         }
         out.write_all(&self.path)?;

gix-url/tests/url/parse/ssh.rs

@@ -1,6 +1,6 @@
 use gix_url::Scheme;
 
-use crate::parse::{assert_url, assert_url_roundtrip, url, url_alternate};
+use crate::parse::{assert_url, assert_url_roundtrip, url, url_alternate, url_with_pass};
 
 #[test]
 fn without_user_and_without_port() -> crate::Result {
@@ -184,6 +184,30 @@ fn scp_like_with_windows_path_and_port_thinks_port_is_part_of_path() -> crate::R
     Ok(())
 }
 
+#[test]
+fn scp_like_with_non_alphanumeric_username() -> crate::Result {
+    let url = assert_url(
+        "_user.name@host.xz:C:/path",
+        url_alternate(Scheme::Ssh, "_user.name", "host.xz", None, b"C:/path"),
+    )?
+    .to_bstring();
+    assert_eq!(url, "_user.name@host.xz:C:/path");
+    Ok(())
+}
+
+// Git passes the non-path part "user@name@host.xz" to OpenSSH, and the ssh
+// command interprets it as user = "user@name", host = "host.xz".
+#[test]
+fn scp_like_with_username_including_at() -> crate::Result {
+    let url = assert_url(
+        "user@name@host.xz:path",
+        url_alternate(Scheme::Ssh, "user@name", "host.xz", None, b"path"),
+    )?
+    .to_bstring();
+    assert_eq!(url, "user@name@host.xz:path");
+    Ok(())
+}
+
 // Git does not care that the host is named `file`, it still treats it as an SCP url.
 // I btw tested this, yes you can really clone a repository from there, just `git init`
 // in the directory above your home directory on the remote machine.
@@ -193,3 +217,19 @@ fn strange_scp_like_with_host_named_file() -> crate::Result {
     assert_eq!(url.to_bstring(), "file:..");
     Ok(())
 }
+
+#[test]
+fn bad_alternative_form_with_password() -> crate::Result {
+    let url = url_with_pass(Scheme::Ssh, "user", "password", "host.xz", None, b"/")
+        .serialize_alternate_form(true)
+        .to_bstring();
+    assert_eq!(url, "ssh://user:password@host.xz/");
+    Ok(())
+}
+
+#[test]
+fn bad_alternative_form_with_port() -> crate::Result {
+    let url = url_alternate(Scheme::Ssh, None, "host.xz", 21, b"/").to_bstring();
+    assert_eq!(url, "ssh://host.xz:21/");
+    Ok(())
+}