fix: assure passwords aren't returned in percent-encoded form.

Author: Byron

Cargo.lock

@@ -26,6 +26,7 @@ serde = { version = "1.0.114", optional = true, default-features = false, featur
 thiserror = "2.0.0"
 url = "2.5.2"
 bstr = { version = "1.3.0", default-features = false, features = ["std"] }
+percent-encoding = "2.3.1"
 
 document-features = { version = "0.2.0", optional = true }
 

gix-url/Cargo.toml

@@ -81,7 +81,7 @@ impl std::fmt::Display for Url {
         let mut storage;
         let to_print = if self.password.is_some() {
             storage = self.clone();
-            storage.password = Some("<redacted>".into());
+            storage.password = Some("redacted".into());
             &storage
         } else {
             self

gix-url/src/impls.rs

@@ -308,6 +308,10 @@ impl Url {
     }
 }
 
+fn percent_encode(s: &str) -> Cow<'_, str> {
+    percent_encoding::utf8_percent_encode(s, percent_encoding::NON_ALPHANUMERIC).into()
+}
+
 /// Serialization
 impl Url {
     /// Write this URL losslessly to `out`, ready to be parsed again.
@@ -318,10 +322,10 @@ impl Url {
         }
         match (&self.user, &self.host) {
             (Some(user), Some(host)) => {
-                out.write_all(user.as_bytes())?;
+                out.write_all(percent_encode(user).as_bytes())?;
                 if let Some(password) = &self.password {
                     out.write_all(b":")?;
-                    out.write_all(password.as_bytes())?;
+                    out.write_all(percent_encode(password).as_bytes())?;
                 }
                 out.write_all(b"@")?;
                 out.write_all(host.as_bytes())?;

gix-url/src/lib.rs

@@ -1,8 +1,8 @@
 use std::convert::Infallible;
 
-use bstr::{BStr, BString, ByteSlice};
-
 use crate::Scheme;
+use bstr::{BStr, BString, ByteSlice};
+use percent_encoding::percent_decode_str;
 
 /// The error returned by [parse()](crate::parse()).
 #[derive(Debug, thiserror::Error)]
@@ -115,13 +115,20 @@ pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error
         serialize_alternative_form: false,
         scheme,
         user: url_user(&url),
-        password: url.password().map(Into::into),
+        password: url.password().map(percent_decoded_utf8),
         host: url.host_str().map(Into::into),
         port: url.port(),
         path: url.path().into(),
     })
 }
 
+fn percent_decoded_utf8(s: &str) -> String {
+    percent_decode_str(s)
+        .decode_utf8()
+        .expect("it's not possible to sneak illegal UTF8 into a URL")
+        .into_owned()
+}
+
 pub(crate) fn scp(input: &BStr, colon: usize) -> Result<crate::Url, Error> {
     let input = input_to_utf8(input, UrlKind::Scp)?;
 
@@ -151,7 +158,7 @@ pub(crate) fn scp(input: &BStr, colon: usize) -> Result<crate::Url, Error> {
         serialize_alternative_form: true,
         scheme: url.scheme().into(),
         user: url_user(&url),
-        password: url.password().map(Into::into),
+        password: url.password().map(percent_decoded_utf8),
         host: url.host_str().map(Into::into),
         port: url.port(),
         path: path.into(),
@@ -162,7 +169,7 @@ fn url_user(url: &url::Url) -> Option<String> {
     if url.username().is_empty() && url.password().is_none() {
         None
     } else {
-        Some(url.username().into())
+        Some(percent_decoded_utf8(url.username()))
     }
 }
 

gix-url/src/parse.rs

@@ -174,7 +174,7 @@ fn display() {
     compare("/path/to/repo", "/path/to/repo", "same goes for simple paths");
     compare(
         "https://user:password@host/path",
-        "https://user:<redacted>@host/path",
-        "it visibly redacts passwords though",
+        "https://user:redacted@host/path",
+        "it visibly redacts passwords though, and it's still a valid URL",
     );
 }

gix-url/tests/access/mod.rs

@@ -39,6 +39,26 @@ fn username_and_password_and_port() -> crate::Result {
     )
 }
 
+#[test]
+fn username_and_password_with_spaces_and_port() -> crate::Result {
+    let expected = gix_url::Url::from_parts(
+        Scheme::Http,
+        Some("user name".into()),
+        Some("password secret".into()),
+        Some("example.com".into()),
+        Some(8080),
+        b"/~byron/hello".into(),
+        false,
+    )?;
+    assert_url_roundtrip(
+        "http://user%20name:password%[email protected]:8080/~byron/hello",
+        expected.clone(),
+    )?;
+    assert_eq!(expected.user(), Some("user name"));
+    assert_eq!(expected.password(), Some("password secret"));
+    Ok(())
+}
+
 #[test]
 fn only_password() -> crate::Result {
     assert_url_roundtrip(